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

Jump to bottom

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

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

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

heschi opened this issue Apr 7, 2020 · 4 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.4.1

Comments

@heschi
Copy link
Contributor

heschi commented Apr 7, 2020 •
edited

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
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Apr 7, 2020
@heschi
Copy link
Contributor Author

heschi commented Apr 7, 2020

cc @mvdan

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

mvdan commented Apr 7, 2020 •
edited

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

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

@stamblerre
Copy link
Contributor

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.

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.1 May 13, 2020
@golang golang locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.4.1
Development

No branches or pull requests
4 participants
@mvdan @stamblerre @gopherbot @heschi