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

Open
gertcuykens opened this issue Sep 12, 2020 · 3 comments
Open

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

gertcuykens opened this issue Sep 12, 2020 · 3 comments

Comments

@gertcuykens
Copy link
Contributor

@gertcuykens 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 this to the Unreleased milestone Sep 12, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Sep 14, 2020
@stamblerre stamblerre added this to Bugs in gopls/v1.0.0 Sep 16, 2020
@pjweinb
Copy link

@pjweinb 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 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

@stamblerre stamblerre commented Sep 21, 2020

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 and removed NeedsDecision labels Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.