Experiment implementing po_extract() #243
Conversation
And display number of messages found
|
|
R/get_r_messages.R
Outdated
| # treat gettextf separately since it takes a named argument, and we ignore ... | ||
| get_named_arg_strings(expr_data, 'gettextf', c(fmt = 1L), recursive = TRUE), | ||
| get_named_arg_strings(expr_data, fmt_funs, c(fmt = 1L), recursive = TRUE), | ||
| if (use_tr) get_named_arg_strings(expr_data, 'tr_', c(string = 1L), recursive = TRUE), |
There was a problem hiding this comment.
an aside: tr_ might be a good opportunity to build something I'd been kind of stuck on:
Since R doesn't have parser-level string concatenation (i.e. C's 'abc' 'def') making translation-ready strings in R will tend to produce lintr-violating wide lines (esp. at 80 but also at 120 characters)... the tooling for base has no shot here because it considers all the inputs as individual strings.
But with custom functions we could allow:
tr_(
'abcd',
'efg'
)
To wind up as abcdefg in the .pot file & also run gettext() on the combined string:
tr_ <- function(...)
gettext(paste0(...))
I think it would require some effort to get it working but maybe worthwhile to prevent infinite scroll strings...
There was a problem hiding this comment.
Yeah, I like that idea. I'll change it to a dots function.
In total we might want something like:
tr_ <- function(...) {
structure(enc2utf8(gettext(paste0(...), domain = "R-pkgdown")), class = "translated")
}That ensures it's correctly flagged as UTF-8 (which gettext()) doesn't do by default, and flags it as a translated string which might be useful for other functions.
There was a problem hiding this comment.
This function could also error if given anything other than a literal string (i.e. tr_(foo) and tr_(foo()) should error because they're not translatable).
There was a problem hiding this comment.
that sounds basically right to me (requiring literal inputs), I do wonder if it will wind up being limiting. I have in mind the N_() construct in C which is needed to keep strings untranslated at compile time, then translated as a variable at run time.
Not sure if it's relevant, so probably it's OK to start being strict and allow the use cases to show themselves as a feature request.
|
Hmmm, it seems like we're cueing off different parts of this function — it both looks for (scans) and records (snapshots) messages for translation. I wonder if we can somehow get both ideas into one word ... How about |
|
|
Conflicts: R/get_message_data.R man/get_message_data.Rd
|
Ok, I think this is pretty close to being done apart from the documentation — the problem is that
Which would you prefer? |
hadley
changed the title
|
|
@MichaelChirico if you want, I can do a separate PR that converts all the .Rds to roxygen (it's 95% automated). Do you want to use |
|
That works, esp. if it'll make it easier for you to be productive going forward (and assuming it's not a huge time drain for you). I would keep an explicit NAMESPACE, I do prefer being more intentional there. |
I was reading `style = 'base'` as the call which was throwing me off. an explicit argument here should help readability.
| po_params = list( | ||
| package = desc[['Package']], | ||
| version = desc[['Version']], | ||
| copyright = NULL, |
There was a problem hiding this comment.
is your sense that these fields (copyright and bugs) aren't really needed?
my sense as well has been these things are basically covered by the DESCRIPTION file already, and kept it to try & be consistent with base (and it's something of a maintenance headache to implement)
There was a problem hiding this comment.
Yeah, I think we should remove them; it's better to leave to the DESCRIPTION and avoid creating duplicates that might get out of date.
|
To be clear, I meant using |
|
Sure. Our current convention is to put them with the package docs anyway. |
Mostly to trigger discussion. More comments below.