x/tools/gopls: deprecate 'Structured' hover kind

[findleyr]

The 'Structured' hover kind does not serve human readable hover content. Rather, it serves JSON describing the hover.
https://github.com/golang/tools/blob/master/gopls/doc/settings.md#hoverkind-enum

gopls should conform to the semantics of the LSP: hover is intended to be human readable. If we want to delegate hover presentation to the client, we must by definition have a thick client, in which case the client can just use a separate command via workspace/executeCommand. By analogy, tsserver has a command for structured hover info: https://github.com/microsoft/TypeScript/blob/55f1248a2052eebdea6bc0e2eef754df89a44bf7/src/server/protocol.ts#L2015.

CC @adonovan

[adonovan]

I vaguely recall that the sole thick client of the Hover JSON output is one of the Vims. (The feature was added in the CL attached to #33352. Paging @myitcv for awareness.)

[myitcv]

Paging @myitcv for awareness.

Thanks, @adonovan - we're actually using what I believe the gopls default is in this space, "FullDocumentation":

https://github.com/govim/govim/blob/6280deb8b406332ed0c9f0eea59671d954b815ab/cmd/govim/gopls_client.go#L159

[adonovan]

[govim is] actually using what I believe the gopls default is in this space, "FullDocumentation":

Great, let's remove kind=structured then.

[gopherbot]

Change https://go.dev/cl/635226 mentions this issue: gopls/internal/settings: drop experimental hoverKind=Structured

[bhcleek]

FWIW, vim-go was using Structured.

[findleyr]

@bhcleek very sorry about the breakage :(.

This was, I think, perhaps the first time we've just removed a setting without a probationary release where it is a warning. We really thought the only user was govim -- I'm afraid the similar naming of the clients may have been part of the confusion. Lesson (re)learned.

What can we do to help? We can revert the deprecation temporarily, though that might be messy (I'll try). If instead you can fix forward, that may be cleanest.

Going forward If you need structured information from hover, we can can add a custom command. As described in the release notes, it was a mistake to shoehorn a JSON API into hover.

[bhcleek]

I can probably refactor vim-go to get what I need, but I won't be able to get to it immediately.

IIRC, I'd relied on Structured, because FullDocumentation was missing some things vim-go needed and the structure of the documentation result when FullDocumentation was used did not come with guarantees; parsing free form text is tricky, especially if the server doesn't guarantee its form across versions.

[findleyr]

Reopening and moving to the v0.19.0 milestone, since this deletion is being reverted.

[bhcleek]

@findleyr

Would it be possible to change the plaintext hover content to use two newlines between the identifier and the beginning of its documentation?

I'm also wondering what determines where single newlines are placed within the actual documentation after the identifier.

edit: strike wondering after checking actual godoc source for fmt.Println and realizing I'd assumed something based on how the pkg.go.dev renders it.

[bhcleek]

@findleyr nevermind, I think I've found a way forward.

[bhcleek]

@findleyr I got most of what I needed done, but there's one aspect of the structured content that I've been unable to find a good answer for. The structured output includes linkPath and linkAnchor fields. Vim-go uses those fields to construct documentation links, notably for opening a browser.

Interestingly, the markdown format provides the links I'd need with FullDocumentation, but I can't mix plaintext and markdown, because the format has to be chosen in the initialize method, and markdown is otherwise not well suited for vim-go's needs.

I was hoping that textdocument/documentLink would provide a useful alternative, but it seems to only provide links for the imports.

Do you have any suggestions?

[findleyr]

Would it suffice to include the links in plaintext mode, in the footer? That seems reasonable.

For users that don't want links, we could we could extend the "linksInHover" setting to plaintext:
https://github.com/golang/tools/blob/master/gopls/doc/settings.md#linksinhover-enum

I don't think we should use documentLink for this.

[bhcleek]

Yes, I think that could work for vim-go's purposes as long as:

1. only the link for the documentation being provided is provided
2. there is some guarantee about where in the plaintext the link will be found (e.g. last line?)

[bhcleek]

@findleyr will there be an intermediate release of gopls before Structured is removed so I can release changes of vim-go that expect the links in plaintext documentation before Structured is removed?

[findleyr]

@bhcleek yes, there will be an intermediate release (v0.19.0).

[bhcleek]

Thank you. I have fatih/vim-go#3702 staged and will update it with the rest of the work it requires after v0.19.0 is released.

[adonovan]

I wonder what is gained by replacing a weakly defined JSON structure that is parsed by some clients by an ill-defined text structure that is parsed by some clients. Perhaps we should just live with it.

[bhcleek]

I agree @adonovan . I'd prefer for Structured to remain.

Vim-go could completely workaround removal of Structured without needing changes to plaintext if gogetdoc did the right thing for fatih/vim-go#2295, but gogetdoc hasn't been updated in a very long time. fatih/vim-go#2822 is the issue where vim-go stopped relying on gogetdoc in favor of gopls to get the documentation links.

[findleyr]

If the only thing causing us to hold on to Structured is the ability to resolve documentation links, I propose we migrate that feature to a custom command, as that will be far less burdensome on the codebase.

[bhcleek]

@findleyr are you proposing to implement textdocument/documentLink more fully?

[findleyr]

@bhcleek no, I was proposing having a separate command (to be invoked via workspace/executeCommand. IIRC Documentlink causes many editors to linkify symbols in a way that would be distracting.

[findleyr]

This is not happening for v0.19.0 moving to v0.20.0.