Skip to content

Add valid first character to avoid Bidi rule violation in Arabic-Indic digits test #734

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

Conversation

OptimumCode
Copy link

@OptimumCode OptimumCode commented Apr 13, 2024

Hello, this PR resolves #737

@OptimumCode OptimumCode requested a review from a team as a code owner April 13, 2024 17:15
@OptimumCode
Copy link
Author

I am sorry, I probably should have created an issue first...

If the issue is required - please, let me know. I will create it and link it to the PR

@OptimumCode
Copy link
Author

Issue has been created

Copy link
Member

@Julian Julian left a comment

Choose a reason for hiding this comment

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

This LGTM given your comment in the issue -- I'll leave it without merging to see if someone else has a read there in the issue, if no one else pipes up we can merge. Thanks!

@gregsdennis
Copy link
Member

I ran these against my implementation, and it seems my implementation apparently doesn't do IDN hostname validation well: these fail both before and after the changes for me. (It's fine, they're optional.)

@mwadams
Copy link
Contributor

mwadams commented Apr 30, 2024

I pass the previous IDN hostname tests. I'll give this one a go tomorrow.

@OptimumCode
Copy link
Author

I pass the previous IDN hostname tests. I'll give this one a go tomorrow.

Hi, @mwadams. Could you please advise if you had a chance to try the updated test case?

@mwadams
Copy link
Contributor

mwadams commented May 6, 2024

Hi - I can confirm that Corvus.JsonSchema still passes the test with this change.

@Julian Julian merged commit c2badb1 into json-schema-org:main May 6, 2024
1 check 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.

Tests for mixed Arabic-Indic digits and Extended Arabic-Indic digits violates the Bidi rule
4 participants