Re: Eglot cannot work with default completion-in-region?

[sbaugh]

João Távora <joaotavora@gmail.com> writes:
>> So Eglot has this somewhat hacky code which runs in :exit-function to
>> delete the completion after completion-in-region inserts it, and insert
>> a different string instead:
>>
>> (cond (textEdit
>>        ;; Revert buffer back to state when the edit
>>        ;; was obtained from server. If a `proxy'
>>        ;; "bar" was obtained from a buffer with
>>        ;; "foo.b", the LSP edit applies to that
>>        ;; 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-range-region range)))
>>            (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
>>        ;; whole completion, since `insertText' is the full
>>        ;; completion's text.
>>        (delete-region (- (point) (length proxy)) (point))
>>        (funcall snippet-fn (or insertText label))))
>>
>> This is code which is often broken, especially with default
>> completion-in-region.
>
> If you know of broken cases, contact bug-gnu-emacs@gnu.org and
> provide a full reproducible error recipe according to Eglot's
> manual (clangd, pyright, or rust-analyzer, gopls preferred,
> other servers must come with installation instructions.  If
> installation too complex/bloaty it may take a while.)
>
>> However, the common case is that sortText==insertText/textEdit.
>
> Is it?  The above all return textEdits, I think.  How can sortText,
> a JSON string, equal textEdit, a JSON object?

If textEdit.range==the completion region and textEdit.newText==sortText,
then the textEdit is trivial and the default completion insertion
behavior is correct and needs no special handling.  So we can skip the
textEdit behavior in the exit-function.

>> In that case, this code is not necessary, and Eglot doesn't need an
>> :exit-function at all.
>
> Indeed.  But how do you plan to know this in advance?

And if for all the completion items, we see that we can skip the
textEdit behavior, then we don't need an :exit-function at all.

>> Can Eglot detect this and avoid this somewhat hacky code in that case?
>
> Not easily or cleanly...

What's the issue with the approach I described above?  Seems like it
would work?

>> It should be a performance improvement and simplification anyway for all
>> completion frameworks.
>
> .. but you can try your hand at a patch.  I'd love to be proven wrong.
>
>> (BTW, besides the issue with default completion-in-region, I also ran
>> into this while trying to write an OCaml-specific wrapper for
>> eglot-completion-at-point function which adds some additional
>> completions to those returned from the LSP.  But if those completions
>> are chosen, then the Eglot exit-function breaks when it tries to look up
>> the insertText/textEdit.  My LSP doesn't use textEdit at all, though, so
>> this is another unnecessary breakage)
>
> What contract exactly is eglot-completion-at-point breaking?

No contract, my modification isn't staying within the bounds of any
explicit contract.  So of course I shouldn't expect it to work.  But
nevertheless I'd like to get it to work, and the exit-function is what
doesn't work, so I'd like to figure out a way to make it work.

We could add a new extra property to completion-at-point-functions,
:text-function, which gets the completion item after it's been selected,
before text properties are stripped, and returns the actual text which
should be inserted into the buffer.

Then Eglot could have a text-function which would hopefully handle most
insertText/textEdit cases (any insertText and any textEdit where the
range is the completion region).  If a completion item requires any
additional changes, the text-function could communicate that to Eglot
somehow and then Eglot could do it in exit-function.

That would be useful to Eglot, right?  It would let Eglot avoid some
hacks to work around the fact that default completion-in-region strips
the text properties before calling exit-function.

I would also find this useful elsewhere too - the fact that
exit-function strips the properties is quite annoying, and the ability
to transform the completion (possibly preserving text properties when
inserted) would be handy.

And then my wrapper around eglot-completion-at-point could just provide
its own text-function which calls Eglot's text-function whenever it sees
an Eglot completion, and runs its own logic on its own completions, and
avoid any breakage.