Subject: Re: bug#73857: [PATCH] * lisp/progmodes/eglot.el: add support for insertReplaceEdit

From: João Távora <[email-masked]>
Date: Thu, 13 Feb 2025 10:21:19 +0000

I've added a variation on Casey's patch, perhaps I forgot to send
email to close this bug:

commit 1143cf09a339d57051a4341103c9e342d8876649
Author: João Távora <[email-masked]>
Date:   Mon Jan 20 18:58:05 2025 +0000

    Eglot: add support for insertReplaceEdit (bug#73857)

    * lisp/progmodes/eglot.el (eglot-server-programs): Mention zig-ts-mode.
    (eglot--lsp-interface-alist): Describe 'InsertReplaceEdit'.
    (eglot-client-capabilities): Advertise 'insertReplaceSupport'.
    (eglot-completion-at-point): Consider 'InsertReplaceEdit'.
    (eglot--apply-text-edits): Consider 'InsertReplaceEdit'.

    * test/lisp/progmodes/eglot-tests.el
    (eglot-test-zig-insert-replace-completion): New test.

    Special thanks to [email-masked]

Added a zig test, too.

On Thu, Feb 13, 2025 at 10:18 AM Stefan Kangas <[email-masked]> wrote:
>
> Dmitry Gutov <[email-masked]> writes:
>
> > Hi!
> >
> > On 02/11/2024 15:22, João Távora wrote:
> >> First, the patch looks good. I haven't been able to test it, but it
> >> looks good, very good even.
> >> The issue seems to be valid, and I've confirmed it, but I don't
> >> fully understand it.  Here, I find it somewhat dissatisfying that
> >> what would  seem to be a client-side issue (i.e. a bug in Eglot) should
> >> be "fixed" by adding a feature so as to somehow circumvent the
> >> problem.
> >> But another possible explanation is that the issue is not a bug in Eglot
> >> but a bug in certain servers (such as rust-analyzer) which given
> >> the reasonable/common absence of a capability in the client (the one
> >> that's being proposed by Casey) utilizes Eglot's current capabilities
> >> in a suboptimal way that triggers a problem.
> >> In that case -- which would have to be confirmed -- fixing the server
> >> or adding the capability to the client are two equally valid alternatives
> >> to solve this.  We seem to have the latter in the patch.
> >> When I find the time I will try to test this patch, unless someone
> >> beats me to it with a convincing test (even better a convincing
> >> automated test in eglot-tests.el).  Such a person could be
> >> Dmitry, who has worked on Eglot completion logic before.  I've
> >> added him to CC.
> >
> > I agree that such a change should come with automated test(s). Happy to provide
> > pointers, or look at writing that myself in a little while.
> >
> > Speaking of client-side issue, it might as well be. Or it is possible that our
> > original expectations were wrong, and the protocol expected the suffix to never
> > be replaced using the original convention. Now the next extension allows us to
> > ask the servers to do that instead.
> >
> > Regarding capabilities, though, it seems the editors that support the extension,
> > so far, all include the ability to configure which of the behaviors (insert or
> > replace) will be used, with a way to invoke the non-default behavior using a
> > combination like Shift+Enter.
> >
> > Is that something we want? Or, to put it differently, are we sure that the
> > "insert" behavior won't be preferred at least in some contexts (only determined
> > by the user)?
> >
> > The protocol doc says this:
> >
> >   * Most editors support two different operations when accepting a completion
> >   * item. One is to insert a completion text and the other is to replace an
> >   * existing text with a completion text. Since this can usually not be
> >   * predetermined by a server it can report both ranges.
> >
> > Examples of configurability:
> >
> >   * VS Code:
> >     https://github.com/microsoft/language-server-protocol/issues/1260#issuecomment-833014788
> >     (link to a video in a comment)
> >   * Sublime: https://github.com/sublimelsp/LSP/pull/1809
> >   * [dbaeumer about Eclipse]:
> >     https://github.com/microsoft/language-server-protocol/issues/1260#issuecomment-836775280
> >
> > I'm not sure which method should be used in our completion UI to say "flip
> > default this time", but maybe a user option would help.
>
> Casey, I see that there were several followups to your proposed patch.
>
> Did you make any progress with it?  Thanks in advance.

-- 
João Távora