WIP: Add Support for Translations

[joelnitta]

As mentioned here, this PR does not completely solve #205 because there are many site elements embedded in mustache templates (not R code), including some outside of {sandpaper} like the {varnish} HTML templates. But it is a start.

[joelnitta]

I encountered two errors during testing, but these also occur on main from where I started the branch (0d87a88) when testing locally (MacOS), so I don't think they are related to this PR.

Error (test-manage_deps.R:154:3): pin_version() will use_specific versions

Error (test-manage_deps.R:210:3): update_cache() will update old package versions

[zkamvar]

/document

[zkamvar]

Thank you, Joel! I went over this PR with @froggleston and we were pleasantly surprised at how straightforward this part of the solution was! I have made some suggestions to remove code from {pkgdown} that's not relevant to the lesson context.

I tested this out with The Workbench documentation and found that it does indeed work with the titles you've given!

One of the things I am wondering about: Do you have an idea of how we would update the translation files? I know you used {potools} to process the translations, but I do not know the specific steps you used. If we know this, we can write a vignette that describes how to update an existing translation or add new translations. I just realised that you mentioned in Slack that @maelle already wrote it https://masalmon.eu/2023/10/06/potools-mwe/!

Next Steps

As you correctly identified, this fixes the titles of the pages and shows that this can work in that context. The next steps are to identify and setup translations for the menu elements, which are defined in {varnish}.

based on #205 (comment), hard-coded English text needs to be replaced with mustache templating:

For example, this link to key points from the header has "Key Points" hard-coded:

<a class="nav-link" href="{{#site}}{{root}}{{/site}}key-points.html">
Key Points
</a>

To use translations, we should do the following:

1. in {sandpaper} add a function that will add $translate field to the {pkgdown} object in build_site() that will include a translation for keypoints:

   add_varnish_translations <- function(pkg) {
     pkg$translate <- list(
       keypoints = tr_("Key Points")
     )
     return(pkg)
   }

2. in {varnish}, use the mustache template to replace the hard-coded text:

   <a class="nav-link" href="{{#site}}{{root}}{{/site}}key-points.html">
   {{#translate}}{{keypoints}}{{/translate}}
   </a>

There is also the matter of addressing how to handle the Lua filter, but we can burn that bridge when we get there:

sandpaper/inst/rmarkdown/lua/lesson.lua

Lines 75 to 77 in 0d87a88

	pandoc.RawBlock("html", "<h3 class='card-title'>Questions</h3>"))
	table.insert(objectives_div.content,
	pandoc.RawBlock("html", "<h3 class='card-title'>Objectives</h3>"))

Please note: I will be out of the office by the time you read this until 2023-11-27 (the 28th in Japan) due to the US holiday week.

[joelnitta]

Thanks @zkamvar for the comments! I know you won't be able to get to this for a few days, but posting my notes now so I don't forget.

I added add_varnish_translations() with translations to replace the mustache templates, and inserted a call to this in build_site().

Furthermore, I started to convert some of the mustache templates in {varnish} into elements wrapped in {{#translate}} / {{/translate}} on this branch of my fork of {varnish}. I started with Keypoints and See all in one page.

I tested using the new elements wrapped in {{#translate}} / {{/translate}} locally, but am not yet having success. Instead of the translation getting correctly inserted, I observe that See all in one page just disappears completely, whether setting a non-default language in lang: or not.

View with my changes to {varnish} and {sandpaper} (through de69eae):

View with my changes to {sandpaper} only:

My best guess at this point is that either I am putting the call to add_varnish_translations() in the wrong place, or there is something else causing the {{#translate}} / {{/translate}} labels not to get processed correctly.

[zkamvar]

Thank you for working further on this! I was able to get it to work!

I had a look and I found out what was wrong with the solution I proposed: the data model of {pkgdown} requires the $translate element to be part of supplemental data and not the pkg object. This is best acheived by setting the translations in the learner_globals and instructor_globals objects, which are both initialised when validate_lesson() is run.

That being said, on of my tasks before I leave is to write up a better description of the data flow model (as I attempted in 31876a4) so that this becomes a bit easier to manage in the future.

[zkamvar]

I've gone ahead and grabbed all of the translatable terms I could in {varnish}, by using the following functions:

get_bare_text <- function(path) {
  html <- xml2::read_html(path)
  no_brace <- "not(starts-with(normalize-space(text()), '{'))"
  no_blank <- "not(normalize-space(text())='')"
  no_mathjax <- "not(starts-with(normalize-space(text()), 'MathJax.Hub'))"
  no_docsearch <- "not(starts-with(normalize-space(text()), 'docsearch'))"

  xpath <- sprintf(".//*[%s and %s and %s and %s]", 
    no_brace, no_blank, no_mathjax, no_docsearch)
  nodes <- xml2::xml_find_all(html, xpath)
  unique(trimws(xml2::xml_text(nodes)))
}

get_varnish_text <- function() {
  templates <- system.file("pkgdown", "templates", package = "varnish")
  res <- lapply(list.files(templates, pattern = "*html", full.names = TRUE),
    get_bare_text)
  reslist <- unique(unlist(res, use.names = FALSE))
  resvar <- abbreviate(reslist, 6)
  clipr::write_clip(paste0(resvar, " = _tr(", sQuote(reslist, q = 2), "),"))
}

There were some things I trimmed out and I am realising that we may need to include templating within these translations (e.g. Licensed under {{ license }} by {{ authors }}). The challenge with that is that the templates do not recursively fill, so they will need to be pre-filled in build_html() before being passed to pkgdown::render_html_page().

[zkamvar]

I've gone ahead and implemented the transformation I suggested in #546 (comment)

[joelnitta]

Thanks @Zhian! I can confirm this is now working for me. I am updating the PO file with the new translations, and filling in the {{#translate}} {/translate}} fields on my branch of {varnish}.

As you point out, translating sentences like Licensed under {{ license }} by {{ authors }} is tricky; another reason for this is that word order differs between languages. Japanese in particular, would have the order reversed: {{authors}} grammatically comes first before {{license}}. I am trying to think of ways to make this work... @MichaelChirico has a good discussion of this in the {potools} docs. As mentioned there, it is recommended to translate complete sentences, but then we run into trouble with non-translatable things like HTML links.

[zkamvar]

Thank you for the resources, Joel! I will work on polishing the translatable strings and then I will get back to you.

[zkamvar]

@joelnitta, thank you for your hard work on this. I'm happy to say that I've tested it out with https://swcarpentry-ja.github.io/git-novice/index.html and it appears to be working well!

@carpentries/workbench-maintainers, I believe we can release this on the 13th with carpentries/varnish#105

[joelnitta]

Awesome! Looking forward to the merge/release.

[zkamvar]

I will probably merge it into main tomorrow once I figure out why the macOS {renv} tests are failing.

[joelnitta]

Oops, we missed one more, Show me the solution

Actually, it looks like all of the accordian_titles need to have translations added

https://github.com/joelnitta/sandpaper/blob/edb60507e323fdc408538277b240e54603308450/inst/rmarkdown/lua/lesson.lua#L197-L201

[joelnitta]

Also, https://swcarpentry-ja.github.io/git-novice/ is now building from this PR branch (and {varnish} PR#105), so that can be used for checking missing website element translations (but I hope there aren't any more!)

[zkamvar]

Thank you for catching that, Joel! I have gone ahead and added those translations and updated the po files. Now all that is needed is for your translation and we can merge!

As a side note: It turns out the random macOS error was due to a bug fix in {renv} that introduced another bug 🤷🏼

[MichaelChirico]

Great work, love to see it!!