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 the document links/hovers for import/module statements #51848

Open
hyangah opened this issue Mar 21, 2022 · 3 comments
Open
Assignees
Labels
Documentation FeatureRequest gopls Tools
Milestone

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Mar 21, 2022

Currently gopls decorates each import statement with a link to the package doc
in pkg.go.dev (or a custom doc site if users configured a different linkTarget) unconditionally.
However, I found the current choice is not optimal because

  • the link to the standard library package always points to the latest stable version of go's. That maybe not be correct.
  • the link to the third-party library package points to the right versioned doc, but that may not exist in pkg.go.dev.
  • the link to the private packages points to the public pkg.go.dev by default, that's not appropriate.
  • the link for the replaced dependencies or go.work-adjusted dependencies can't be right.
  • redirecting users to the external website is distracting, and often surprising.
  • asking users to click an external link without presenting the full URLs can be problematic - imagine a repo has .vscode/settings.json with malicious linkTarget configured.

I propose we change this part of UX/UI.

  • render proper package doc (or at least the snippet - the very first line) as a hover.
  • add the link to one of the package file, maybe the one provided the package doc. (note: currently VS Code and LSP lack handling of folder-type links - microsoft/vscode#141564)
  • add the link to the pkg.go.dev as a link in the hover message (like other hovers for symbols) so users know where the clicking action takes them.

I also propose a similar change for the links used in go.mod files.
For document links, maybe we can consider the go.mod file of the module stored in the module cache.

@gopherbot gopherbot added Tools gopls labels Mar 21, 2022
@gopherbot gopherbot added this to the Unreleased milestone Mar 21, 2022
@muirdm
Copy link

@muirdm muirdm commented Mar 21, 2022

Can gopls run a doc server locally and point URLs to itself? That way it can serve the right version of everything (presumably), and it would work offline. I imagine running godoc would be easy - not sure the state of the pkg.go.dev stuff.

add the link to one of the package file, maybe the one provided the package doc

This behavior overlaps somewhat with go-to-definition. go-to-definition currently gives a list of the package files for the user to choose.

@hyangah
Copy link
Contributor Author

@hyangah hyangah commented Mar 21, 2022

Can gopls run a doc server locally and point URLs to itself? That way it can serve the right version of everything (presumably), and it would work offline. I imagine running godoc would be easy - not sure the state of the pkg.go.dev stuff.

vscode-go (&gopls) plans to do it eventually - we prefer more simplified documentation (either a plain html or markdown without special styling/javascript) than what pkg.go.dev shows and want ability to get the documentation part only so the editor can choose the right style (e.g. dark theme, ... )

add the link to one of the package file, maybe the one provided the package doc
This behavior overlaps somewhat with go-to-definition. go-to-definition currently gives a list of the package files for the user to choose.

Go-to-definition is what I currently use to avoid visiting pkg.go.dev, and "link to one of the package file" was suggested because I think that is better than the current link to pkg.go.dev. If gopls can provide a simple local documentation service soon, I agree that's a good choice.

Still I think presenting the package snippet, link to the doc (local server), and link to the pkg.go.dev is handy - so no extra click to go to pkg.go.dev to learn more about the newer version, etc or read a quick summary about the package.

@findleyr findleyr removed this from the Unreleased milestone Mar 23, 2022
@findleyr findleyr added this to the gopls/on-deck milestone Mar 23, 2022
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 18, 2022

Change https://go.dev/cl/400820 mentions this issue: internal/lsp: render proper package documentation when hovering over a package

gopherbot pushed a commit to golang/tools that referenced this issue Apr 25, 2022
…ge import

Update FindHoverContext to retrieve & render package documentation for a hovered package. Add a regtest for this hovering feature.

Updates golang/go#51848

Change-Id: If57396d59be9c4cf7e09b64e39832de6f996c7ca
Reviewed-on: https://go-review.googlesource.com/c/tools/+/400820
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Dylan Le <dungtuanle@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation FeatureRequest gopls Tools
Projects
None yet
Development

No branches or pull requests

5 participants