Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: PEX-20271 replacements context #140

Merged
merged 4 commits into from
Sep 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,32 @@ Doing that manually could be laborious, that's why `i18n-extract` supports a pat
}
```

Replacements json file should be a simple object, where the keys are the original translation strings and the values are the new ones.
Replacements json file should be an object where the keys are translation context names.
Values of each context are objects for the original translation strings and the values are the new ones.

```json
{
"some context" : {
"Old translation": "New translation",
"Old translation2": "New translation2"
}
}
```

If the translation does not have the context, then it should be stored in the `""` (empty string) field.

```json
{
"" : {
"Old translation": "New translation without context",
},
"some context": {
"Old translation": "New translation with some context",
}
}
```


Having the original JS code:
```javascript
// translators: comment 1
Expand Down
2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export default translate
* locale
* @param configObj A hashmap with keys `default` (default locale) and `map` (mapping of locales to
* other locales)
* @param replacements A hashmap with translation context keys and translation hashmaps as their values
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question With this change, should we bump the version in package.json?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasR I think we can bump the minor version when we publish it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would opt for minor as well

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@star4beam After reviewing I think this could be a semver major change since the way replacement json needs to be configured has changed thus making it not backwards compatible. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Matwog Hmm, ok!

*
**/
export function init(getLangLoaderFn, configObj = {}, replacements) {
getLangLoader = getLangLoaderFn
Expand Down
51 changes: 30 additions & 21 deletions src/scripts/babel-gettext-extractor/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,30 +224,39 @@ export default function plugin() {
}

context[translate.msgid] = translate
if (
replacements &&
(typeof replacements[translate.msgid] === 'string' ||
typeof replacements[translate.msgid_plural] === 'string')
) {
const newTranslate = {
...translate,
comments: {
...translate.comments,
extracted: (`REPLACEMENT for "${translate.msgid}"`).trim(),
},
}

newTranslate.msgid =
typeof replacements[translate.msgid] === 'string'
? replacements[newTranslate.msgid]
: newTranslate.msgid
if (replacements) {
const contextName = translate.msgctxt
const contextReplacements = replacements[contextName]
if (
contextReplacements &&
(typeof contextReplacements[translate.msgid] === 'string' ||
typeof contextReplacements[translate.msgid_plural] === 'string')
) {
const newTranslate = {
...translate,
comments: {
...translate.comments,
extracted:
`REPLACEMENT for "${translate.msgid}"`.trim() +
(translate.msgctxt ? `, context: "${translate.msgctxt}"` : ''),
},
}

newTranslate.msgid =
typeof contextReplacements[translate.msgid] === 'string'
? contextReplacements[newTranslate.msgid]
: newTranslate.msgid

newTranslate.msgid_plural =
typeof replacements[translate.msgid_plural] === 'string'
? replacements[newTranslate.msgid_plural]
: newTranslate.msgid_plural
newTranslate.msgid_plural =
typeof contextReplacements[translate.msgid_plural] === 'string'
? contextReplacements[newTranslate.msgid_plural]
: newTranslate.msgid_plural

context[newTranslate.msgid] = newTranslate
newTranslate.msgctxt = translate.msgctxt

context[newTranslate.msgid] = newTranslate
}
}

const output = gettextParser.po.compile(data)
Expand Down
37 changes: 23 additions & 14 deletions src/translate.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,12 @@ const defaultOptions = {
markdown: false,
}

const NO_CONTEXT = ""

export default (singleton) => {
return function translate(text, plural, options) {
// singleton.replacements optionally contains the translation replacements for the first two string arguments
if (singleton.replacements) {

if (typeof text === 'string' && singleton.replacements[text]) {
text = singleton.replacements[text]
}

if (typeof plural === 'string' && singleton.replacements[plural]) {
plural = singleton.replacements[plural]
}
}


// singleton.messages contains the translation messages for the currently active languae
// format: singular key -> [ plural key, singular translations, plural translation ]
Expand All @@ -33,10 +25,27 @@ export default (singleton) => {
finalOptions = {
...defaultOptions,
...finalOptions,
context:
finalOptions && finalOptions.context
? `${finalOptions.context}\u0004`
: '',
}

const context = finalOptions && finalOptions.context
finalOptions.context = context ? `${finalOptions.context}\u0004` : ''

const replacementContextKey = context || NO_CONTEXT
// singleton.replacements optionally contains the translation replacements for the first two string arguments by context
const { replacements } = singleton

if (replacements && replacements[replacementContextKey]) {
const replacementsContext = replacements[replacementContextKey]

if (replacementsContext) {
if (isString(text) && replacementsContext[text]) {
text = replacementsContext[text]
}

if (isString(finalPlural) && replacementsContext[finalPlural]) {
finalPlural = replacementsContext[finalPlural]
}
}
}

const [translatedSingular, translatedPlural] = (
Expand Down
5 changes: 5 additions & 0 deletions test/fixtures/replacements/index.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
i18n('Needs replacement')

i18n('Needs replacement', { context: 'someContext' })

i18n('Needs replacement plural 1', 'Needs replacement plural 2', { context: 'anotherContext' })

i18n('No replacement is needed')

12 changes: 11 additions & 1 deletion test/fixtures/replacements/replacements.json
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@

{
"Needs replacement": "A replacement for 'Needs replacement'"
"": {
"Needs replacement": "A replacement for 'Needs replacement'"
},
"someContext": {
"Needs replacement": "A replacement for 'Needs replacement'"
},
"anotherContext": {
"Needs replacement with context plural 1": "A replacement for 'Needs replacement with context plural 1'",
"Needs replacement with context plural 2": "A replacement for 'Needs replacement with context plural 2'"
}
}
45 changes: 31 additions & 14 deletions test/specs/i18n.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,38 +36,55 @@ describe('i18n', () => {
})

describe('replacements', () => {
it('should use the corresponding trasnlation from the replacements object en_US', () => {
beforeEach(reset)
it('should use the corresponding translation by context de_DE', () => {
setLocale('de_DE')
const replacements = { someContext: { 'Old translation': 'New translation' } }
return init(getLangLoader, config, replacements).then(() => {
expect(i18n('Old translation', { context: 'someContext' })).toBe('Neue Übersetzung someContext')
})
})

it('should use the corresponding translation from the replacements object en_US', () => {
setLocale('en_US')
const replacements = {"Old translation": "New translation"}
return init(getLangLoader, {}, replacements).then(() => {
const replacements = { "": { "Old translation": "New translation" } }
return init(getLangLoader, config, replacements).then(() => {
expect(i18n('Old translation')).toBe('New translation')
})
})

it('should use the corresponding trasnlation from the replacements object de_DE', () => {
it('should use the corresponding translation from the replacements object de_DE', () => {
setLocale('de_DE')
const replacements = {"Old translation": "New translation"}
return init(getLangLoader, {}, replacements).then(() => {
const replacements = { "": { "Old translation": "New translation" } }
return init(getLangLoader, config, replacements).then(() => {
expect(i18n('Old translation')).toBe('Neue Übersetzung')
})
})

it('should not use the replaced translation when there are no match en_US)', () => {
setLocale('en_US')
const replacements = {"Foobar": "New translation"}
return init(getLangLoader, {}, replacements).then(() => {
const replacements = { "": { "Foobar": "New translation" } }
return init(getLangLoader, config, replacements).then(() => {
expect(i18n('Old translation')).toBe('Old translation')
})
})

it('should not use the replaced translation when there are no match de_DE)', () => {
setLocale('de_DE')
const replacements = {"Foobar": "New translation"}
return init(getLangLoader, {}, replacements).then(() => {
const replacements = { "": { "Foobar": "New translation" } }
return init(getLangLoader, config, replacements).then(() => {
expect(i18n('Old translation')).toBe('Alte Übersetzung')
})
})
})

it('should use the corresponding translation by context en_US', () => {
setLocale('en_US')
const replacements = { someContext: { "Old translation": "New translation someContext" } }
return init(getLangLoader, config, replacements).then(() => {
expect(i18n('Old translation', { context: 'someContext' })).toBe('New translation someContext')
})
})
})

describe('#translate', () => {
it('should return a plain string whenever possible', () => {
Expand Down
8 changes: 8 additions & 0 deletions test/specs/locales/de_DE.po
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,11 @@ msgstr "Alte Übersetzung"

msgid "New translation"
msgstr "Neue Übersetzung"

msgctxt "someContext"
msgid "Old translation"
msgstr "Alte Übersetzung"

msgctxt "someContext"
msgid "New translation"
msgstr "Neue Übersetzung someContext"
8 changes: 8 additions & 0 deletions test/specs/locales/en_US.po
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,11 @@ msgstr "Old translation"

msgid "New translation"
msgstr "New translation"

msgctxt "someContext"
msgid "Old translation"
msgstr "Old translation"

msgctxt "someContext"
msgid "New translation"
msgstr "New translation someContext"
22 changes: 21 additions & 1 deletion test/specs/scripts/extract.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('extract', () => {
expect(messages).not.toContain('msgid "Not in the result"')
})
})

describe('replacements', () => {
const replacementsDir = `${fixtureDir}/replacements`

Expand All @@ -63,6 +63,26 @@ describe('extract', () => {
expect(messages).toContain(`msgid "A replacement for 'Needs replacement'`)
expect(messages).toContain('#. REPLACEMENT for "Needs replacement"')
expect(messages).toContain('msgid "No replacement is needed"')

// context
expect(messages).toContain(
[
`#: fixtures/replacements/index.js:3`,
`#. REPLACEMENT for "Needs replacement", context: "someContext"`,
`msgctxt "someContext"`,
`msgid "A replacement for 'Needs replacement'"`,
].join('\n')
)

// context plural
expect(messages).toContain(
[
`#: fixtures/replacements/index.js:5`,
`msgctxt "anotherContext"`,
`msgid "Needs replacement plural 1"`,
`msgid_plural "Needs replacement plural 2"`,
].join('\n')
)
})
})

Expand Down