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