#2126: Fix language selection drop down - #2234
Open
oanding-blrng wants to merge 3 commits into
Open
Conversation
… property files for i18n. Extend testGetInstanceWithLocale() test method to inculde additional test cases. Added ticket to the CHANGELOG.adoc.
8 tasks
oanding-blrng
marked this pull request as ready for review
July 29, 2026 14:03
hohwille
approved these changes
Jul 30, 2026
hohwille
left a comment
Member
There was a problem hiding this comment.
@oanding-blrng thanks for this PR with your new approach. Looks great to me👍
I only have some rather cosmetic remarks before we will merge.
| NlsService service = new NlsService(Locale.ENGLISH); | ||
| @ParameterizedTest | ||
| @MethodSource("testGetInstanceWithLocaleProvider") | ||
| public void testGetInstanceWithLocale(Locale systemLocale, Locale providedLocal, Locale expectedServiceLocal, String expectedString) { |
Member
There was a problem hiding this comment.
2nd and 3rd param seem to lack a trailing e.
Comment on lines
+77
to
+83
| arguments(Locale.GERMANY, Locale.ENGLISH, Locale.ENGLISH, "English (en)"), | ||
| arguments(Locale.GERMANY, Locale.GERMAN, Locale.GERMAN, "Deutsch (de)"), | ||
| arguments(Locale.GERMANY, null, Locale.GERMANY, "Deutsch (de)"), | ||
| arguments(Locale.UK, Locale.ENGLISH, Locale.ENGLISH, "English (en)"), | ||
| arguments(Locale.UK, Locale.GERMAN, Locale.GERMAN, "Deutsch (de)"), | ||
| arguments(Locale.UK, null, Locale.UK, "English (en)"), | ||
| arguments(Locale.FRANCE, null, Locale.FRANCE, "English (en)") |
Member
There was a problem hiding this comment.
Wouldn't it make sense to use this test to also test that our NLS is working in a way it can actually translate so we also test that Deutsch (de) becomes German (de) if the locale language is English and vice versa for German we would get Englisch (en) instead of English (en)?
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR fixes #2126
Identification of root cause:
In the current setup of property files for localization, the method
IDEasy/gui/src/main/java/com/devonfw/ide/gui/nls/NlsService.java
Lines 242 to 245 in ec264ff
Locale.ENGLISHas an argument. This leads to a list with only one entry for the language selection dropdown, because for both locals (en and de) the German ResourceBundle gets returned.This behaviour is "normal" in the sense, that ResourceBundle first tries to find a properties file for
en_UK, can't find one and goes up the chain forenand after that the root file. But before the root is being used, ResourceBundle also tries the system locale (for me this would returnde_DE). So ResourceBundle would search forde_DE, and after thatde. Because of this "missing link" in the chain for English, this results in the observed behaviour mentioned in the issue #2126. Additional information are in #2221.Implemented changes:
messages_en.properties, so the ResourceBundle can find the correct bundle and fallbacks automatically for the keys to the root properties, because no keys are being overwrittentestGetInstanceWithLocale(..)test method inNlsServiceTest.javato cover additional machine setupsTesting instructions
Please add concise, understandable instructions on how a reviewer can test/verify the functionality of your contribution here:
mvn test -pl gui. The tests mentioned in the issue only german is listed in language selection dropdown #2126 should pass nowRetest:
ide gui. The language selection dropdown should now list English and GermanChecklist for this PR
Make sure everything is checked before merging this PR. For further info please also see
our DoD.
mvn clean testlocally all tests pass and build is successful#«issue-id»: «brief summary»(e.g.#921: fixed setup.bat). If no issue ID exists, title only.In Progressand assigned to you or there is no issue (might happen for very small PRs)with
internal