golink: strip trailing punctuation when resolving links #159
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's not uncommon for auto-linkers or simple copy/paste errors to accidentally include trailing punctuation in a golink. When resolving links, if the initial link was not found, then try again with common punctuation (that is invalid in link names anyway) removed. Fixes #148 Change-Id: Ia101a4a3005adb9118051b3416f5a64a4a45987d Signed-off-by: Will Norris <[email protected]>
creachadair
approved these changes
Jan 10, 2025
| if errors.Is(err, fs.ErrNotExist) { | ||
| // Trim common punctuation from the end and try again. | ||
| // This catches auto-linking and copy/paste issues that include punctuation. | ||
| if s := strings.TrimRight(short, ".,()[]{}"); short != s { |
There was a problem hiding this comment.
Even though it's valid, it might be worth taking ? too, since such a URL's query is empty anyway at that point (the same argument could apply to ., since it's valid in paths). YMMV.
There was a problem hiding this comment.
hmm... I tried adding ?, but turns out it's not really needed. Since this is based solely on the r.URL.Path value, it seems that the only way a question mark could appear is if it was url encoded as %3F. But then we never url-decode, so it wouldn't actually get stripped off. Since we're just trying to handle accidental punctuation, I don't think we need to worry about ?, since it would get stripped already by the URL parsing.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's not uncommon for auto-linkers or simple copy/paste errors to accidentally include trailing punctuation in a golink. When resolving links, if the initial link was not found, then try again with common punctuation (that is invalid in link names anyway) removed.
Fixes #148
Change-Id: Ia101a4a3005adb9118051b3416f5a64a4a45987d