all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "João Távora" <joaotavora@gmail.com>
To: Spencer Baugh <sbaugh@janestreet.com>
Cc: sbaugh@catern.com,  emacs-devel@gnu.org,
	 monnier@iro.umontreal.ca, jdtsmith@gmail.com
Subject: Re: Eglot cannot work with default completion-in-region?
Date: Mon, 29 Jan 2024 22:21:01 +0000	[thread overview]
Message-ID: <874jevzsqa.fsf@gmail.com> (raw)
In-Reply-To: <iereddz7sgx.fsf@janestreet.com> (Spencer Baugh's message of "Mon, 29 Jan 2024 16:14:06 -0500")

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



  reply	other threads:[~2024-01-29 22:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2024-01-29 22:51           ` JD Smith
2024-01-30  0:32             ` João Távora

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874jevzsqa.fsf@gmail.com \
    --to=joaotavora@gmail.com \
    --cc=emacs-devel@gnu.org \
    --cc=jdtsmith@gmail.com \
    --cc=monnier@iro.umontreal.ca \
    --cc=sbaugh@catern.com \
    --cc=sbaugh@janestreet.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.