fix(localize): translation file parser selection #35793
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
petebacondarwin
added
type: bug/fix
effort2: days
area: i18n
petebacondarwin
force-pushed
the
i18n-translation-file-selection
branch
from
e7131b9 to
9dcc985
Compare
mhevery
approved these changes
Mar 5, 2020
...rc/tools/test/translate/translation_files/translation_parsers/xtb_translation_parser_spec.ts
Outdated
Show resolved
Hide resolved
...tools/test/translate/translation_files/translation_parsers/xliff1_translation_parser_spec.ts
Outdated
Show resolved
Hide resolved
clydin
approved these changes
Mar 5, 2020
...localize/src/tools/src/translate/translation_files/translation_parsers/translation_parser.ts
Show resolved
Hide resolved
petebacondarwin
force-pushed
the
i18n-translation-file-selection
branch
from
d5af5ed to
950ec6b
Compare
petebacondarwin
added
target: patch
AndrewKushnir
approved these changes
Mar 6, 2020
|
@petebacondarwin quick question: when can we remove deprecated APIs (after the corresponding release on CLI side)? Could you please create a ticket on that, so that we clean them up when possible? Thank you. |
|
@AndrewKushnir - regarding the deprecated overload. Since this is not actually a public API it does not have to follow our official deprecation rules. I just added the deprecation tag to remind the CLI to change their implementation. Once they have done so, we should be able to safely remove this overload. Tracking at FW-1979 |
This modifies the internal (but shared with CLI) API for loading/parsing translation files. Now the parsers will return a new `Diagnostics` object along with any translations and locale extracted from the file. It is up to the caller to decide what to do about this, if there are errors it is suggested that an error is thrown, which is what the `TranslationLoader` class does.
This enables complex work to be done in `TranslationParser.canParse()` without duplicating the work in `TranslationParser.parse()`.
…files Previously, the `Xliff1TranslationParser` only matched files that had a narrow choice of extensions (e.g. `xlf`) and also relied upon a regular expression match of an optional XML namespace directive. This commit relaxes the requirement on both of these and, instead, relies upon parsing the file into XML and identifying an element of the form `<xliff version="1.2">` which is the minimal requirement for such files.
…files Previously, the `Xliff2TranslationParser` only matched files that had a narrow choice of extensions (e.g. `xlf`) and also relied upon a regular expression match of an optional XML namespace directive. This commit relaxes the requirement on both of these and, instead, relies upon parsing the file into XML and identifying an element of the form `<xliff version="2.0">` which is the minimal requirement for such files.
This commit improves the `canParse()` method to check that the file is valid XML and has the expected root node. Previously it was relying upon a regular expression to do this.
petebacondarwin
force-pushed
the
i18n-translation-file-selection
branch
from
950ec6b to
4f2df99
Compare
matsko
pushed a commit
that referenced
this pull request
Mar 9, 2020
…5793) This modifies the internal (but shared with CLI) API for loading/parsing translation files. Now the parsers will return a new `Diagnostics` object along with any translations and locale extracted from the file. It is up to the caller to decide what to do about this, if there are errors it is suggested that an error is thrown, which is what the `TranslationLoader` class does. PR Close #35793
matsko
pushed a commit
that referenced
this pull request
Mar 9, 2020
This enables complex work to be done in `TranslationParser.canParse()` without duplicating the work in `TranslationParser.parse()`. PR Close #35793
matsko
pushed a commit
that referenced
this pull request
Mar 9, 2020
…files (#35793) Previously, the `Xliff1TranslationParser` only matched files that had a narrow choice of extensions (e.g. `xlf`) and also relied upon a regular expression match of an optional XML namespace directive. This commit relaxes the requirement on both of these and, instead, relies upon parsing the file into XML and identifying an element of the form `<xliff version="1.2">` which is the minimal requirement for such files. PR Close #35793
matsko
pushed a commit
that referenced
this pull request
Mar 9, 2020
…files (#35793) Previously, the `Xliff2TranslationParser` only matched files that had a narrow choice of extensions (e.g. `xlf`) and also relied upon a regular expression match of an optional XML namespace directive. This commit relaxes the requirement on both of these and, instead, relies upon parsing the file into XML and identifying an element of the form `<xliff version="2.0">` which is the minimal requirement for such files. PR Close #35793
matsko
pushed a commit
that referenced
this pull request
Mar 9, 2020
This enables complex work to be done in `TranslationParser.canParse()` without duplicating the work in `TranslationParser.parse()`. PR Close #35793
matsko
pushed a commit
that referenced
this pull request
Mar 9, 2020
…files (#35793) Previously, the `Xliff1TranslationParser` only matched files that had a narrow choice of extensions (e.g. `xlf`) and also relied upon a regular expression match of an optional XML namespace directive. This commit relaxes the requirement on both of these and, instead, relies upon parsing the file into XML and identifying an element of the form `<xliff version="1.2">` which is the minimal requirement for such files. PR Close #35793
matsko
pushed a commit
that referenced
this pull request
Mar 9, 2020
…files (#35793) Previously, the `Xliff2TranslationParser` only matched files that had a narrow choice of extensions (e.g. `xlf`) and also relied upon a regular expression match of an optional XML namespace directive. This commit relaxes the requirement on both of these and, instead, relies upon parsing the file into XML and identifying an element of the form `<xliff version="2.0">` which is the minimal requirement for such files. PR Close #35793
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
These changes should fix #35525, and should be compatible with current CLI integration code (@clydin to confirm), although the
hintbased approach with accompanied change to remove the need to passdiagnosticsto constructors should also improve the performance and maintainability.This should be completely backward compatible with previous versions.
FW-1888