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 insert-replace support (insertReplaceEdit) #40871

Open
muirdm opened this issue Aug 18, 2020 · 5 comments
Open

x/tools/gopls: add insert-replace support (insertReplaceEdit) #40871

muirdm opened this issue Aug 18, 2020 · 5 comments

Comments

@muirdm
Copy link

@muirdm muirdm commented Aug 18, 2020

Currently gopls is overwriting the surrounding suffix (in addition to prefix) when inserting candidates:

// completing to "food" currently results in "food" instead of "foodoo"
f<>oo

I think it's from https://go-review.googlesource.com/c/tools/+/208501 changing the surrounding's range from [ident.Pos(), c.pos] to [ident.Pos(), ident.End()].

/cc @heschik

@gopherbot gopherbot added this to the Unreleased milestone Aug 18, 2020
@stamblerre stamblerre removed this from the Unreleased milestone Aug 25, 2020
@stamblerre stamblerre added this to the gopls/v.0.5.0 milestone Aug 25, 2020
@stamblerre stamblerre removed this from the gopls/v.0.5.0 milestone Aug 26, 2020
@stamblerre stamblerre added this to the gopls/v1.0.0 milestone Aug 26, 2020
@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Sep 9, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@schellj
Copy link

@schellj schellj commented Sep 18, 2021

I'm not familiar with this codebase, but it appears that there might already be a commit relevant to a fix for this issue at https://go-review.googlesource.com/c/tools/+/196119/. There's also a related issue report that was frozen due to age at #34011.

@schellj
Copy link

@schellj schellj commented Sep 20, 2021

Also relevant to this behavior is the LSP client's insertReplaceSupport capability property (see https://microsoft.github.io/language-server-protocol/specification). If the client has that capability, the server is supposed to return two edit ranges; one for inserting the completion and one for replacing existing text with the completion.

@muirdm
Copy link
Author

@muirdm muirdm commented Sep 20, 2021

This issue regressed since it was fixed in https://go-review.googlesource.com/c/tools/+/196119/ .

insertReplaceSupport

Interesting. Adding support for that could be the best way forward since preferences differ.

@zikaeroh
Copy link
Contributor

@zikaeroh zikaeroh commented Sep 20, 2021

Selfishly, I still prefer the behavior I advocated for in #34011, so any way that preserves the behavior will make me happy. 🙂

@schellj
Copy link

@schellj schellj commented Sep 20, 2021

@zikaeroh I generally agree. However, it seems that according to the LSP spec, supporting insertReplaceSupport seems to be the way that the server would ideally resolve this preference between users (or even each given completion selection).

Sorry, I failed to read the second part of your comment 😊

@findleyr findleyr changed the title x/tools/gopls: completion insertion is overwriting suffix x/tools/gopls: add insert-replace support (insertReplaceEdit) Oct 4, 2021
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
6 participants