-
Notifications
You must be signed in to change notification settings - Fork 89
hover: Include struct and interface fields #169
Conversation
The original display shows raw/original strings as it is which is better than the sourcegraph which shows them as cooked strings. See: Original version: Sourcegraph version: Though it is cool that the Sourcegraph version also shows godoc. |
@junkblocker shouldn't you just peak definition for that? |
@keegancsmith, yes but that's irrelevant to comparing current vs new implementation hovertip views. |
@ramya-rao-a is the goal for us to be exactly the same as the older version? I personally prefer the new implementation for hover vs what vscode-go used to do (in fact I also want to get rid of the tags). But I'm happy to make it more like the old implementation if that is what makes sense. Also in future, if we get the signature helper to work on structs I suppose that would help the common reason people look at this info? |
I also prefer the new implementation. |
@keegancsmith , @ramya-rao-a - Can't this be a switch? I'd prefer to see both the uncooked display and the struct tags. |
There can always be cases where a feature implementation by the language server is different than that of what vscode-go has at the moment. It might also be better. So no, the goal is not to "exactly" match the existing behavior. But it should match existing needs. In the current case, having the docs squeezed in between the struct type and struct members does look a bit odd, but that's my personal opinion. I'd say, lets keep it as is, and wait for more feedback from users. This would also affect hover in source graph, do you (at source graph) have any preference? |
It would be nice to have a poll :) |
Making the struct tags raw strings is definitely better ( As for showing the doc synopsis (first sentence), Sourcegraph users generally do like seeing that in addition to the struct definition. (We are totally fine having different behavior on Sourcegraph vs. VSCode, of course.) The improvement we should make is to display the doc prose in a sans-serif font, not in monospace. That'll better distinguish the doc synopsis from the struct definition, and it'll make the doc synopsis take up less space. Feel free to upvote this if you like seeing doc synopses and downvote if you do not. 😄 |
@sqs Just to be clear, when you say "doc synopsis" you mean the first line and "doc prose" you mean the second correct? |
Actually, I meant those interchangeably, but I was mistaken. I thought we were truncating the doc string to the first sentence as is often done for Go (using https://golang.org/pkg/go/doc/#Synopsis). Apparently we are not anymore. My bad.
Anyway, my proposal was to show a sans-serif first sentence docstring only, plus the (monospace) type/struct definition. This way, you get most of the benefit of having the docstring without it taking up a lot of space. Seems better than no-docstring or full-docstring to me, and it follows Go convention.
But very happy to change it or allow configuration if people don't like it this way.
|
We should definitely include the doc string. No arguments there. Regarding the font issue, what is happening is the language server is returning a markdown content for the hover and not simple text. See go-langserver/langserver/hover.go Line 149 in 23a59cc
Simple text is shown in a smaller font (you can see the current behavior in VS Code), markdown for some reason appears bigger and different font. I'll take a look at that separately from VS Code side. Things left to debate/discuss/vote/poll are
|
For structs and interfaces we now include an extra hover content field containing the fields in the struct.
Fixes #168