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
Conform to IDNA version 15.1.0 revision 31 #454
Conform to IDNA version 15.1.0 revision 31 #454
Conversation
Can we add a test? |
I don't think we should add options that don't exist in PHP. This would defeat the purpose of this library. |
I have decided to remove the "IgnoreInvalidPunycode" option as suggested since the option is not available in PHP. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
Can you please add some tests?
4ae5419
to
da4b033
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed my CS comments by force-pushing on your fork.
Tests would still be welcome. And I have one question (see inline comments).
Are you still interested in finishing this PR?
@@ -280,10 +280,6 @@ private static function mapCodePoints($input, array $options, Info $info) | |||
|
|||
switch ($data['status']) { | |||
case 'disallowed': | |||
$info->errors |= self::ERROR_DISALLOWED; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reasoning for this change according to the spec is "Checking for disallowed characters both before and after normalization yielded inconsistent results for unnormalized vs. normalized text".
Yes, sorry, haven't had a lot of free time lately. I'll try to put some tests together this week. Thanks for taking care of the style issues. |
eccc15d
to
385d1d5
Compare
Thank you @TRowbotham. |
This adds the necessary changes to conform to IDNA version 15.1.0 revision 31.
Notable Changes