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: reconsider placement of pkg.go.dev link in hover #36992

Open
brunnre8 opened this issue Feb 3, 2020 · 7 comments
Open

x/tools/gopls: reconsider placement of pkg.go.dev link in hover #36992

brunnre8 opened this issue Feb 3, 2020 · 7 comments

Comments

@brunnre8
Copy link

@brunnre8 brunnre8 commented Feb 3, 2020

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

go version go1.13.7 linux/amd64

golang.org/x/tools/gopls v0.3.0
    golang.org/x/tools/gopls@v0.3.0 h1:l9KKK1/n6CIbfgaUvHBWAvCfOxcl1N+KSOK79OlPIao=
    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
    github.com/sergi/go-diff@v1.0.0 h1:Kpca3qRNrduNnOQeazBd0ysaKrUJiIuISHxogkT9RPQ=
    golang.org/x/mod@v0.1.1-0.20191105210325-c90efee705ee h1:WG0RUwxtNT4qqaXX3DPA8zHFNm/D9xaBpxzHt1WcA/E=
    golang.org/x/sync@v0.0.0-20190423024810-112230192c58 h1:8gQV6CLnAEikrhgkHFbMAEhagSSnXWGV915qUMm9mrU=
    golang.org/x/tools@v0.0.0-20200130224948-02f1738cbe39 h1:5ERHXLQfA0b8cHOwaOfWaaGekrA4+Ka/N74zilLnsIk=
    golang.org/x/xerrors@v0.0.0-20191011141410-1b5146add898 h1:/atklqdjdhuosWIl6AIbOeHJjicWYPqR9bpxqxYG2pA=
    honnef.co/go/tools@v0.0.1-2019.2.3 h1:3JgtbtFHMiCmsznwGVTUWbgGov+pVqnlf1dEJTNAXeM=
    mvdan.cc/xurls/v2@v2.1.0 h1:KaMb5GLhlcSX+e+qhbRJODnUUBvlw01jt4yrjFIHAuA=

Does this issue reproduce with the latest release?

yes

Issue

gopls v0.3.0 introduces go.dev links on hover.

While this is in principle a good thing for some, currently it adds this to the top of the hover output.
Now, this is the wrong order in my view as I don't want to switch to a browser when I have the hover documentation right there in my editor.

Can the order be switched so that it puts the link at the end of the actual docs?
That still helps the people who'd like to see the link, but it's not a waste of space if the popup window is small for some reason.

Example

@gopherbot gopherbot added the gopls label Feb 3, 2020
@stamblerre stamblerre changed the title gopls: pkg.go.dev link in wrong order on hover x/tools/gopls: reconsider placement of pkg.go.dev link in hover Feb 3, 2020
@gopherbot gopherbot added this to the Unreleased milestone Feb 3, 2020
@gopherbot gopherbot added the Tools label Feb 3, 2020
@golang golang deleted a comment from gopherbot Feb 3, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.4.0 Feb 3, 2020
@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Feb 3, 2020

I'm not sure if I agree here. Placing the link at the bottom of a potentially large block of documentation would require users to scroll through the hover window to reach it (at least, in VS Code). I think the issue maybe lies more with the presentation of hover in different editors, which is why we've filed microsoft/language-server-protocol#772 in the LSP repository.

I might be open to changing things around, like maybe presenting the symbol in the signature as the link, or removing the "on pkg.go.dev" piece.

By the way, what editor client are you using in the screenshot here? The link should only be provided in markdown if the client supports it.

@myitcv
Copy link
Member

@myitcv myitcv commented Feb 3, 2020

The discussion in #33352 is also relevant here, as is the experimental solution we came up with.

@brunnre8
Copy link
Author

@brunnre8 brunnre8 commented Feb 3, 2020

require users to scroll through the hover window to reach it (at least, in VS Code)

I don't quite understand why this is an issue? How often do you really want to navigate to an external site if the rest of the hover already tells you how to use the function/type?

Clicking on the doc link is in my view the less common case, even in VScode.
Also please keep in mind that VScode isn't the only editor in existence.

By the way, what editor client are you using in the screenshot here?
editor is vim with the coc client

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Feb 3, 2020

I don't quite understand why this is an issue? How often do you really want to navigate to an external site if the rest of the hover already tells you how to use the function/type?

I, as well as other users who requested this feature, have found it to be useful - otherwise we wouldn't have added it.

We can use this issue to track user complaints with the link feature, but I don't think we should make any significant changes without more user input and more thought on how these changes should look. I would prefer that the link be easily accessible to users, and I don't think that putting it at the bottom would allow for this.

I would also suggest filing an issue in the coc.vim repo about the markdown formatting.

@brunnre8
Copy link
Author

@brunnre8 brunnre8 commented Feb 3, 2020

Yeah and that's fine, no one said that you need to get rid of the link.

The issue I'm having is that you essentially say that the link is more important than the documentation itself, which I heavily disagree with.
Getting the docs is the whole point of using hover in the first place.
As is, you remove 3 lines of the hover screen estate with information I can't use inside of vim.

Re coc, nothing wrong with that, that's how markdown looks in vim ... TUI and all

@gilcrest
Copy link

@gilcrest gilcrest commented Feb 6, 2020

Hi - thanks for all the work on gopls - it keeps getting better! I'm trying to understand the idea behind this - right now, this hover is appearing in code that I've marked as private with GOPRIVATE and the link does not bring me to a real module (which is good, actually, because it's private!). Is this correct behavior?

It actually gave me a scare when I first saw it...

Thanks!

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Feb 11, 2020

Sorry about that! We actually have an open issue for respecting GOPRIVATE settings - #36998.

@stamblerre stamblerre modified the milestones: gopls/v0.4.0, gopls/v1.0.0 Feb 26, 2020
@stamblerre stamblerre added the Thinking label Mar 12, 2020
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.5.0 May 21, 2020
@stamblerre stamblerre removed this from the gopls/v0.5.0 milestone Jun 24, 2020
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
5 participants
You can’t perform that action at this time.