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

net/url: Improve documentation for PathUnescape #26139

Closed
tooolbox opened this issue Jun 29, 2018 · 4 comments
Closed

net/url: Improve documentation for PathUnescape #26139

tooolbox opened this issue Jun 29, 2018 · 4 comments
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@tooolbox
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.10.2

Does this issue reproduce with the latest release?

N/A

What operating system and processor architecture are you using (go env)?

N/A

What did you do?

Looked at the godoc for PathUnescape

What did you expect to see?

PathUnescape does the inverse transformation of PathEscape, converting each 3-byte encoded substring of the form "%AB" into the hex-decoded byte 0xAB. It returns an error if any % is not followed by two hexadecimal digits.

PathUnescape is identical to QueryUnescape except that it does not unescape '+' to ' ' (space).

What did you see instead?

PathUnescape does the inverse transformation of PathEscape, converting each 3-byte encoded substring of the form "%AB" into the hex-decoded byte 0xAB. It also converts '+' into ' ' (space). It returns an error if any % is not followed by two hexadecimal digits.

PathUnescape is identical to QueryUnescape except that it does not unescape '+' to ' ' (space).

Commentary

It seems like the documentation for PathUnescape was copied from QueryUnescape, with the last line added. However, it then contradicts itself about its behavior with + characters.

Fix

Remove the line It also converts '+' into ' ' (space). from the PathUnescape comment. I would submit a PR but the amount of hoops I'd have to jump through to contribute one line of code is a little much; I'd like to ask that someone who's already set up as Go contributor take care of this.

Thanks!

@ianlancetaylor ianlancetaylor added NeedsFix The path to resolution is known, but the work has not been done. help wanted labels Jun 29, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.12 milestone Jun 29, 2018
@ianlancetaylor
Copy link
Contributor

@tooolbox In case you didn't know, we do accept Github pull requests now.

@conspicuousClockwork
Copy link
Contributor

I shamelessly made the fix myself and opened a pull request. @tooolbox I'll close it myself if you would like to take credit for commit!

@gopherbot
Copy link

Change https://golang.org/cl/121696 mentions this issue: net/url: correct the documentation for PathUnescape

gopherbot pushed a commit that referenced this issue Jun 29, 2018
Fixes issue #26139

Change-Id: Id9a3e5c443ee175ad9add6296ed45bdf328b15a0
GitHub-Last-Rev: b3f8a8f
GitHub-Pull-Request: #26146
Reviewed-on: https://go-review.googlesource.com/121696
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@tooolbox
Copy link
Author

@ianlancetaylor Ah, good to know.

@conspicuousClockwork Ouch, wouldn't have minded. I guess I got what I asked for in the first place ;)

@golang golang locked and limited conversation to collaborators Jun 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants