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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 02 Nov 2024 13:56:35 +0200

Ping!  João, any comments to the patch or the issue in general?

> Cc: [email-masked]
> Date: Fri, 18 Oct 2024 08:56:32 +0300
> From: Eli Zaretskii <[email-masked]>
> 
> > From: Casey Banner <[email-masked]>
> > Date: Thu, 17 Oct 2024 20:54:38 -0400
> > 
> > Since 3.16, LSP supports the capability `insertReplaceSupport`. This
> > allows textEdit to be an `InsertReplaceEdit` see:
> > (https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertReplaceEdit)
> > 
> > This patch adds support for this capability, and uses the `replace`
> > field of the `InsertReplaceEdit`. Original functionality (ie.
> > `TextEdit`) is preserved.
> > 
> > The benefits of this were originally discussed here:
> > https://github.com/joaotavora/eglot/discussions/1456, but this is a summary:
> > 
> > Consider this file:
> > 
> > ```
> > const Foo = struct {
> >     correct_name: u32,
> > };
> > 
> > fn example(foo: Foo) u32 {
> >     return foo.correct_name;
> > }
> > ```
> > 
> > 1. Place the cursor on 6:22 (the _ in correct_name)
> > 2. Backspace once to delete the t
> > 3. Receive the following LSP message: `<-- textDocument/completion[20] {"jsonrpc":"2.0","id":20,"result":
> > {"isIncomplete":false,"items":[{"label":"correct_name","kind":5,"detail":"u32","documentation":
> > {"kind":"plaintext","value":""},"sortText":"2_correct_name","textEdit":{"range":{"start":
> > {"line":5,"character":15},"end":{"line":5,"character":21}},"newText":"correct_name"}}]}}`
> > 4. Accept the completion
> > 5. The buffer now contains `    return foo.correct_name_name;` on line 6
> > 
> > I expected it to replace the entire token, resulting in `    return foo.correct_name;`.
> > 
> > Indeed with this patch applied (and an LSP that supports the
> > capability), the behaviour I expected is now what happens.
> > 
> > This is the first real elisp that I've written besides configuration, so
> > I'm not sure if this is the correct way, but it seems to work for me.
> > 
> > Patch is attached.
> 
> Thanks.
> 
> João, any comments?
> 
> > From bbf79f95636d699ccf9ba7028e6c3dce23af2378 Mon Sep 17 00:00:00 2001
> > From: kcbanner <[email-masked]>
> > Date: Thu, 17 Oct 2024 00:43:32 -0400
> > Subject: [PATCH] * lisp/progmodes/eglot.el: add support for insertReplaceEdit
> > 
> > ---
> >  lisp/progmodes/eglot.el | 25 +++++++++++++++++++------
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> > index 0a14146a245..8285506928f 100644
> > --- a/lisp/progmodes/eglot.el
> > +++ b/lisp/progmodes/eglot.el
> > @@ -647,6 +647,7 @@ eglot--uri-path-allowed-chars
> >                        (:detail :deprecated :children))
> >        (TextDocumentEdit (:textDocument :edits) ())
> >        (TextEdit (:range :newText))
> > +      (InsertReplaceEdit (:newText :insert :replace))
> >        (VersionedTextDocumentIdentifier (:uri :version) ())
> >        (WorkDoneProgress (:kind) (:title :message :percentage :cancellable))
> >        (WorkspaceEdit () (:changes :documentChanges))
> > @@ -959,7 +960,8 @@ eglot-client-capabilities
> >                                                         ["documentation"
> >                                                          "details"
> >                                                          "additionalTextEdits"])
> > -                                      :tagSupport (:valueSet [1]))
> > +                                      :tagSupport (:valueSet [1])
> > +                                      :insertReplaceSupport t)
> >                                      :contextSupport t)
> >               :hover              (list :dynamicRegistration :json-false
> >                                         :contentFormat (eglot--accepted-formats))
> > @@ -3368,12 +3370,19 @@ eglot-completion-at-point
> >                          ;; state, _not_ the current "foo.bar".
> >                          (delete-region orig-pos (point))
> >                          (insert (substring bounds-string (- orig-pos (car bounds))))
> > -                        (eglot--dbind ((TextEdit) range newText) textEdit
> > -                          (pcase-let ((`(,beg . ,end)
> > +                        (eglot--dcase textEdit
> > +                          (((TextEdit) range newText)
> > +                           (pcase-let ((`(,beg . ,end)
> >                                         (eglot-range-region range)))
> >                              (delete-region beg end)
> >                              (goto-char beg)
> > -                            (funcall (or snippet-fn #'insert) newText))))
> > +                            (funcall (or snippet-fn #'insert) newText)))
> > +                          (((InsertReplaceEdit) newText replace)
> > +                           (pcase-let ((`(,beg . ,end)
> > +                                        (eglot-range-region replace)))
> > +                             (delete-region beg end)
> > +                             (goto-char beg)
> > +                             (funcall (or snippet-fn #'insert) newText)))))
> >                         (snippet-fn
> >                          ;; A snippet should be inserted, but using plain
> >                          ;; `insertText'.  This requires us to delete the
> > @@ -3602,8 +3611,12 @@ eglot--apply-text-edits
> >                            (replace-buffer-contents temp)))
> >                        (when reporter
> >                          (eglot--reporter-update reporter (cl-incf done))))))))
> > -            (mapcar (eglot--lambda ((TextEdit) range newText)
> > -                      (cons newText (eglot-range-region range 'markers)))
> > +            (mapcar (lambda (text-edit-or-insert-replace-edit)
> > +                      (eglot--dcase text-edit-or-insert-replace-edit
> > +                        (((TextEdit) range newText)
> > +                         (cons newText (eglot-range-region range 'markers)))
> > +                        (((InsertReplaceEdit) newText replace)
> > +                         (cons newText (eglot-range-region replace 'markers)))))
> >                      (reverse edits)))
> >        (undo-amalgamate-change-group change-group)
> >        (when reporter
> > -- 
> > 2.46.0.windows.1
> > 
> 
> 
> 
>