From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh Newsgroups: gmane.emacs.devel Subject: Re: Eglot cannot work with default completion-in-region? Date: Mon, 29 Jan 2024 16:14:06 -0500 Message-ID: References: <87ttmy7pog.fsf@catern.com> <87r0i1blch.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="31700"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: sbaugh@catern.com, emacs-devel@gnu.org, monnier@iro.umontreal.ca To: =?utf-8?B?Sm/Do28gVMOhdm9yYQ==?= Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Tue Jan 30 04:24:00 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rUej9-0007zM-DD for ged-emacs-devel@m.gmane-mx.org; Tue, 30 Jan 2024 04:23:59 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rUeiQ-00025s-U7; Mon, 29 Jan 2024 22:23:14 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rUYxF-0008Tg-Ff for emacs-devel@gnu.org; Mon, 29 Jan 2024 16:14:09 -0500 Original-Received: from mxout5.mail.janestreet.com ([64.215.233.18]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rUYxD-0002hL-FB for emacs-devel@gnu.org; Mon, 29 Jan 2024 16:14:09 -0500 In-Reply-To: <87r0i1blch.fsf@gmail.com> (=?utf-8?Q?=22Jo=C3=A3o_T=C3=A1vor?= =?utf-8?Q?a=22's?= message of "Sun, 28 Jan 2024 14:09:50 +0000") DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1706562846; bh=Hted5JoJcKpxWWMuyynObZNawqcsLY1z7Z9Le+ImsA0=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=gFoDWXjHM1Z2uZp1UWSnSVX1ymYxRsqrO1EwoBpCJjHYFvgVvt+tCjm+JajF59Z7w 9x7PsHut0mBBT57QObTIsWw0nGNEYK2hRL0R4B1nSFaVBd03aqZn6izFneEHZRXKca cCGflFadfusIWnE9LDpaPe1wZphx34O9Fd1VY4N5IiVrPiedSsU3q95TlPfdL0B2hO zU7U1RFeRGhoVENi71RFvVbyXXq1TwvaCGntv4pIBIQ2euIJUlzs1Om/RwNgkmnRkp mNhWDrStOExI1grYDRdM5JMv+B1hafLL6HAIMTwlvhKf3Ma5TVIQYVl+HN/xz8V2gK dKK8rosOJdXHA== Received-SPF: pass client-ip=64.215.233.18; envelope-from=sbaugh@janestreet.com; helo=mxout5.mail.janestreet.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-Mailman-Approved-At: Mon, 29 Jan 2024 22:23:07 -0500 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:315622 Archived-At: Jo=C3=A3o T=C3=A1vora writes: > sbaugh@catern.com writes: >> Jo=C3=A3o T=C3=A1vora writes: >>> Is it? The above all return textEdits, I think. How can sortText, >>> a JSON string, equal textEdit, a JSON object? >> >> If textEdit.range=3D=3Dthe completion region and textEdit.newText=3D=3Ds= ortText, >> 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.=20 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 a= ll >>>> 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.=20=20 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=3D)) :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 e= glot > > 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=C3=A3o 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.