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: usePlaceholders=false disables variable name suggestions when using the var! suffix completion #69626

Open
urandom opened this issue Sep 25, 2024 · 4 comments
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@urandom
Copy link

urandom commented Sep 25, 2024

gopls version

golang.org/x/tools/gopls v0.16.1

go env

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/urandom/.cache/go-build'
GOENV='/home/urandom/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/urandom/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/urandom/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/urandom/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4155953414=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I've disabled the usePlaceholders option, since having it enabled inserts invalid syntax, causing more work.

Then I attempted to complete a .var! suffix completion on a function call, in order to have the variables defined.

What did you see happen?

, := theFunctionThatWasCompleted()

What did you expect to see?

someVar, err := theFunctionThatWasCompleted()

Editor and settings

Zed:

  "lsp": {
    "gopls": {
      "initialization_options": {
        "usePlaceholders": false
      }
    }
  },

Logs

No response

@urandom urandom added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Sep 25, 2024
@gopherbot gopherbot added this to the Unreleased milestone Sep 25, 2024
@findleyr
Copy link
Member

Thanks for the report. This sounds like a bug in postfix completions.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.18.0 Sep 26, 2024
@muirdm
Copy link

muirdm commented Sep 28, 2024

Seems like it is this line. I don't think we have ever really established snippet best practices when placeholders are disabled. I guess it depends on how good the default placeholder values are. If we expect the user to normally want the default values, we should insert them (with no placeholder). If we expect the user to want to replace them, we should omit them and leave the cursor there so the user can type.

Personally I think we should address the usability issues with placeholders and get away from supporting a global no-placeholder mode for snippets. Turning off placeholders just doesn't make sense for most snippets. If there are still contentious uses of placeholders (e.g. func call params), we introduce specific config flags to control those individual uses.

@urandom
Copy link
Author

urandom commented Sep 30, 2024

Personally I think we should address the usability issues with placeholders and get away from supporting a global no-placeholder mode for snippets. Turning off placeholders just doesn't make sense for most snippets. If there are still contentious uses of placeholders (e.g. func call params), we introduce specific config flags to control those individual uses.

Turning off placeholders is currently more usable than having them on, at least for me. The placeholder completion results in code that is not syntactically correct - for function calls, it contains both argument names and their types. This causes a lot more work for people like me that are not used to snippet systems and their key bindings. More often than not, I end up having to manually delete the inserted types because I somehow "exited" the snippet. Perhaps if the placeholders being returned didn't contain the actual types, that would appease clumsy people like me as well. However, with signature help on, there's really no reason for me to have the placeholders in signatures in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. 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