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: support fill struct behavior #37576

Open
frioux opened this issue Feb 28, 2020 · 9 comments
Open

x/tools/gopls: support fill struct behavior #37576

frioux opened this issue Feb 28, 2020 · 9 comments

Comments

@frioux
Copy link

@frioux frioux commented Feb 28, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

n/a (feature request)

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/frew/.cache/go-build"
GOENV="/home/frew/.config/go/env"
GOEXE=""
GOFLAGS="-mod=readonly"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/frew/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org"
GOROOT="/home/frew/sdk/go1.14"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/frew/sdk/go1.14/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/run/shm/go-build445574914=/tmp/go-build -gno-record-gcc-switches"

What did you do?

n/a (feature request)

What did you expect to see?

n/a (feature request)

What did you see instead?

n/a (feature request)

Feature request

vim-go had a command called :GoFillStruct and it would transform (for example), this:

x := Person{}

into

x := Person{
	Name: "",
	Employed: false,
	Age: 0,
}

Basically it'd set all the fields to their zero value. In govim/govim#709 I was told that this needs to be added to gopls first, so here I am :)

@gopherbot gopherbot added this to the Unreleased milestone Feb 28, 2020
@stamblerre stamblerre changed the title x/tools/gopls: feature Request: fill struct x/tools/gopls: support fill struct behavior Mar 2, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v1.0.0 Mar 2, 2020
@golang golang deleted a comment from gopherbot Mar 2, 2020
@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Mar 2, 2020

Thanks for the issue report! I agree that we should support this feature.

@muirdm: Do you think we could fit this behavior into completion?

@muirdm

This comment has been minimized.

Copy link

@muirdm muirdm commented Mar 2, 2020

For my understanding, what is the use case for this?

@stamblerre

This comment has been minimized.

Copy link
Contributor

@stamblerre stamblerre commented Mar 2, 2020

I don't know if it's particularly useful for the Person case described above, but I would find it useful if the struct contained map values. (My classic example is go/types.Info, which I always find super annoying to create.) My ideal scenario when completing that struct is that, if I have a value of the correct type in scope, it should be used -- otherwise, we should make a map in the struct declaration.

@muirdm

This comment has been minimized.

Copy link

@muirdm muirdm commented Mar 2, 2020

The zero value of map is "nil", so this proposal as written wouldn't help in the types.Info case.

In my experience having the literal completion that gives you a candidate for make(map[some.Long]annoying.Map) is already a major improvement. Having structs get automatically filled in with in-scope objects sounds hard to avoid false positives.

@frioux

This comment has been minimized.

Copy link
Author

@frioux frioux commented Mar 2, 2020

@muirdm

This comment has been minimized.

Copy link

@muirdm muirdm commented Mar 3, 2020

I can see how that could be part of your workflow, but it also seems to overlap with existing facilities such as textDocument/hover which will show you all the struct fields or just a normal completion request in the struct literal, which also will list all the struct fields and their types.

To answer the original question of fitting this into completion, we could but I'm not sure what would trigger the completion or how it would show up clearly to the user. Maybe better as a code action?

@frioux

This comment has been minimized.

Copy link
Author

@frioux frioux commented Mar 3, 2020

I don't know what a code action is, but in the original setting it was something the user would explicitly request, rather than fitting into the same workflow as method completion. I personally think that's fine; this is something I'd do a few times a day, not constantly, like method completion.

1 similar comment
@frioux

This comment has been minimized.

Copy link
Author

@frioux frioux commented Mar 3, 2020

I don't know what a code action is, but in the original setting it was something the user would explicitly request, rather than fitting into the same workflow as method completion. I personally think that's fine; this is something I'd do a few times a day, not constantly, like method completion.

@leitzler

This comment has been minimized.

Copy link
Contributor

@leitzler leitzler commented Mar 6, 2020

I support the codeAction suggestion, seems like it would be a better fit (maybe of kind refactor.rewrite.fillstruct).

As a feature this is something that, at least I, hear people ask for quite often. With the use case to get the field names quickly added when creating a new struct instance. Hover does show the same info, but it would be really helpful if gopls could add it for you so that you don't have to add them manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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