Skip to content

Conversation

@eggrobin
Copy link
Member

@eggrobin eggrobin commented Dec 23, 2025

@eggrobin eggrobin requested a review from markusicu December 23, 2025 22:13
@markusicu
Copy link
Member

Wowza... @macchiati will like this :-)

Comment on lines +1893 to +1894
// Link_Bracket
public enum Link_Term_Values implements Named {
Copy link
Member

Choose a reason for hiding this comment

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

Weird: These two lines don't go together.

Not a regression here because I see that the next section has the same problem:

    // Lowercase_Mapping
    public enum Math_Class_Values implements Named {

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, for non-enumerated values the generator emits a comment. It should probably invest in a new line, so that the comment does not look like it is about the next enumerated type…

@markusicu
Copy link
Member

Except: It looks like adding the new properties without updating a bunch of propertywise tests breaks those...
Maybe exclude newly added properties from those tests?

@eggrobin
Copy link
Member Author

eggrobin commented Dec 23, 2025

Looks like this breaks the comparison tests because it is actually 17.0 data pretending to be for 18.0 (so new characters get linkification properties derived from their unassignedness, but their comparanda have meaningful linkification properties).
We should probably move it to the 17.0 directory of the tools and generate an 18.0 dev directory (with the understanding that the linkification/17.0 directory is not stable until we have published UTS58).

@markusicu
Copy link
Member

We should probably move it to the 17.0 directory of the tools and generate an 18.0 dev directory

ok

markusicu
markusicu previously approved these changes Dec 24, 2025
macchiati
macchiati previously approved these changes Dec 24, 2025
@eggrobin eggrobin dismissed stale reviews from macchiati and markusicu via 28088f6 December 24, 2025 02:52
new UnicodeSet(
"[\\p{XID_Continue}\\p{block=basic_latin}-\\p{Cc}]",
new ParsePosition(0),
VersionedSymbolTable.frozenAt(Settings.LATEST_VERSION_INFO))
Copy link
Member

Choose a reason for hiding this comment

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

Should the version here be sensitive to the new flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. Done.

@eggrobin eggrobin reopened this Dec 24, 2025
@eggrobin eggrobin merged commit 6df8f96 into unicode-org:main Dec 24, 2025
28 of 30 checks passed
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