Strip indent from extract and gettext #2086
Conversation
tofumatt
left a comment
tofumatt
left a comment
There was a problem hiding this comment.
Looks good, just a few nits.
I think we can also remove the dedent preprocessing we had in the babel config now, right?
src/core/i18n/utils.js
Outdated
| } | ||
|
|
||
| // Functionality based on oneLine form declandewet/common-tags https://goo.gl/4PzaJI | ||
| // If this function is altered muffinresearch/babel-gettext-extractor also needs |
There was a problem hiding this comment.
Nitpick but I think you should link to the URL of the repo here, not just the name. Makes it easier to scan and a bit more obvious (I did a double-take when reading it!)
src/core/i18n/utils.js
Outdated
| // Functionality based on oneLine form declandewet/common-tags https://goo.gl/4PzaJI | ||
| // If this function is altered muffinresearch/babel-gettext-extractor also needs | ||
| // to be updated. | ||
| function oneLineTranslationString(str) { |
There was a problem hiding this comment.
I'm guessing this was copy/pasted and we can't use oneLine directly.
Can we change the variable name to string or i18nString? I'm just wary of no vowel variables.
| <p> | ||
| {i18n.gettext(oneLine` | ||
| {i18n.gettext(` | ||
| Sorry, but we can't find anything at the URL you entered.`)} |
There was a problem hiding this comment.
Nitpick but this looks like it didn't even need oneLine. Oops!
There was a problem hiding this comment.
Heh, yeah I'll fix.
Yeah it's being used in a few other places, and it still needs to be done for disco too. |
2 participants
Fixes mozilla/addons#10251
I've now updated the fork of babel-gettext-extractor [1] after a bit of a clean-up and overhaul adding more tests and fixing up the lint.
This branch updates to the rev of that lib with the new features this now supports removing leading indents from extracted values.
This patch also wraps our Jed instance functions to support stripping the same indent from keys because otherwise the lookups would fail. This means from now on any multiline translations should use untagged template literals and not use dedent or oneLine.
I'll also add a commit to this to update the docs so it's clear and we should also look to add a eslint rule to check for tagged template literals as args to any Jed gettext methods.
Lastly to ensure tests see the actual Jed output this changes the FakeJed to be a spy wrapped real Jed. Otherwise several tests look at translation strings broke due to leading whitespace in the text.
[1] https://github.com/muffinresearch/babel-gettext-extractor/commits/master
Note
After landing we'll need to extract and merge both amo and the disco pane so that the new strings currently missing can be translated.