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.

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

CLDR-17792 Don't strip ZWSP for value compare, allow currency symbol collision for it #3867

Conversation

pedberg-icu
Copy link
Contributor

@pedberg-icu pedberg-icu commented Jul 13, 2024

CLDR-17792

  • This PR completes the ticket.

Currently Survey Tools is reporting a display collision for \u200B (ZWSP) used as currency symbol for PTE in pt_PT, even though this is the value that has been there for several releases, and is the correct value to use since for this particular currency in pt_PT the symbol is actually in the decimal separator element; see https://st.unicode.org/cldr-apps/v#/pt_PT/C_SEEurope/7f78787c0e4c5be3 (best to view this with coverage set to Comprehensive to see the associated fields).

The error does not appear in offline unit tests or Console Check, so the fix here is somewhat speculative:

  • Update SimpleXMLSource.NON_ALPHANUM so \u200B does not get stripped when comparing values across paths
  • Update CheckDisplayCollisions to specifically allow a collision for \u200B as a currency symbol if there is an explicit decimal separator for the same currency.
  • Add a test for the above, and also a test to verify that DAIP is not altering the \u200B as a currency symbol.

ALLOW_MANY_COMMITS=true

@pedberg-icu pedberg-icu merged commit 7750859 into unicode-org:main Jul 14, 2024
11 checks passed
@pedberg-icu pedberg-icu deleted the CLDR-17792-ZWSP-dont-strip-for-value-compare-allow-cur-sym-collision branch July 14, 2024 20:32
Copy link
Member

@srl295 srl295 left a comment

Choose a reason for hiding this comment

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

post merge LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants