-
Notifications
You must be signed in to change notification settings - Fork 17.5k
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: add method documentation & jump to page source to hovered type. #66721
Comments
Change https://go.dev/cl/577235 mentions this issue: |
The change also includes a jump to definition feature for all methods.The change is quite fail-safe, such that if it fails in any way, it will still display the hover info without the method docs(as before). The change is localised to the the gopls/internal/golang/hover.go file, in function hover and formatHover. I should also mention that it does not alter the method displayed or it order, it just add the method docs (if any) and a jump to page source url to the hover info. Fixes golang/go#66721 Fixes golang/go#66721 Signed-off-by: tobigiwa <giwaoluwatobi@gmail.com>
Change https://go.dev/cl/577356 mentions this issue: |
I suspect this change could make the hover popup very long indeed. Also, one typically needs just a brief reminder of which methods are available on the type. If you need to study the methods more closely, you can jump to their definition, or (in the forthcoming release) open the documentation for the type. |
P.S. thanks for sending a potential fix, but in the future it's really worth discussing the desired behavior before starting to write code. |
@adonovan I was only looking at the slack channel for response on this, I sincerely did'nt notice you have said something here 4 days ago. Thanks for taking the time. Regarding your earlier comment, I can summarize your concerns into 2: Performance and Utility, correct?. Regarding performance, I did not notice any difference (from my computer here, which is entirely not enough 😆), but if there is a way to benchmark or show something tangible to convince you instead, please let me know. In the case of it utility, I actually borrowed the feature from a popular Go IDE (you know which, refrainning from naming), which is quite helpful and useful for me (also not enough, lot of people use Gopls). If it utility, I cannot argue for much I'll suppose. |
If it only performance, please let me know how to go about it.
|
My concern was only about the user experience; I don't expect performance to be a problem. The complete documentation for all the methods of a type seems like a lot of information to show in a popup. Perhaps you could attach a screenshot of the feature in the "popular IDE" that might demonstrate the desired effect. You might also be interested in a new feature in the upcoming gopls/v0.16.0 release: an integrated viewer for Go package documentation; see https://cs.opensource.google/go/x/tools/+/master:gopls/doc/release/v0.16.0.md. |
Just looked at the The method names are actually url (obvious when you hover, shows the underline), when you click them... The full type popup tears down then this 👆, to get back, you just press the type name in the receiver. Very neat, but this would require HTML, which is sanitized in vscode-go extension (I actually discovered so, I have not looked at the extension code, I actually did... did'nt figure much of it). My plan was to use the details tag of markdown, but it still html, sanitized too. It looks like this;
func (b *Builder) copyCheck() The docs then goes here. |
I don't know why it showing the copy thing... guessing Github, like I mentioned, I could not test this out, since the html was sanitized. |
Re-opening so we can discuss this in our triage meeting. |
Oh...this is good, I did see the word "triage" in the contribution guide ( I did'nt keep in my head, I'll look it up again), I had another thought about implementing what the "popular IDE" has. As the LSP specs dictates, the return from Hover can be a I just want to mention this too. |
I think we're very limited in what we can do in a hover popup, and the current content was chosen to be as dense as possible while retaining readability. A documentation viewer (forthcoming in gopls@v0.16.0) is a better place to view documentation. |
This is sad 😔🙁 |
gopls version
Build info ---------- golang.org/x/tools/gopls v0.15.2 golang.org/x/tools/gopls@v0.15.2 h1:4JKt4inO8JaFW3l/Fh9X1k/5JQn+iUOpdc4/Lpi0mOs= github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y= golang.org/x/mod@v0.15.0 h1:SernR4v+D55NyBH2QiEQrlBAnj1ECL6AGrA5+dPaMY8= golang.org/x/sync@v0.6.0 h1:5BMeUDZ7vkXGfEr1x9B4bRcTH4lpkTkpdh0T/J+qjbQ= golang.org/x/telemetry@v0.0.0-20240209200032-7b892fcb8a78 h1:vcVnuftN4J4UKLRcgetjzfU9FjjgXUUYUc3JhFplgV4= golang.org/x/text@v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/tools@v0.18.1-0.20240311201521-78fbdeb61842 h1:No0LMXYFkp3j4oEsPdtY8LUQz33gu79Rm9DE+izMeGQ= golang.org/x/vuln@v1.0.1 h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU= honnef.co/go/tools@v0.4.6 h1:oFEHCKeID7to/3autwsWfnuv69j3NsfcXbvJKuIcep8= mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo= mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8= go: go1.22.1
go env
What did you do?
No error, just feature request.
What did you see happen?
Hover on user defined type, or any type with method just shows method signature only. Most of the time, I'll like to know what these method do and be able to jump into them easily.
What did you expect to see?
Method docs and jump to page source in hover popup.
Editor and settings
Nothing extra.
Logs
No response
The text was updated successfully, but these errors were encountered: