Skip to content

Comments

Editorial: Fix incorrect use of UnicodeMatchPropertyValue#3587

Open
gibson042 wants to merge 12 commits intotc39:mainfrom
gibson042:gh-3586-unicodematchproperty
Open

Editorial: Fix incorrect use of UnicodeMatchPropertyValue#3587
gibson042 wants to merge 12 commits intotc39:mainfrom
gibson042:gh-3586-unicodematchproperty

Conversation

@gibson042
Copy link
Member

@gibson042 gibson042 commented May 4, 2025 •
edited
Loading

Fixes #3586
Closes #3590

Also includes commits with incidental fixes in nearby algorithms and steps.

Jack-Works
Jack-Works reviewed May 5, 2025
View reviewed changes
bakkot
bakkot reviewed May 5, 2025
View reviewed changes
gibson042
gibson042 commented May 5, 2025
View reviewed changes
@Jack-Works
Copy link
Member

Jack-Works commented May 7, 2025

I have another question, if you search "scx" in https://unicode.org/Public/UCD/latest/ucd/PropertyValueAliases.txt, you will find there is no record for it. And according to the spec,

UnicodePropertyValueExpression :: UnicodePropertyName = UnicodePropertyValue

It is a Syntax Error if the source text matched by UnicodePropertyValue is not a property value or property value alias for the Unicode property or property alias given by the source text matched by UnicodePropertyName listed in PropertyValueAliases.txt.

All RegExp that have the form of /\p{Script_Extensions=Anything}/u (e.g., /\p{Script_Extensions=Zanabazar_Square}/u) are an early error, but it is not true in the implementation.

@gibson042 gibson042 mentioned this pull request May 8, 2025
Open
@gibson042
Copy link
Member Author

gibson042 commented May 8, 2025

Nice observation, @Jack-Works! Unicode property Script_Extensions (scx) is unusual in being set-valued rather than scalar-valued, and as such need special consideration in our spec. I have added editorial corrections to this PR, and opened #3590 for a potential followup.

Jack-Works
Jack-Works reviewed May 10, 2025
View reviewed changes
spec.html Outdated
1. If _p_ is `Script_Extensions`, then
1. Assert: _vs_ is a property value or property value alias for property “Script” listed in <a href="https://unicode.org/Public/UCD/latest/ucd/PropertyValueAliases.txt"><code>PropertyValueAliases.txt</code></a>.
1. Let _v_ be the Set containing the “short name”, “long name”, and any other aliases corresponding with value _vs_ for property “Script” in <a href="https://unicode.org/Public/UCD/latest/ucd/PropertyValueAliases.txt"><code>PropertyValueAliases.txt</code></a>.
1. Return the CharSet containing all Unicode code points whose character database definition includes the property “Script_Extensions” with value having a non-empty intersection with _v_.
Copy link
Member

@Jack-Works Jack-Works May 10, 2025

Choose a reason for hiding this comment

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

Do you need to call MaybeSimpleCaseFolding here?

Copy link
Member Author

@gibson042 gibson042 May 11, 2025

Choose a reason for hiding this comment

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

Hmm, that would affect any code point that case-folds across script (or to/from Common). I don't know if there are any, but it's easy enough to accommodate. Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelficarra michaelficarra michaelficarra left review comments

@bakkot bakkot bakkot left review comments

+1 more reviewer

@Jack-Works Jack-Works Jack-Works left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

editorial change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implementation divergence for Script_Extensions values UnicodeMatchPropertyValue assertion fails by UnicodePropertyValueExpression

5 participants

@gibson042 @Jack-Works @michaelficarra @bakkot @ljharb