Add changes command #55
Conversation
There was a problem hiding this comment.
Looks pretty good! I added small comments to illustrate, and made commit some changes to the PR.
It is mainly based around some trickier parts of the gettext format (message context), which was previously ignored. I ran it against a large app of ours, and it seems to work pretty good now! Thank you for the PR!
lib/commands/changes.js
Outdated
| * @param {*} options | ||
| */ | ||
| _compareWithPoFile(options) { | ||
| let poFile = `${options.extractTo}/${options.language}.po`; |
There was a problem hiding this comment.
I would use the .pot file here (e.g options.potName --> messages.pot by default), as that might be different from the en.po, and can be considered the baseline :)
lib/commands/changes.js
Outdated
| if (this.fileExists_(poFile)) { | ||
| let poData = fs.readFileSync(poFile); | ||
| let jsonData = parser.po.parse(poData); | ||
| let existingTranslations = jsonData.translations['']; |
There was a problem hiding this comment.
The '' property are the default translations without a context. There can also be translations with a context, which would be namespaced by context. e.g.:
translations: {
'': {...},
'my context 1': {...}
}
}
So we should probably loop over this, e.g. Object.keys(jsonData.translations).forEach((namespace) => {...}).
|
Note sure why the tests are not passing, linting looks fine locally... I'll investigate! |
With using the syntax
ember l10n:changesits possible to check if new Message Items are added in the Application in order to review them before they get manifested via
ember l10n:extractThe CL Output could be improved and it needs to be checked if the comparison check works for all possible scenarios.