x/tools/gopls: usePlaceholders=false disables variable name suggestions when using the var! suffix completion

urandom commented 1 month ago

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

gabyhelp commented 1 month ago

Related Issues and Documentation

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

findleyr commented 1 month ago

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

muirdm commented 1 month ago

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 commented 1 month ago

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.

golang / go