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

[Feature] Cover JSON schema string format #141

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Feature] Cover JSON schema string format #141

wants to merge 1 commit into from
+675 −2

Conversation

cyx-6
Copy link

@cyx-6 cyx-6 commented Jan 8, 2025

This PR covers the most string formats in JSON schema, except "idn-email", "idn-hostname", "iri", and "iri-reference".

@Ubospica
Copy link
Collaborator

Ubospica commented Feb 5, 2025

Please rebase to the latest main since we just force-pushed it. See #177 (comment) for more info.

@cyx-6
regex
f1979d5 
ut part 1

wip

uri and uri-reference

ipv6

json-pointer uri-template

refactor email
@cyx-6 cyx-6 force-pushed the json-schema-format branch from fa8049f to f1979d5 Compare February 25, 2025 06:06
benchislett
benchislett reviewed Mar 7, 2025
View reviewed changes
cpp/json_schema_converter.cc
RegexToEBNF(relative_json_pointer_regex_pattern, false);
return "\"\\\"\" " + relative_json_pointer_ebnf + " \"\\\"\"";
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably remove the "format" key in WarnUnsupportedKeywords below

Copy link
Author

Choose a reason for hiding this comment

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

Sure. Actually some sub-types in "format" are still missing, e.g. iri and idn-email. Maybe we need to notify in detail.

benchislett
benchislett reviewed Mar 7, 2025
View reviewed changes
Copy link
Contributor

@benchislett benchislett left a comment

Choose a reason for hiding this comment

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

Is this PR still under review? Could it be merged before the next upcoming release?

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

@benchislett benchislett benchislett left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@cyx-6 @Ubospica @benchislett