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

x/tools/gopls: fails to link some scheme-less URLs #38285

Closed
heschik opened this issue Apr 7, 2020 · 4 comments
Closed

x/tools/gopls: fails to link some scheme-less URLs #38285

heschik opened this issue Apr 7, 2020 · 4 comments
Labels
Milestone

Comments

@heschik
Copy link
Contributor

@heschik heschik commented Apr 7, 2020

We use the Relaxed regex from https://github.com/mvdan/xurls to find URLs to linkify. That regex accepts URLs without schemes like golang.org rather than http://golang.org, which we then parse with url.Parse. That works okay for most URLs. (From url.Parse: "Trying to parse a hostname and path without a scheme is invalid but may not necessarily return an error, due to parsing ambiguities.").

However, for at least some URLs with colons, e.g. 127.0.0.1:80, parsing fails ("first path segment in URL cannot contain colon") and the URL is not linkified. golang.org:8080 works for reasons I haven't dug into.

@gopherbot gopherbot added this to the Unreleased milestone Apr 7, 2020
@heschik
Copy link
Contributor Author

@heschik heschik commented Apr 7, 2020

cc @mvdan

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Apr 7, 2020
@mvdan
Copy link
Member

@mvdan mvdan commented Apr 7, 2020

This seems like more of a problem with the URL parsing, than xurls per se, no? I'm happy to discuss bugs and enhancements, but I'm not sure if there are any here for me :)

As an idea, perhaps try https://golang.org/pkg/net/#SplitHostPort before https://golang.org/pkg/net/url/#Parse. That could help you support ipv4:port and [ipv6]:port, if changing the behavior of url.Parse is not possible.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 8, 2020

Change https://golang.org/cl/227559 mentions this issue: internal/lsp: linkify IP addresses in textDocument/documentLink

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Apr 8, 2020

Thanks for the advice, @mvdan! You're right - it's strictly a gopls issue. I ended up finding #18824, which I used to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.