Eglot cannot work with default completion-in-region?

Eglot cannot work with default completion-in-region?

[Spencer Baugh]

Hi,

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?  It has a hard dependency on using company-mode or similar
third-party packages?

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.

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?

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.

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.

However, the common case is that sortText==insertText/textEdit.  In that
case, this code is not necessary, and Eglot doesn't need an
:exit-function at all.

Can Eglot detect this and avoid this somewhat hacky code in that case?
It should be a performance improvement and simplification anyway for all
completion frameworks.

(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)

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

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.

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

[Daniel Mendler via Emacs development discussions.]

sbaugh@catern.com writes:

[96 lines...]

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

FWIW both Company and Corfu preserve text properties when calling the
exit function as long as the completion candidate is unique or was
selected explicitly in the popup menu.

Duplicate candidates (with respect to equal) are kept in the
Corfu/Company popup menus. Duplicates are also kept in the Completions
buffer as long as their prefix or suffix annotations are distinct, but
unfortunately the distinction is lost, text properties are stripped, as
soon as a candidate is selected.

Daniel

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

[João Távora]

sbaugh@catern.com writes:

[Spencer, if you mail user agent has a CC option, please use it to in my
CC, otherwise, you make finding these replies harder for me.  This is
very common practice in this list. ]

> João Távora <joaotavora@gmail.com> writes:
>> 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.

Couple of points:

* How do you know that is actually the "common" case?  For example, it's
  not uncommon at all to have some language servers provide completions
  for symbols which also add in an '#include', or an 'import' when you
  choose it (according to each languages module system, of course).

* Are you familiar with how LSP expresses edits?  The very same edit can
  be expressed in a multitude of ways.  Ultimately, the only way in
  which an edit can be determined to be trivial is by applying it and
  seeing it if was trivial.

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

Indeed "for all".  So this would mean iterating the full completion list
for non-trivial computation.  Even if that's manageable, are you aware
that some completion lists are incomplete at first and then are resolved
to complete lists as the completion session progresses?

Also, I hope that you are aware that LSP fundamentally disrepects any
Emacs completion style.  Because unless you can somehow teach Elisp to
arbitrary language servers on the spot, they really have no idea what
"flex", "partial-completion", "emacs22" or "orderless" is.

I consider this to be a much, much greater nuisance than the partial
completion limitations that you picked to start this thread. 

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

The only way to know is to try it.  And I don't envision easy or clean
stemming from it.  But if you have this approach very clear in your
head and demonstrate that:

* it's fast, or fast enough
* it significantly improves the completion experience compared to what
  it is now
* it doesn't break any existing use cases
* it doesn't add an unmanageable amount of complexity to the already
  complex function

Then I'll be willing to consider it, of course.

But for me, this is too tall an order.  I just don't see the benefit.

Anyway, as I said before, I think the starting point for this would have
to be a systematic structured survey of how Eglot behaves currently in a
number of language servers (including yours, of course, but not limited
to it) and encode that as unit tests for Eglot (failing or passing).

That effort is very valuable in itself and it is a prerequisite for any
deep changes to that function.  You can have a look at the exiting
'eglot-test-basic-completions' and 'eglot-test-non-unique-completions'
tests.  I'd welcome many more such tests.

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

Maybe you can write an OCaml-server specific LSP-aware completion
function for your OCaml major mode using Eglot's API.  I think that's
what makes the most sense.  You'll be able to know for sure what the
server does.  

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

That's something to pitch to Stefan.  But that API is pretty hairy as it
is.

> 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 don't know exactly what you're talking about or what this loss of
text-propeties actually entails.  If there's a bug, I'd like to know
about it, but I should be able to reproduce it.

For things to be "useful for Eglot" they have to translate to actual
tangible benefits for Eglot users in some server/major-mode combination
(and no bugs)

"Eglot users" here means user of:

  Emacs -Q some-file.xyz -f eglot

Where "xyz" is potentially _any_ file type managed by _any_ LSP server,
past or future.

It doesn't mean users of:

  Emacs -Q -l my-special-library-or-configuration.el some-file.ocaml -f eglot

That's _not_ to say the latter isn't useful for some, or even many,
users.  I just don't count that as "useful for Eglot".

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

I think you should try your hand at an OCaml-speciifc LSP-ware
completion function.  If the Elisp API of Eglot isn't sufficiently
flexible, we can make it more flexible.

João

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

[Spencer Baugh]

João Távora <joaotavora@gmail.com> writes:
> sbaugh@catern.com writes:
>> João Távora <joaotavora@gmail.com> writes:
>>> 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.
>
> Couple of points:
>
> * How do you know that is actually the "common" case?  For example, it's
>   not uncommon at all to have some language servers provide completions
>   for symbols which also add in an '#include', or an 'import' when you
>   choose it (according to each languages module system, of course).

Sure, but most completions are not going to add an import.  That's why I
say this is the common case.  Of course I don't actually know, so I do
agree with your argument to add more tests for some other LSP servers,
to see what their behavior is in practice.

> * Are you familiar with how LSP expresses edits?  The very same edit can
>   be expressed in a multitude of ways.  Ultimately, the only way in
>   which an edit can be determined to be trivial is by applying it and
>   seeing it if was trivial.

Yes.  But a one-element list with the values I described above is pretty
easy to check for.

>>>> 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.
>
> Indeed "for all".  So this would mean iterating the full completion list
> for non-trivial computation.  Even if that's manageable, are you aware
> that some completion lists are incomplete at first and then are resolved
> to complete lists as the completion session progresses?

Ok, so maybe :exit-function is still needed, but it can be a no-op most
of the time.

> Also, I hope that you are aware that LSP fundamentally disrepects any
> Emacs completion style.  Because unless you can somehow teach Elisp to
> arbitrary language servers on the spot, they really have no idea what
> "flex", "partial-completion", "emacs22" or "orderless" is.
>
> I consider this to be a much, much greater nuisance than the partial
> completion limitations that you picked to start this thread. 

Of course.  That is indeed a significant annoyance.

But to be somewhat provocative: elisp-completion-at-point also doesn't
know anything about completion styles, yet completion-styles still
affect completion with elisp-completion-at-point.

>>>> 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?
>
> The only way to know is to try it.  And I don't envision easy or clean
> stemming from it.  But if you have this approach very clear in your
> head and demonstrate that:
>
> * it's fast, or fast enough
> * it significantly improves the completion experience compared to what
>   it is now
> * it doesn't break any existing use cases
> * it doesn't add an unmanageable amount of complexity to the already
>   complex function
>
> Then I'll be willing to consider it, of course.

I will try to meet these requirements.

> But for me, this is too tall an order.  I just don't see the benefit.
>
> Anyway, as I said before, I think the starting point for this would have
> to be a systematic structured survey of how Eglot behaves currently in a
> number of language servers (including yours, of course, but not limited
> to it) and encode that as unit tests for Eglot (failing or passing).
>
> That effort is very valuable in itself and it is a prerequisite for any
> deep changes to that function.  You can have a look at the exiting
> 'eglot-test-basic-completions' and 'eglot-test-non-unique-completions'
> tests.  I'd welcome many more such tests.

Reasonable.

I can add some tests of completion with ocamllsp, rust-analyzer, and
pyright.

Do these tests run with actual language servers in any CI system?  If
so, should I make sure that the tests I add, work in that CI system?

>>>> 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.
>
> Maybe you can write an OCaml-server specific LSP-aware completion
> function for your OCaml major mode using Eglot's API.  I think that's
> what makes the most sense.  You'll be able to know for sure what the
> server does.  

Maybe.

>> 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.
>
> That's something to pitch to Stefan.  But that API is pretty hairy as it
> is.
>
>> 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 don't know exactly what you're talking about or what this loss of
> text-propeties actually entails.  If there's a bug, I'd like to know
> about it, but I should be able to reproduce it.

I mean the behavior of default completion-in-region that this code in
Eglot is working around:

;; When selecting from the *Completions*
;; buffer, `proxy' won't have any properties.
;; A lookup should fix that (github#148)
(get-text-property
 0 'eglot--lsp-item
 (cl-find proxy (funcall proxies) :test #'string=))

:text-function would let you delete this code since you could rely the
text properties being present.

> For things to be "useful for Eglot" they have to translate to actual
> tangible benefits for Eglot users in some server/major-mode combination
> (and no bugs)
>
> "Eglot users" here means user of:
>
>   Emacs -Q some-file.xyz -f eglot
>
> Where "xyz" is potentially _any_ file type managed by _any_ LSP server,
> past or future.
>
> It doesn't mean users of:
>
>   Emacs -Q -l my-special-library-or-configuration.el some-file.ocaml -f eglot
>
> That's _not_ to say the latter isn't useful for some, or even many,
> users.  I just don't count that as "useful for Eglot".

Of course.

I intend the benefit to be better support for try-completion for any
file type and any LSP server.  And also simplification of the code.

>> 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.
>
> I think you should try your hand at an OCaml-speciifc LSP-ware
> completion function.  If the Elisp API of Eglot isn't sufficiently
> flexible, we can make it more flexible.
>
> João

eglot-completion-at-point is a 200 line quite hairy function.  I don't
really want to duplicate its code.  If Eglot exposed parts of eglot-cap
as individual APIs, that would work too, but I think the capf API is a
reasonable way for Eglot to do that.  It's just that the current capf
API doesn't provide enough information to use eglot-cap without breaking
it.

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

[João Távora]

Spencer Baugh <sbaugh@janestreet.com> writes:

>> * Are you familiar with how LSP expresses edits?  The very same edit can
>>   be expressed in a multitude of ways.  Ultimately, the only way in
>>   which an edit can be determined to be trivial is by applying it and
>>   seeing it if was trivial.
>
> Yes.  But a one-element list with the values I described above is pretty
> easy to check for.

We shall see :-) But it also means that a given server can keep its 100%
LSP compliance and break your heuristic from one day to the next, right?
Also heads up, the LSP spec has some kind of insertTextModeSupport
capability that Eglot doesn't support yet, but might conceivably want to
support in the future (maybe not, I admit I don't know very well what
it's for.).

>>>>> 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.
>>
>> Indeed "for all".  So this would mean iterating the full completion list
>> for non-trivial computation.  Even if that's manageable, are you aware
>> that some completion lists are incomplete at first and then are resolved
>> to complete lists as the completion session progresses?
>
> Ok, so maybe :exit-function is still needed, but it can be a no-op most
> of the time.

I think s/most/some.  I'm not 100% clear when this is.  But sure, let's
assume you're right.

>> Also, I hope that you are aware that LSP fundamentally disrepects any
>> Emacs completion style.  Because unless you can somehow teach Elisp to
>> arbitrary language servers on the spot, they really have no idea what
>> "flex", "partial-completion", "emacs22" or "orderless" is.
>>
>> I consider this to be a much, much greater nuisance than the partial
>> completion limitations that you picked to start this thread. 
>
> Of course.  That is indeed a significant annoyance.
>
> But to be somewhat provocative: elisp-completion-at-point also doesn't
> know anything about completion styles, yet completion-styles still
> affect completion with elisp-completion-at-point.

I very much appreciate your provocation :-) really do, but I've been
through this many times, often with JD Smith, who may already be reading
this (I'm CCing him).  Again, happy to be proven wrong, but I really
doubt it.

That's because it's a fundamentally different problem.

With elisp-completion-at-point, Emacs functions have near-instantaneous
access to the full completion set because these Elisp completions happen
to live in the same addressing space as the Emacs that's requesting the
completion.  So while the table is indeed style-agnostic, estimably so,
we can still use Elisp functional tricks to feed the configured
completion styles direct access to the full set of completions coming
from the table.

Now, in LSP, completions have to "come in" from a different processe's
addressing space through a comparatively much thinner, less capable
straw.  There's no portable way to even request the full completion set
for a given position, and even if there was it would probably be
unmanageable to transmit it fully over JSON.  All clients can do is ask
the LSP server "given that I have point here in this coordinates, what
do you think are reasonable completions?".  And then you get 0 or some
completions, often not the full set, sometimes not even an indication if
it is partial or full.

There is no way for styles to "drive" the table like they do the elisp
completion table.  It may look like there is, but it's an illusion.
Some people are using "orderless" with LSP (via Eglot or others), but
they're guaranteed to be missing completions here and there except in
perhaps in the most trivial cases.

>> Anyway, as I said before, I think the starting point for this would have
>> to be a systematic structured survey of how Eglot behaves currently in a
>> number of language servers (including yours, of course, but not limited
>> to it) and encode that as unit tests for Eglot (failing or passing).
>>
>> That effort is very valuable in itself and it is a prerequisite for any
>> deep changes to that function.  You can have a look at the exiting
>> 'eglot-test-basic-completions' and 'eglot-test-non-unique-completions'
>> tests.  I'd welcome many more such tests.
>
> Reasonable.
>
> I can add some tests of completion with ocamllsp, rust-analyzer, and
> pyright.
>
> Do these tests run with actual language servers in any CI system?  If
> so, should I make sure that the tests I add, work in that CI system?

Yes and no.  Mostly yes.  Here in Emacs proper the Hydra CI system only
has clangd and little more.  So a significant chunk of Eglot tests are
just skipped.  There is the GitHub actions CI set up on GitHub which
runs clangd, rust-analyzer, pylsp and some others.  You can just clone
my Eglot Github repo, copy over Emacs's lisp/progmodes/eglot.el and
lisp/progmodes/eglot-tests.el and it should start running your new
tests.

Adding new servers to the GitHub CI setup shouldn't be terribly hard
(though it's a bit laborious, as usual with CI things).

You're also welcome to setup your own Micro$oft-less CI solution.

> ;; When selecting from the *Completions*
> ;; buffer, `proxy' won't have any properties.
> ;; A lookup should fix that (github#148)
> (get-text-property
>  0 'eglot--lsp-item
>  (cl-find proxy (funcall proxies) :test #'string=))
>
> :text-function would let you delete this code since you could rely the
> text properties being present.

Ah OK, that's one of the earlier issues, haven't read that in a while.
But sure, pitch this to Stefan.

>> For things to be "useful for Eglot" they have to translate to actual
>> tangible benefits for Eglot users in some server/major-mode combination
>> (and no bugs)
>>
>> "Eglot users" here means user of:
>>
>>   Emacs -Q some-file.xyz -f eglot
>>
>> Where "xyz" is potentially _any_ file type managed by _any_ LSP server,
>> past or future.
>>
>> It doesn't mean users of:
>>
>>   Emacs -Q -l my-special-library-or-configuration.el some-file.ocaml -f eglot
>>
>> That's _not_ to say the latter isn't useful for some, or even many,
>> users.  I just don't count that as "useful for Eglot".
>
> Of course.
>
> I intend the benefit to be better support for try-completion for any
> file type and any LSP server.

Great.

> And also simplification of the code.

Oh, now you're making it sexy

> eglot-completion-at-point is a 200 line quite hairy function.  I don't
> really want to duplicate its code.

Of course.  I'm not particularly proud of that beast, it just grew that
way.  LSP's completion section is also the most complicated in the whole
spec (and Eglot implements about half of it).

You have carte blanche to split it up into multiple functions.

I do heartily recommend writing some more tests first.  Doesn't need to
be an exhaustive battery, but importantly some of these tests must check
that Company and completion-at-point keep working and requesting new
completions only when they needs to.  Maybe some manual testing will be
needed or you'll need to ask Dmitry about ways to control company.el
popups from tests.

> If Eglot exposed parts of eglot-cap as individual APIs, that would
> work too,

Fine by me.  Go for it, really, if you're confident and can demonstrate
you haven't broken too many things.  For example JD Smith has frequently
pitched an idea that would change how eglot-c-a-p does caching, so that
it would work perfectly with Corfu by default.  He's confident it can
work just as well as it does now.  See this discussion

  https://github.com/joaotavora/eglot/discussions/1202#discussioncomment-5548656

I've turned the idea down in the past, but if you think it can
contribute to your eglot-c-a-p overhaul AND it doesn't break Company and
other things, I'm willing to reconsider my opinion.  Especially now that
icomplete-in-region is a thing and _I think_ it also assumes completion
tables behaves like JD/Corfu likes them to.  JD can lead the way here,
if he's available.

João

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

[JD Smith]

[-- Attachment #1: Type: text/plain, Size: 1538 bytes --]

> That's because it's a fundamentally different problem.

I agree with this.  Fancy completion styles work by sending all possible completions, and the completion-at-point systems suggests you do absolutely no filtering based on what the user has typed, leaving the style functions to do that.  This only works when all that data is in memory or close at hand.  A completion style simply has no relevance to the LSP server; it only knows what's in the buffer at point when completion was requested. 

> On Jan 29, 2024, at 5:21 PM, João Távora <joaotavora@gmail.com> wrote:
> 
> It may look like there is, but it's an illusion.
> Some people are using "orderless" with LSP (via Eglot or others), but
> they're guaranteed to be missing completions here and there except in
> perhaps in the most trivial cases.

Unless such people configure orderless to pass every single LSP completion result from the initial orderless term, then turn on all the bells and whistles for winnowing that set down for subsequent terms, which is how I use orderless with LSP.  Definitely not an illusion, but does require a "context" switch as you go.  This switch is aided by the corfu in-buffer completion system's mnemonic of M-SPACE to insert a space (the default orderless separator) and keep completion active.  Before the space?  LSP rules.  After the space?  Orderless takes over.  

But yes, it's a tricky impedance mismatch to manage (and there are many others in the completion-of-data-which-lives-outside-the-emacs-process space). 

[-- Attachment #2: Type: text/html, Size: 4508 bytes --]

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

[João Távora]

On Mon, Jan 29, 2024 at 10:51 PM JD Smith <jdtsmith@gmail.com> wrote:

> Unless such people configure orderless to pass every single LSP completion result from the initial orderless term, then turn on all the bells and whistles for winnowing that set down for subsequent terms, which is how I use orderless with LSP.  Definitely not an illusion, but does require a "context" switch as you go.
> But yes, it's a tricky impedance mismatch to manage (and there are many others in the completion-of-data-which-lives-outside-the-emacs-process space).

OK, so not an illusion for sufficiently savvy experts :-)

João