feat(l10n): extract strings, add docs, load locales on demand#452
feat(l10n): extract strings, add docs, load locales on demand#452
Conversation
|
86aeb70 was deployed to: https://fred-pr452.review.mdn.allizom.net/ |
|
Converted to draft, as it has merge conflicts. |
bcolsson
left a comment
bcolsson
left a comment
There was a problem hiding this comment.
I understand that more work is happening around localization at some point in the future, but I noticed some major issues that could cause headaches down the road, so adding comments. I'd suggest making changes before asking localizers to translate these. (If the expectation is to use Pontoon, then the complicated Fluent logic wouldn't be supported well either.)
| footer-copyright = Portions of this content are ©1998–{ $year } by individual mozilla.org contributors. Content available under <a data-l10n-name="cc">a Creative Commons license</a>. | ||
| search-modal-site-search = Site search for <em>{ $query }</em> | ||
| site-search-search-stats = Found { $results } documents. | ||
| site-search-suggestion-matches = |
There was a problem hiding this comment.
While this is technically valid fluent it's best to avoid this pattern. This should be two strings - otherwise you can't guarantee that all patterns will have translations (reference).
Typically you want to remove the logic from the strings themselves, so it's preferable to have a -greaterthan and a -equals string then have the logic happening outside of fluent to choose the correct string.
| compat-support-flags = | ||
| { NUMBER($has_added) -> | ||
| [one] From version { $version_added } | ||
| *[other] { "" } | ||
| }{ $has_last -> | ||
| [one] | ||
| { NUMBER($has_added) -> | ||
| *[zero] Until { $versionLast } users | ||
| [one] { " " }until { $versionLast } users | ||
| } | ||
| *[zero] | ||
| { NUMBER($has_added) -> | ||
| *[zero] Users | ||
| [one] { " " }users | ||
| } | ||
| } | ||
| { " " }must explicitly set the <code data-l10n-name="name">{ $flag_name }</code>{ " " } | ||
| { $flag_type -> | ||
| *[preference] preference | ||
| [runtime_flag] runtime flag | ||
| }{ NUMBER($has_value) -> | ||
| [one] { " " }to <code data-l10n-name="value">{ $flag_value }</code> | ||
| *[other] { "" } | ||
| }{ "." } | ||
| { $flag_type -> | ||
| [preference] To change preferences in { $browser_name }, visit { $browser_pref_url }. | ||
| *[other] { "" } | ||
| } |
There was a problem hiding this comment.
This is untranslatable. Even in English I barely understand what a final string would look like, so compounding translations with different word order, you'll have a very difficult time to get something comprehensible out of this.
This should be split into multiple strings. E.g.
string-a = From version { version }
string-b = From version { version } until {version} users
ETc.
| compat-experimental = Experimental | ||
| compat-nonstandard = Non-standard | ||
| compat-no = No | ||
| compat-support-full = Full support |
There was a problem hiding this comment.
I'm finding this and multiple other strings with the exact same ID. You'll need to find a way to avoid duplication.
| compat-legend-preview = In development. Supported in a pre-release version. | ||
| compat-legend-no = { compat-support-no } | ||
| compat-legend-unknown = Compatibility unknown | ||
| compat-legend-experimental = { compat-experimental }. Expect behavior to change in the future. |
There was a problem hiding this comment.
I sort of understand why you want to use a reference here and below, but there is the potential for localization issues here. It'd be simpler for localizers to just have the word instead of a message reference.
caugner
left a comment
caugner
left a comment
There was a problem hiding this comment.
LGTM, just one question: When new strings are added inline, are these automatically added to the Fluent file on pre-commit?
caugner
changed the title
I wanted to push a commit and say "they are now!" - but unfortunately it's a little more complicated. I've added a pre-push hook lint run to catch any errors before CI, but in terms of running the extractor on the pre-commit hook, if you have files that are unstaged but which have strings to be scraped, they'll be pulled into the wrong commit. We should be able to work around that by passing a list of files to the extractor to act on - but that's something I think makes sense for a follow up PR at this point. |
| - name: lint l10n strings | ||
| run: npm run l10n -- --lint |
There was a problem hiding this comment.
Given how fast this runs, should we not also run it on pre-commit?
% time npm run l10n -- --lint
> @mdn/fred@2.2.1 l10n
> node l10n/cli.js --lint
npm run l10n -- --lint 0.64s user 0.07s system 113% cpu 0.627 total
There was a problem hiding this comment.
The glob option supports multiple values, so we could run it only when relevant files were changed.
There was a problem hiding this comment.
The failure case is something like:
- add string to components/a.js
- add string to components/b.js
- stage a.js
- commit
- new strings from both a and b are in template.ftl
5 participants
Gets things to a stage where all strings are - in theory - l10n-able. Next steps will be improving the l10n experience, and further optimising which strings we ship to the client.
The added README is a good source for some of what's going on here.