Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CLDR-17680 fix spurious CheckConsistentCasing and CheckCoverage #3767

Merged
merged 2 commits into from
May 31, 2024
Merged

CLDR-17680 fix spurious CheckConsistentCasing and CheckCoverage #3767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3875f8b
CLDR-17680 fix spurious CheckConsistentCasing and CheckCoverage warnings
srl295 May 30, 2024
1ad437d
CLDR-17680 hide overall errors
srl295 May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion tools/cldr-apps/js/src/views/GeneralInfo.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
Open Dashboard
</button>

<cldr-overall-errors />
<!-- <cldr-overall-errors /> -->
</div>
</template>

Expand Down
16 changes: 8 additions & 8 deletions tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckConsistentCasing.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,19 +64,19 @@ public CheckCLDR handleSetCldrFileToCheck(
} catch (Exception e) {
types = Collections.emptyMap();
}
if ((types == null || types.isEmpty()) && !SpecialLocales.isScratchLocale(locale)) {
possibleErrors.add(
new CheckStatus()
.setCause(this)
.setMainType(CheckStatus.warningType)
.setSubtype(Subtype.incorrectCasing)
.setMessage("Could not load casing info for {0}", locale));
}
} else {
// no casing info - since the types Map is global, and null checks aren't done,
// we are better off with an empty map here
types = Collections.emptyMap();
}
if ((types == null || types.isEmpty()) && !SpecialLocales.isScratchLocale(locale)) {
possibleErrors.add(
new CheckStatus()
.setCause(this)
.setMainType(CheckStatus.warningType)
.setSubtype(Subtype.incorrectCasing)
.setMessage("Could not load casing info for {0}", locale));
}
Comment on lines -72 to -79
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, this warning would fire for Arabic, etc. because the script is not cased, so control would reach line 70. Then types is checked again on this line and the warning fires.

The change moves the warning to line 67, so it fires only if casing is expected but not present.

// types may be null, avoid NPE
hasCasingInfo = (types == null) ? false : types.size() > 0;
return this;
Expand Down
6 changes: 3 additions & 3 deletions tools/cldr-code/src/main/java/org/unicode/cldr/test/CheckCoverage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ public CheckCLDR handleSetCldrFileToCheck(
supplementalData =
SupplementalDataInfo.getInstance(cldrFileToCheck.getSupplementalDirectory());
coverageLevel = CoverageLevel2.getInstance(supplementalData, localeID);
PluralInfo pluralInfo = supplementalData.getPlurals(PluralType.cardinal, localeID);
if (pluralInfo == supplementalData.getPlurals(PluralType.cardinal, LocaleNames.ROOT)
&& !SpecialLocales.isScratchLocale(localeID)) {
PluralInfo pluralInfo =
supplementalData.getPlurals(PluralType.cardinal, localeID, false);
if (pluralInfo == null && !SpecialLocales.isScratchLocale(localeID)) {
Comment on lines +134 to +136
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, the warning would always fire for Japanese, etc, because the plural rules matched root. This changes to only fire if plural rules were not found.

possibleErrors.add(
new CheckStatus()
.setCause(this)
Expand Down
Loading