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

Refactor parse query and percent-encode sets #152

Closed
wants to merge 2 commits into from
Closed

Refactor parse query and percent-encode sets #152

wants to merge 2 commits into from

Conversation

annevk
Copy link
Contributor

@annevk annevk commented May 14, 2020

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

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

This doesn't appear up to date with https://whatpr.org/url/518.html, in particular the percent-encode after encoding operation or various other changes. Unfortunately maybe we lagged behind a bit on other editorial changes which would make this a bigger lift.

Do you want me to help get this up to date?

@annevk
Copy link
Contributor Author

annevk commented May 14, 2020

I would appreciate help, yes. In particular with pulling in some kind of encoding library and figuring out how to run the tests in encoding/ relevant to URLs. (It also seems like jsdom/whatwg-encoding isn't quite compliant, but maybe it's good enough for some tests...)

I was also thinking that maybe url-state-machine.js and urlencoded.js could both use a shared percent-encoded.js that contains all that logic.

@domenic
Copy link
Member

domenic commented May 14, 2020

Great. Depending on how much time you have today, getting #154 (whatwg/url#459) and #151 (whatwg/url#512) merged first would be nice. And ideally #148 (whatwg/url#505), though that still needs tests. But we can also do it independently, and deal with any merge conflicts as necessary.

@annevk
Copy link
Contributor Author

annevk commented May 14, 2020

Covered the first two. I also lack implementer interest for that last one unfortunately.

domenic added a commit that referenced this pull request Jun 25, 2020
Follows whatwg/url#518.

This generally tries to make the code correspond more explicitly to the specification in a few ways. It doesn't handle non-UTF-8 encodings yet, though.

Supersedes #152.
@domenic domenic closed this Jun 25, 2020
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.

2 participants