Export po_compile()

[hadley]

This function was previously called update_mo_files(). I don't know if dev_compile() is better, but I like the idea of using prefixes to distinguish between functions for developers and translators. Let me know if you have a better name, or if you'd prefer me to leave as is.

I also made a few changes to make it more user friendly since it's now exported: it discovers the package name (if needed), and can lazily recompile just the files that have changed. I also tweaked the messaging so you get one message per translation (which makes it easier to see what the msgfmt messages apply to).

Two questions:

• Is my scoped use of roxygen2 ok? (roxygen2 is designed to work like this so there's no chance of it clobbering your hand written files)

• Do you want me to add tests? If so, I'd probably start by refactoring the code to extract out a data frame (data table?) that contains the po path, mo_path, and language.

[hadley]

Renamed to po_compile(); that feels at least a little better to me.

[MichaelChirico]

Is my scoped use of roxygen2 ok? (roxygen2 is designed to work like this so there's no chance of it clobbering your hand written files)

Yep that's perfect. I was just wondering (and hoping) if roxygen could work for incrementally migrating like that.

[codecov-commenter]

Codecov Report

Merging #234 (eff4ee5) into master (583db64) will decrease coverage by 0.56%.
The diff coverage is 77.14%.

@@            Coverage Diff             @@
##           master     #234      +/-   ##
==========================================
- Coverage   98.81%   98.24%   -0.57%     
==========================================
  Files          17       18       +1     
  Lines        1351     1371      +20     
==========================================
+ Hits         1335     1347      +12     
- Misses         16       24       +8     

Impacted Files	Coverage Δ
R/po_compile.R	72.41% <72.41%> (ø)
R/msgmerge.R	76.66% <100.00%> (-6.27%)	⬇️
R/translate_package.R	100.00% <100.00%> (ø)
R/utils.R	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 583db64...eff4ee5. Read the comment docs.

[MichaelChirico]

Tests would be ideal. Caveat that the testthat setup for the package is a bit messy, it could probably go for some improvement... don't get stuck on that, would rather merge first & do tests as a follow-up if that's becoming a blocker.

[MichaelChirico]

Added you as a collaborator to facilitate:

https://github.com/MichaelChirico/potools/invitations

[hadley]

I'll work on adding some tests too

[hadley]

Ok, I think this should be good to go for now, assuming you're ok with a withr dep for the tests. I've only added one test for now, since testing compilation requires some easy way to create a .po file, which run_msginit() provides in #234.

[MichaelChirico]

withr dep already comes with testthat so it's not really a new dep; anyway, I am not generally averse to new deps for potools, so don't hesitate to incorporate anything you think will be a valuable inclusion.

[MichaelChirico]

(not sure how to reply to a comment on a thread in "outdated" code so cc-ing to PR thread)

Why not? Because tools:::en_quote() doesn't work on a non-unicode locale?

I am not sure... my guess is to avoid encoding problems. Not sure if it's just to avoid the headache or if there's a deeper issue. The wording in the R docs is a bit vague (from ?tools::update_pkg_po):

In a UTF-8 locale, a ‘translation’ ‘[email protected]’ is created with UTF-8 directional quotes, compiled and installed under ‘inst/po’.

There's also this page from the R site:

https://developer.r-project.org/Translations30.html

If you are working with a copy of the R sources you can do

cd R_build_dir/po
make update-pkg-po update-RGui

(On Windows, use make -f Makefile.win: it skips the en@quot translations.) This will update the translations for all languages, check them with tools::checkPoFiles, compile all those which pass the checks and install the translations in the translations package. The latter can then be copied to an R installation to test the new translations.

This can be done on a per-package basis via tools::update_pkg_po. (This does work on Windows but skips the en@quot translations when not in a UTF-8 locale.)

And here's how it happens in tools::update_pkg_po():

https://github.com/wch/r-source/blob/430360aa394b02046dd7175c0fc55e347e947f80/src/library/tools/R/translations.R#L150-L164

i.e., "in a UTF-8 locale" means if (l10n_info()[["UTF-8"]]) { ... }

[hadley]

I'm thinking about backing off on the idea of supporting tr_n() — tr_() has a reasonable advantage over gettext() in terms of key presses, and can gain some other useful functions as we've discussed above. But tr_n() is going to be rarely used and has few advantages just using the base ngettext().

[MichaelChirico]

for interface consistency you might just duplicate ngettext (tr_n = ngettext)

[hadley]

Should be good to merge now; definitely needs more tests but I think they'll be easier to add as the other pieces start to fall in place.

[MichaelChirico]

Thanks a bunch Hadley! Don't let me forget to add you as a co-author (running out of steam for the night)

[hadley]

I'm happy to add myself. Do you mind if I also switch to Authors@R?

[MichaelChirico]

yep, the only reason I didn't just go ahead & add you is I wanted to switch