On Fri, 10 Dec 2021 00:02:31 GMT, Daniel Le <duke@openjdk.java.net> wrote:
Locale.filterTags methods ignore actual weight when matching "*" (as if it is 1) because LocaleMatcher.{filterBasic,filterExtended} do so.
Fix the bug and add regression test cases for it as well as existing behavior.
Thanks for the fix. Looks good overall. Some minor comments follow. Also for the test case, - add the bug id `8276302` to the `@bug` tag. - modify the copyright year to 2021. src/java.base/share/classes/sun/util/locale/LocaleMatcher.java line 131:
129: 130: if (!caseInsensitiveMatch(list, lowerCaseTag) 131: && !shouldIgnoreFilterBasicMatch(zeroRanges, lowerCaseTag)) {
Is there any reason to flip the evaluation order of the `if` statement? src/java.base/share/classes/sun/util/locale/LocaleMatcher.java line 157:
155: * returning all the tags, remove those which are matching the range with 156: * quality weight q=0. 157: */
Please keep the comments in the modified code (except the `*` part). Although it's OK to remove this method as it eliminates extra `ArrayList` allocation, please keep the comments in the modified code (except the `*` part). src/java.base/share/classes/sun/util/locale/LocaleMatcher.java line 161:
159: List<LanguageRange> zeroRange, Collection<String> tags) { 160: if (zeroRange.isEmpty()) { 161: tags = removeDuplicates(tags);
Can `removeDuplicates()` now be removed? There seems to be no usage. src/java.base/share/classes/sun/util/locale/LocaleMatcher.java line 167:
165: List<String> matchingTags = new ArrayList<>(); 166: for (String tag : tags) { 167: // change to lowercase for case-insensitive matching
Applies to this comment. src/java.base/share/classes/sun/util/locale/LocaleMatcher.java line 171:
169: if (!shouldIgnoreFilterBasicMatch(zeroRange, lowerCaseTag) 170: && !caseInsensitiveMatch(matchingTags, lowerCaseTag)) { 171: matchingTags.add(tag); // preserving the case of the input tag
And this comment, too. ------------- PR: https://git.openjdk.java.net/jdk/pull/6789