-
-
Notifications
You must be signed in to change notification settings - Fork 111
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
fix: o.o.c.segmentation.SRX to load conf and save srx in more robust way and remove warning message #1159
Conversation
f6951c9
to
c0defb6
Compare
There are two test failures.
|
176fa88
to
16ab78e
Compare
cda89d4
to
6cae905
Compare
- reafactor SRXTest class - Add germany locale conf file built from OmegaT 5.4.0 as test data - refactor SRX class to help testing - Load resource bundle in specified test locale Signed-off-by: Hiroshi Miura <[email protected]>
- Harden the save method to robust for localized language name. - Even when MapRule has a localized language code, it detects language from a language pattern and write standard name. Signed-off-by: Hiroshi Miura <[email protected]>
- rulename for text in Germany was changed in v5.5 - when reading "segmentation.conf" generated before v5.4, migration is failed. - Add workaround to detect ancient rulename Signed-off-by: Hiroshi Miura <[email protected]>
6cae905
to
a69a418
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are several points to be improved. I leave some comments for the reviewers.
/** Language Name */ | ||
private String languageCode; | ||
|
||
public MapRule(Languagemap languagemap, List<Rule> rules) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the ctor with Languagemap
as argument. It reduces a dependency of classes. A retrieval of fields of languagemap
is done in the caller.
public MapRule(String language, String pattern, List<Rule> rules) { | ||
this.setLanguage(language); | ||
String code = LanguageCodes.getLanguageCodeByPattern(pattern); | ||
this.setLanguage(code != null ? code : language); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We got language from the pattern, if it got "EN.*" as pattern we can know it is for English.
Use non localized message for debug level
- Update LanguageCode.getLanguageCodeByName - add null check at first - move a migration heuristics code from MapRule - Update MapRule javadoc descriptions Signed-off-by: Hiroshi Miura <[email protected]>
Signed-off-by: Hiroshi Miura <[email protected]>
public static class SRXMigrateJaTest { | ||
|
||
@org.junit.Rule | ||
public final LocaleRule localeRule = new LocaleRule(new Locale("ja")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a hack to change System Locale to run unit test cases.
Because LocaleRule
utility class is not related to the OmegaT functions, please ignore it if you don't know.
Signed-off-by: Hiroshi Miura <[email protected]>
Pull request type
Which ticket is resolved?
What does this PR change?
Other information
#1158