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.

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

IDNA support #152

Merged
merged 9 commits into from Jan 18, 2016
Merged

IDNA support #152

merged 9 commits into from Jan 18, 2016

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Dec 5, 2015

@valenting, I rebased your PR #119 and then worked on it some. Could you have a look?

Two things are known missing (and have FIXME comments):

There are some tests that are supposed to return an error but currently don’t. (The corresponding assert! is commented out.) Some are because of the bidi rule, but I’m not sure about others. (For example a\u200Cb.)

Review on Reviewable

@SimonSapin SimonSapin mentioned this pull request Dec 5, 2015
if {
let mut chars = label.chars();
let _first = chars.next();
let _first = chars.next();

This comment has been minimized.

@pyfisch

pyfisch Jan 16, 2016

Contributor

should probably be _second?

// Input is from nfc(), so it must be in NFC?
// Can not contain '.' since the input is from .split('.')
if {
let mut chars = label.chars();

This comment has been minimized.

@pyfisch

pyfisch Jan 16, 2016

Contributor

You can use let mut chars = label.chars().skip(2);

http://doc.rust-lang.org/std/iter/trait.Iterator.html#method.skip

@SimonSapin SimonSapin merged commit c35d040 into master Jan 18, 2016
4 checks passed
4 checks passed
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@SimonSapin SimonSapin deleted the idna branch Feb 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.