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: add type to the hover for types #41357

Closed
gertcuykens opened this issue Sep 12, 2020 · 6 comments
Closed

x/tools/gopls: add type to the hover for types #41357

gertcuykens opened this issue Sep 12, 2020 · 6 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@gertcuykens
Copy link
Contributor

gertcuykens commented Sep 12, 2020

For some reason tools beside go doc like to get rid of the keyword type for example, granted in most cases a experienced go developer instantly knows the symbol refers to a const, var, func or type. But in some cases especially this first one, it's very easily misunderstood that HandlerFunc is a type. For example the name contains Func and and the definition contains func, so it's very crustial to not leave out the word type in the beginning to not confuse people who are new to go. Yes type it's in the description at least but that just lucky somebody put it in the comment.

image

image

image

Fortunately func is always included, but that's not a excuse to drop type var const

image

go doc json for example is very consistent in that regard making sure to always start with func type const or var

image

@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 Sep 12, 2020
@gopherbot gopherbot added this to the Unreleased milestone Sep 12, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Sep 14, 2020
@stamblerre stamblerre added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 17, 2020
@pjweinb
Copy link

pjweinb commented Sep 17, 2020

To be clear, this looks like a request for clearer Hover messages. Is that right?

@gertcuykens
Copy link
Contributor Author

gertcuykens commented Sep 18, 2020

@pjweinb I think I described it a bit more precise then that :) In summary just don't remove type const var func in the beginning of a hover message. Thank you

example2:

go doc http.HandlerFunc

package http // import "net/http"
type HandlerFunc func(ResponseWriter, *Request)
The HandlerFunc...
func (f HandlerFunc) ServeHTTP(w ResponseWriter, r *Request)

in hover message you get

HandlerFunc func(ResponseWriter, *Request)
http.HandlerFunc on pkg.go.dev
The HandlerFunc....

A person new to go thinks this
HandlerFunc = func(ResponseWriter, *Request)
so that means i can do this
HandlerFunc(ResponseWriter, *Request)
definitely a function because I read
...Func and func...

compiler error message haha wrong its a type

actually even worse you get

too many arguments to conversion to http.HandlerFunc: http.HandlerFunc("a", "b")

https://play.golang.org/p/NfpIDUr6teT

New go user ??? ok I am going to stick to js instead, go is waaaay to confusing

All I want to say is leaving out type makes a huge difference for new go developers

HandlerFunc func(ResponseWriter, *Request)

type HandlerFunc func(ResponseWriter, *Request)

@stamblerre
Copy link
Contributor

I think the only case where a keyword is not present in the hover message is in the case of types, which is likely because of the way we format their hover. We can consider adding type to hover messages.

@stamblerre stamblerre changed the title x/tools/gopls: no consistency between const var func and types x/tools/gopls: add type to the hover for types Sep 21, 2020
@stamblerre stamblerre added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Sep 21, 2020
@nasjp
Copy link

nasjp commented Sep 30, 2020

I tried hovering in Goland and the 'type' is displayed.
It seems to be the problem with vscode-go, not gopls.

golang

@stamblerre
Copy link
Contributor

@nasjp: Goland does not use gopls, so this is a gopls issue.

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.5.2 Oct 7, 2020
@stamblerre stamblerre self-assigned this Oct 7, 2020
@gopherbot
Copy link

Change https://golang.org/cl/260006 mentions this issue: internal/lsp: add "type " to hover messages for structs/interfaces/etc

@golang golang locked and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants