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

[João Távora]

On Fri, Jan 26, 2024 at 8:38 PM Spencer Baugh <sbaugh@janestreet.com> wrote:

> The following message is a courtesy copy of an article
> that has been posted to gmane.emacs.devel as well.

I suppose this email will appear in emacs-devel soon, might
as well start answering.

> The recent commit d376462c7183752bf44b9bd20bf5020fe7eaf75a prompted by
> issue https://github.com/joaotavora/eglot/issues/1339 says:
>
> >I declare it impossible to make C-M-i use of 'try-completion' behave
> >sanely with LSP in its current state.  YMMV.  Use a completion tooltip,
> >like Company.
>
> So completion from Eglot, which is built into Emacs, is broken out of
> the box?

No.

> It has a hard dependency on using company-mode or similar
> third-party packages?

No.

> If this is the case, perhaps Eglot should make it more clear that it
> cannot be used without also installing and using company-mode or some
> other package.  Perhaps it should abort rather than running if the user
> is using the default completion-in-region.

This would be nonsense, given the minute dimension of the problem.

> Alternatively, as someone who uses default completion-in-region and
> would like to use Eglot, is there some way to make the common case
> behave correctly?

_Your_ common case is not other's. I'd just use it and take note where
it breaks down.  AFAIK it's _partial_ completion doesn't work as you
would expect from simpler completion models that only serve
strings.  A structured survey of cases using a variety of LSPs
would be welcome, if you're inclined to contribute this work.

I just fixed a small minor bug in eglot--dumb-tryc, by
the way, so make sure you grab latest Eglot.

> I guess the issue is that in the LSP protocol, there's a difference
> between the "sortText" and "filterText" which are used for displaying
> completions and letting the user choose them, and the
> "insertText"/"textEdit" which are used for inserting them.

Not the only reason, but that's one of them, yes.

> 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?

> 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?

> Can Eglot detect this and avoid this somewhat hacky code in that case?

Not easily or cleanly...

> 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?

João