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

Closed
hyangah opened this issue Mar 21, 2022 · 3 comments
Closed

x/tools/gopls: reconsider the document links/hovers for import/module statements #51848

hyangah opened this issue Mar 21, 2022 · 3 comments
Assignees
@dle8
Labels
Documentation FeatureRequest 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.8.4

Comments

@hyangah
Copy link
Contributor

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 - Ability to expand a selected folder in the file explorer 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 This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Mar 21, 2022
@gopherbot gopherbot added this to the Unreleased milestone Mar 21, 2022
@muirdm
Copy link

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 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 modified the milestones: Unreleased, gopls/on-deck Mar 23, 2022
@gopherbot
Copy link

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
internal/lsp: render package documentation when hovering over a packa…
559469a 
…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>
@dle8 dle8 closed this as completed Aug 17, 2022
@findleyr findleyr modified the milestones: gopls/later, gopls/v0.8.4 Oct 20, 2022
@golang golang locked and limited conversation to collaborators Oct 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dle8 dle8

Labels
Documentation FeatureRequest 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.8.4
Development

No branches or pull requests
5 participants
@hyangah @gopherbot @muirdm @dle8 @findleyr