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: improve textDocument/documentLink #33505

Open
stamblerre opened this issue Aug 6, 2019 · 8 comments
Open

x/tools/gopls: improve textDocument/documentLink #33505

stamblerre opened this issue Aug 6, 2019 · 8 comments

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Aug 6, 2019

Some additional features we should add support for in textDocument/documentLink:

  • Links missing http:// or https:// should be treated as links. For example, golang.org/issue/12345. (Completed via golang.org/cl/194661).
  • Links to the Go issue tracker should be treated as links. For example, golang/go#12345.

Please add any others as ideas come up.

@gopherbot gopherbot added this to the Unreleased milestone Aug 6, 2019
@stamblerre stamblerre added Suggested and removed Documentation labels Aug 6, 2019
@stamblerre stamblerre added help wanted and removed Suggested labels Aug 8, 2019
@eduvim

This comment has been minimized.

Copy link

@eduvim eduvim commented Aug 13, 2019

I'm working on it

eduvim added a commit to eduvim/tools that referenced this issue Sep 10, 2019
The existing implementation has no consideration of links with no
protocol specification, so "example.com/comments" now it's trated as
"https://example.com/comments"

"Fixes golang/go#33505"
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Sep 10, 2019

Change https://golang.org/cl/194537 mentions this issue: internal/links: Improve links parser, no protocol specification

eduvim added a commit to eduvim/tools that referenced this issue Sep 11, 2019
The existing implementation has no consideration of links with no
protocol specification, so "example.com/comments" now it's trated as
"https://example.com/comments"

"Fixes golang/go#33505"
eduvim added a commit to eduvim/tools that referenced this issue Sep 11, 2019
The existing implementation has no consideration of links with no
protocol specification, so "example.com/comments" now it's trated as
"https://example.com/comments"

"Fixes golang/go#33505"
eduvim added a commit to eduvim/tools that referenced this issue Sep 11, 2019
The existing implementation has no consideration of links with no
protocol specification, so "example.com/comments" now it's trated as
"https://example.com/comments"

"Fixes golang/go#33505"

Corrects the regexp definition
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Sep 11, 2019

Change https://golang.org/cl/194661 mentions this issue: internal/links: Improve links parser, no protocol specification

eduvim added a commit to eduvim/tools that referenced this issue Sep 12, 2019
The existing implementation has no consideration of links with no
protocol specification, so "example.com/comments" now it's trated as
"https://example.com/comments"

Fixes golang/go#33505

Corrects the regexp definition
eduvim added a commit to eduvim/tools that referenced this issue Nov 30, 2019
The existing implementation has no consideration of links with no
protocol specification, so "example.com/comments" now it's trated as
"https://example.com/comments"

Fixes golang/go#33505

Corrects the regexp definition
@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Dec 3, 2019

Re-opening because not all of these features have been implemented.

@stamblerre stamblerre reopened this Dec 3, 2019
@eduvim

This comment has been minimized.

Copy link

@eduvim eduvim commented Dec 3, 2019

what we can do is to check at links file if the link is http | https or nothing followed by golang/go#number . I can help with it. I just need to understand it in order to don't make a lot of pr's xD

@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Dec 3, 2019

@eduvim: If you'd like to continue work on this, I think we could add a second regex to match on golang/go#1234. Please feel free to add any other ideas as they come up.

@stamblerre stamblerre modified the milestones: Unreleased, gopls unplanned Dec 4, 2019
@zikaeroh

This comment has been minimized.

Copy link

@zikaeroh zikaeroh commented Dec 4, 2019

Is there any chance that the hueristic can be disabled without also disabling import links? I'm finding the current changes pretty distracting when it thinks things are links which actually aren't, e.g.:

image

I'd personally prefer this feature to not just accept anything that's foo.bar and start treating it as links in strings/comments.

And, if you hadn't seen it, there's already some great work in this area by @mvdan I use in another project: https://github.com/mvdan/xurls

@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Dec 4, 2019

Thank you for the suggestion @zikaeroh! I would be in favor of us using this library to do a better job of detecting links. What do you think, @ianthehat?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.