x/tools/gopls: Revert the changes from the last refactor.

[tttoad]

gopls version

golang.org/x/tools/gopls v0.16.2
golang.org/x/tools/gopls@v0.16.2 h1:K1z03MlikHfaMTtG01cUeL5FAOTJnITuNe0TWOcg8tM=
github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW
6Y=
golang.org/x/mod@v0.20.0 h1:utOm6MM3R3dnawAiJgn0y+xvuYRsm1RKM/4giyfDgV0=
golang.org/x/sync@v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
golang.org/x/telemetry@v0.0.0-20240829154258-f29ab539cc98 h1:Wm3cG5X6sZ0RSVRc/H1/sciC4AT6HAKgLCSH2lbpR/c=
golang.org/x/text@v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4=
golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653 h1:6bJEg2w2kUHWlfdJaESYsmNfI1LKAZQi6zCa7LUn7eI=
golang.org/x/vuln@v1.0.4 h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I=
honnef.co/go/tools@v0.4.7 h1:9MDAWxMoSnB6QoSqiVr7P5mtkT9pOc1kSxchzPCnqJs=
mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo=
mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.23.1

go env

GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/toad/Library/Caches/go-build'
GOENV='/Users/toad/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/toad/work/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/toad/work'
GOPRIVATE=''
GOPROXY='https://goproxy.cn,direct'
GOROOT='/Users/toad/go/go1.23.1/'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/toad/go/go1.23.1/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/toad/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/toad/work/demo10/go.mod'
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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-pref
ix-map=/var/folders/g1/tgmnlrdn3vxgv08kdgh9vpkw0000gn/T/go-build4008461701=/tmp/go-build -gno-record-gcc-switch
es -fno-common'

What did you do?

package a
func S(aa, bb int) { // Remove useless parameters bb.
}
---------------------
package b

import (
	"demo/a"
)

func B() {
	a.S(1, 2)
}

When the parameter b is removed, I want to revert.
Change multiple files, I need to switch to multiple files to undo.

What did you see happen?

No "CodeAction".

What did you expect to see?

Revert the last change.

Editor and settings

No response

Logs

No response

[gabyhelp]

Related Issues

• x/tools/gopls: Add refactor "move to a new file" #65707 (closed)
• x/tools/gopls: Implement action should ignore importing the current package #61830 (closed)
• x/tools/gopls: disable code actions from gopls #68783 (closed)
• x/tools/gopls: Show specific file name when renaming failed. #69563 (closed)
• x/tools/gopls: refactor.rewrite.removeUnusedParam: avoid literalization for recursive spread call #70554
• x/tools/gopls: diagnostics from modules outside `go.work` stick around #48929 (closed)
• x/tools/gopls: "Split arguments into separate lines" loses variadic ellipsis #70519 (closed)
• x/tools/gopls: negated build flag causes error #59346
• x/tools/gopls: gopls doc viewer failed with "package not found" error #70453 (closed)
• x/tools/gopls: invalid errors when using "pkgm" VS Code snippet #39296 (closed)

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

[adonovan]

The applyWorkspaceEdit operation has a label field, documented thus:

	/**
	 * An optional label of the workspace edit. This label is
	 * presented in the user interface for example on an undo
	 * stack to undo the workspace edit.
	 */
	label?: string;

This seems to imply that it is the client's responsibility to provide an undo operation at a higher level of operation than the individual DocumentChanges. Gopls does not set this field, and it probably should. I did a quick test with it set to an arbitrary value and found that VS Code implements high-level undo (but doesn't mention the label in the UI, so it may be irrelevant), and Emacs+eglot does not; I don't know to what extent this feature is supported by other clients.

[xzbdmw]

This can be achieved by bufdo in nvim, a smarter plugin can do it better:

iShot_2024-11-30_06.45.36.mp4

[findleyr]

This would probably be surprisingly tricky to implement. From gopls' perspective, we just ask the client to apply an edit (or return the edit from an operation), and then may or may not receive subsequent didChange notifications. Each of these notifications is for a different file, and so for edits spanning multiple files there's no way to know when all changes are done.

We could theoretically just collect edits following a change, and as long as they match edits we recently sent, hold open a code action to revert the edits. Even that is pretty hacky though, and furthermore there's no reason that clients have to send us exactly the edits we sent: there are many different ways to represent the same logical edit. So we'd have to apply those edits and then rederive our edit representation to see if it matches.

So we'd have to do all that work, and we'd have to be really accurate in order to make this feature reliable. And if the feature misbehaved, it would be very confusing for our users.

The reason this is all so awkward is this: the LSP is designed around the principle that clients control state and lifecycles of operations. Exposing a 'revert' feature from gopls is in direct opposition to this principle.

As much as I'd appreciate such a feature. I don't think we can reasonable do this. This must be implemented by clients.