Skip to content

Regex editorial improvements, and permit "." #1470

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

Merged
merged 6 commits into from
Jun 18, 2024

Conversation

lukem
Copy link
Contributor

@lukem lukem commented Nov 11, 2023

Add "." to the permitted simple atoms, for issue #1469.
Consistently format the regex examples.

Consistently "quote" examples.
Split complemented simple and range class examples.
Improve "(" .. ")" grouping example.
@gregsdennis
Copy link
Member

gregsdennis commented Feb 15, 2024

We've discussed this before, and I think it may be worth bringing it up again.

IETF has now published https://datatracker.ietf.org/doc/rfc9485/ to support interoperable regex in JSON Path. It may be wise to reconsider this in place of ECMA. Doing so would mean being able to remove all of these restrictions.

@notEthan
Copy link
Contributor

@gregsdennis I feel like I'm inclined to agree with this person's opinion that iregexp is not a good fit for JSON Schema - though I have not kept up with any changes since 2022 that may have changed that situation.

@gregsdennis
Copy link
Member

Yeah, I hear you. But finding interoperable regex support has been a thorn in our side for a long time.

It really has no bearing on this PR, though.

@lukem
Copy link
Contributor Author

lukem commented Feb 22, 2024

I've added 3 commits addressing various review feedback above.

Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

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

This looks fine to me.

@gregsdennis gregsdennis requested a review from a team February 23, 2024 02:04
@gregsdennis gregsdennis merged commit a45d267 into json-schema-org:main Jun 18, 2024
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