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

Ignore UTS46 validity criteria V2 #240

Merged
merged 1 commit into from
Nov 22, 2016
Merged

Ignore UTS46 validity criteria V2 #240

merged 1 commit into from
Nov 22, 2016

Conversation

canova
Copy link

@canova canova commented Nov 18, 2016

Fixes #160
r? @SimonSapin or @Manishearth


This change is Reviewable

@canova canova force-pushed the idna branch 2 times, most recently from 66d4f1e to 7a11639 Compare November 18, 2016 21:08
(third, fourth) == (Some('-'), Some('-'))
} || label.starts_with("-")
// Spec says, the label must not contain a HYPHEN-MINUS character in both the
// third and fourth positions. But nobody follow this criteria. See the spec issue below:
Copy link
Member

Choose a reason for hiding this comment

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

"Spec says that the", and "follows"

Copy link
Author

Choose a reason for hiding this comment

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

Fixed, thanks!

@Manishearth
Copy link
Member

LGTM but I would like sign-off from @SimonSapin here

let fourth = chars.next();
(third, fourth) == (Some('-'), Some('-'))
} || label.starts_with("-")
// Spec says that the, label must not contain a HYPHEN-MINUS character in both the
Copy link
Member

Choose a reason for hiding this comment

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

no comma

@emilio
Copy link
Member

emilio commented Nov 19, 2016

Isn't this the same as #208? If so, it's really sad that that PR has been sitting there for so long :(

@Manishearth
Copy link
Member

It removes the configurability.

@canova
Copy link
Author

canova commented Nov 19, 2016

Fixed the typo

@Manishearth
Copy link
Member

r? @SimonSapin

@SimonSapin
Copy link
Member

@bors-servo r+

Thanks!

@bors-servo
Copy link
Contributor

📌 Commit 695a507 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

⌛ Testing commit 695a507 with merge 52234db...

bors-servo pushed a commit that referenced this pull request Nov 22, 2016
Ignore UTS46 validity criteria V2

Fixes #160
r? @SimonSapin or @Manishearth

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-url/240)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis

@bors-servo bors-servo merged commit 695a507 into servo:master Nov 22, 2016
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.

5 participants