* Eglot cannot work with default completion-in-region?
@ 2024-01-26 20:38 Spencer Baugh
2024-01-27 14:45 ` João Távora
0 siblings, 1 reply; 9+ messages in thread
From: Spencer Baugh @ 2024-01-26 20:38 UTC (permalink / raw)
To: emacs-devel; +Cc: joaotavora
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)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-26 20:38 Eglot cannot work with default completion-in-region? Spencer Baugh @ 2024-01-27 14:45 ` João Távora 2024-01-27 15:37 ` sbaugh 0 siblings, 1 reply; 9+ messages in thread From: João Távora @ 2024-01-27 14:45 UTC (permalink / raw) To: Spencer Baugh, emacs-devel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-27 14:45 ` João Távora @ 2024-01-27 15:37 ` sbaugh 2024-01-28 10:23 ` Daniel Mendler via Emacs development discussions. 2024-01-28 14:09 ` João Távora 0 siblings, 2 replies; 9+ messages in thread From: sbaugh @ 2024-01-27 15:37 UTC (permalink / raw) To: emacs-devel 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-27 15:37 ` sbaugh @ 2024-01-28 10:23 ` Daniel Mendler via Emacs development discussions. 2024-01-28 14:09 ` João Távora 1 sibling, 0 replies; 9+ messages in thread From: Daniel Mendler via Emacs development discussions. @ 2024-01-28 10:23 UTC (permalink / raw) To: sbaugh; +Cc: emacs-devel, Dmitry Gutov 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-27 15:37 ` sbaugh 2024-01-28 10:23 ` Daniel Mendler via Emacs development discussions. @ 2024-01-28 14:09 ` João Távora 2024-01-29 21:14 ` Spencer Baugh 1 sibling, 1 reply; 9+ messages in thread From: João Távora @ 2024-01-28 14:09 UTC (permalink / raw) To: sbaugh; +Cc: emacs-devel 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-28 14:09 ` João Távora @ 2024-01-29 21:14 ` Spencer Baugh 2024-01-29 22:21 ` João Távora 0 siblings, 1 reply; 9+ messages in thread From: Spencer Baugh @ 2024-01-29 21:14 UTC (permalink / raw) To: João Távora; +Cc: sbaugh, emacs-devel, monnier 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-29 21:14 ` Spencer Baugh @ 2024-01-29 22:21 ` João Távora 2024-01-29 22:51 ` JD Smith 0 siblings, 1 reply; 9+ messages in thread From: João Távora @ 2024-01-29 22:21 UTC (permalink / raw) To: Spencer Baugh; +Cc: sbaugh, emacs-devel, monnier, jdtsmith 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-29 22:21 ` João Távora @ 2024-01-29 22:51 ` JD Smith 2024-01-30 0:32 ` João Távora 0 siblings, 1 reply; 9+ messages in thread From: JD Smith @ 2024-01-29 22:51 UTC (permalink / raw) To: João Távora; +Cc: Spencer Baugh, sbaugh, emacs-devel, monnier [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Eglot cannot work with default completion-in-region? 2024-01-29 22:51 ` JD Smith @ 2024-01-30 0:32 ` João Távora 0 siblings, 0 replies; 9+ messages in thread From: João Távora @ 2024-01-30 0:32 UTC (permalink / raw) To: JD Smith; +Cc: Spencer Baugh, sbaugh, emacs-devel, monnier 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-01-30 0:32 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-26 20:38 Eglot cannot work with default completion-in-region? Spencer Baugh 2024-01-27 14:45 ` João Távora 2024-01-27 15:37 ` sbaugh 2024-01-28 10:23 ` Daniel Mendler via Emacs development discussions. 2024-01-28 14:09 ` João Távora 2024-01-29 21:14 ` Spencer Baugh 2024-01-29 22:21 ` João Távora 2024-01-29 22:51 ` JD Smith 2024-01-30 0:32 ` João Távora
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.