* [PATCH] Eglot: auto-import completion item
@ 2022-11-12 13:23 Marcin Pajkowski
2022-11-17 9:50 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Marcin Pajkowski @ 2022-11-12 13:23 UTC (permalink / raw)
To: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 854 bytes --]
* lisp/progmodes/eglot.el (eglot-client-capabilities): Advertise
resolveSupport capabilities
---
lisp/progmodes/eglot.el | 1 +
1 file changed, 1 insertion(+)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 63ebbe6cab..953c0f45fc 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -736,6 +736,7 @@ eglot-client-capabilities
t
:json-false)
:deprecatedSupport t
+ :resolveSupport (:properties
["documentation" "details" "additionalTextEdits"])
:tagSupport (:valueSet [1]))
:contextSupport t)
:hover (list :dynamicRegistration :json-false
--
2.38.1
[-- Attachment #2: Type: text/html, Size: 1108 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Eglot: auto-import completion item
2022-11-12 13:23 [PATCH] Eglot: auto-import completion item Marcin Pajkowski
@ 2022-11-17 9:50 ` Eli Zaretskii
2022-11-17 10:41 ` João Távora
0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2022-11-17 9:50 UTC (permalink / raw)
To: Marcin Pajkowski, João Távora; +Cc: emacs-devel
> From: Marcin Pajkowski <marcin.pajkowski@gmail.com>
> Date: Sat, 12 Nov 2022 14:23:52 +0100
>
> * lisp/progmodes/eglot.el (eglot-client-capabilities): Advertise
> resolveSupport capabilities
> ---
> lisp/progmodes/eglot.el | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 63ebbe6cab..953c0f45fc 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -736,6 +736,7 @@ eglot-client-capabilities
> t
> :json-false)
> :deprecatedSupport t
> + :resolveSupport (:properties ["documentation" "details" "additionalTextEdits"])
> :tagSupport (:valueSet [1]))
> :contextSupport t)
> :hover (list :dynamicRegistration :json-false
> --
> 2.38.1
João, what about this one? Should we install it? What are
resolveSupport capabilities about?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Eglot: auto-import completion item
2022-11-17 9:50 ` Eli Zaretskii
@ 2022-11-17 10:41 ` João Távora
2022-11-20 20:52 ` Marcin Pajkowski
0 siblings, 1 reply; 6+ messages in thread
From: João Távora @ 2022-11-17 10:41 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Marcin Pajkowski, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]
What these particular capabilities are about can be seen
in the LSP spec. I haven't checked, but I think this refers to
Eglot's capability to present meta information about a given
completion item and to do additional things elsewhere in the
buffer once a given completion item is chosen by the user.
So, typically you may be given the completion:
std::cout
and if you do choose it, Eglot and LSP try to ensure that
an #include <iostream> appears in the top of your C++ file.
In general, normally both client and server advertise their
capabilities to each other so that the other side refrains from
doing anything that is not explicitly mentioned in these
advertisements. This is how there are really no "versions of
the protocol" (well there are, but they aren't used in the way
a normal versioned API is used). It doesn't always work that
way, and both clients and servers will both request stuff that
isn't possible and volunteer stuff that isn't needed.
It's indeed odd to find that Eglot starts advertising a
capability just by itself, without adding any new functional
code presumably backing that advertisement, so you're right
to be suspicious, Eli.
But it may be just that the advertisement was incorrectly
glossed over in the past (which wouldn't necessarily present
a problem since many servers don't bother to check
advertisements, as explained), or -- also possible -- that the
advertisement wasn't even specified in the past.
Most importantly, I would like Marcin to explain, if possible,
what actual problem with what server this is solving.
The reasoning and conclusion should be in the commit message
for this patch for future reference.
João
On Thu, Nov 17, 2022 at 9:50 AM Eli Zaretskii <eliz@gnu.org> wrote:
> > From: Marcin Pajkowski <marcin.pajkowski@gmail.com>
> > Date: Sat, 12 Nov 2022 14:23:52 +0100
> >
> > * lisp/progmodes/eglot.el (eglot-client-capabilities): Advertise
> > resolveSupport capabilities
> > ---
> > lisp/progmodes/eglot.el | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> > index 63ebbe6cab..953c0f45fc 100644
> > --- a/lisp/progmodes/eglot.el
> > +++ b/lisp/progmodes/eglot.el
> > @@ -736,6 +736,7 @@ eglot-client-capabilities
> > t
> > :json-false)
> > :deprecatedSupport t
> > + :resolveSupport (:properties
> ["documentation" "details" "additionalTextEdits"])
> > :tagSupport (:valueSet [1]))
> > :contextSupport t)
> > :hover (list :dynamicRegistration :json-false
> > --
> > 2.38.1
>
> João, what about this one? Should we install it? What are
> resolveSupport capabilities about?
>
>
--
João Távora
[-- Attachment #2: Type: text/html, Size: 4182 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Eglot: auto-import completion item
2022-11-17 10:41 ` João Távora
@ 2022-11-20 20:52 ` Marcin Pajkowski
2022-11-21 13:56 ` João Távora
0 siblings, 1 reply; 6+ messages in thread
From: Marcin Pajkowski @ 2022-11-20 20:52 UTC (permalink / raw)
To: João Távora; +Cc: Eli Zaretskii, emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 4053 bytes --]
Hi Emacs developers,
I use clangd and rust-analyzer LSP servers and found this issue
is specific to rust-analyzer.
This advertisement is required by rust-analyzer server
to provide completions for symbols that are not resolved
but can be automatically imported. Without advertising, this capability
server does not report such completions, which kind of makes sense IMHO:
- server can avoid some computations,
- client which does not support additionalTextEdits capability
doesn't receive items that can't be auto-imported. User of such
LSP client could probably consider such suggestions inaccurate
(I applied completion and my code does not compile).
Reference:
https://rust-analyzer.github.io/manual.html#completion-with-autoimport
I reflected my clarifications in a commit message and put the patch
in a separate file.
Best regards,
Marcin
czw., 17 lis 2022 o 11:40 João Távora <joaotavora@gmail.com> napisał(a):
> What these particular capabilities are about can be seen
> in the LSP spec. I haven't checked, but I think this refers to
> Eglot's capability to present meta information about a given
> completion item and to do additional things elsewhere in the
> buffer once a given completion item is chosen by the user.
> So, typically you may be given the completion:
>
> std::cout
>
> and if you do choose it, Eglot and LSP try to ensure that
> an #include <iostream> appears in the top of your C++ file.
>
> In general, normally both client and server advertise their
> capabilities to each other so that the other side refrains from
> doing anything that is not explicitly mentioned in these
> advertisements. This is how there are really no "versions of
> the protocol" (well there are, but they aren't used in the way
> a normal versioned API is used). It doesn't always work that
> way, and both clients and servers will both request stuff that
> isn't possible and volunteer stuff that isn't needed.
>
> It's indeed odd to find that Eglot starts advertising a
> capability just by itself, without adding any new functional
> code presumably backing that advertisement, so you're right
> to be suspicious, Eli.
>
> But it may be just that the advertisement was incorrectly
> glossed over in the past (which wouldn't necessarily present
> a problem since many servers don't bother to check
> advertisements, as explained), or -- also possible -- that the
> advertisement wasn't even specified in the past.
>
> Most importantly, I would like Marcin to explain, if possible,
> what actual problem with what server this is solving.
>
> The reasoning and conclusion should be in the commit message
> for this patch for future reference.
>
> João
>
> On Thu, Nov 17, 2022 at 9:50 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Marcin Pajkowski <marcin.pajkowski@gmail.com>
>> > Date: Sat, 12 Nov 2022 14:23:52 +0100
>> >
>> > * lisp/progmodes/eglot.el (eglot-client-capabilities): Advertise
>> > resolveSupport capabilities
>> > ---
>> > lisp/progmodes/eglot.el | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
>> > index 63ebbe6cab..953c0f45fc 100644
>> > --- a/lisp/progmodes/eglot.el
>> > +++ b/lisp/progmodes/eglot.el
>> > @@ -736,6 +736,7 @@ eglot-client-capabilities
>> > t
>> > :json-false)
>> > :deprecatedSupport t
>> > + :resolveSupport (:properties
>> ["documentation" "details" "additionalTextEdits"])
>> > :tagSupport (:valueSet [1]))
>> > :contextSupport t)
>> > :hover (list :dynamicRegistration :json-false
>> > --
>> > 2.38.1
>>
>> João, what about this one? Should we install it? What are
>> resolveSupport capabilities about?
>>
>>
>
> --
> João Távora
>
[-- Attachment #1.2: Type: text/html, Size: 5625 bytes --]
[-- Attachment #2: Eglot-Advertise-completion.resolveSupport-capabilities.patch --]
[-- Type: text/x-patch, Size: 1249 bytes --]
From 4cba7ffa9ad8934601efe7391b5b036661d9f240 Mon Sep 17 00:00:00 2001
From: Marcin Pajkowski <marcin.pajkowski@gmail.com>
Date: Sun, 20 Nov 2022 20:03:57 +0100
Subject: [PATCH] Eglot: Advertise completion.resolveSupport capabilities
Some servers avoid reporting completion items that require
"additionalTextEdits" capability. Actually eglot-completion-at-point
function supports such feature so it can be adverised to LSP server.
* lisp/progmodes/eglot.el (eglot-client-capabilities)
---
lisp/progmodes/eglot.el | 1 +
1 file changed, 1 insertion(+)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 9555d21b00..4193ee0106 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -737,6 +737,7 @@ eglot-client-capabilities
t
:json-false)
:deprecatedSupport t
+ :resolveSupport (:properties ["documentation" "details" "additionalTextEdits"])
:tagSupport (:valueSet [1]))
:contextSupport t)
:hover (list :dynamicRegistration :json-false
--
2.38.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] Eglot: auto-import completion item
2022-11-20 20:52 ` Marcin Pajkowski
@ 2022-11-21 13:56 ` João Távora
2022-11-22 20:26 ` Marcin Pajkowski
0 siblings, 1 reply; 6+ messages in thread
From: João Távora @ 2022-11-21 13:56 UTC (permalink / raw)
To: Marcin Pajkowski; +Cc: Eli Zaretskii, emacs-devel
Marcin Pajkowski <marcin.pajkowski@gmail.com> writes:
Hi Marcin,
the patch looks good to install, but please send it to the bug tracker
where the discussion started instead: I don't have the bug # handy.
Minor comments below my sig.
João
> Subject: [PATCH] Eglot: Advertise completion.resolveSupport capabilities
>
> Some servers avoid reporting completion items that require
> "additionalTextEdits" capability. Actually eglot-completion-at-point
> function supports such feature so it can be adverised to LSP server.
^^^^^^^^^
typo!
Also missing a line bug#xxxxx here.
> * lisp/progmodes/eglot.el (eglot-client-capabilities)
Here's, you're supposed to add a ':' and then briefly state what you did
to the definition.
Like this:
* lisp/progmodes/eglot.el
(eglot-client-capabilities): Advertise :resolveSupport.
> ---
> lisp/progmodes/eglot.el | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 9555d21b00..4193ee0106 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -737,6 +737,7 @@ eglot-client-capabilities
> t
> :json-false)
> :deprecatedSupport t
> + :resolveSupport (:properties ["documentation" "details" "additionalTextEdits"])
Can you break this line so that it fits in under 80 columns?
> :tagSupport (:valueSet [1]))
> :contextSupport t)
> :hover (list :dynamicRegistration :json-false
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Eglot: auto-import completion item
2022-11-21 13:56 ` João Távora
@ 2022-11-22 20:26 ` Marcin Pajkowski
0 siblings, 0 replies; 6+ messages in thread
From: Marcin Pajkowski @ 2022-11-22 20:26 UTC (permalink / raw)
To: João Távora; +Cc: Eli Zaretskii, emacs-devel
[-- Attachment #1.1: Type: text/plain, Size: 2132 bytes --]
Hi João,
Thanks for reviewing my patch!
I added bug reference, fixed typo and sent copy of this patch to the bug
tracker.
Marcin
pon., 21 lis 2022 o 14:55 João Távora <joaotavora@gmail.com> napisał(a):
> Marcin Pajkowski <marcin.pajkowski@gmail.com> writes:
>
> Hi Marcin,
>
> the patch looks good to install, but please send it to the bug tracker
> where the discussion started instead: I don't have the bug # handy.
> Minor comments below my sig.
>
> João
>
> > Subject: [PATCH] Eglot: Advertise completion.resolveSupport capabilities
> >
> > Some servers avoid reporting completion items that require
> > "additionalTextEdits" capability. Actually eglot-completion-at-point
> > function supports such feature so it can be adverised to LSP server.
> ^^^^^^^^^
> typo!
>
> Also missing a line bug#xxxxx here.
>
> > * lisp/progmodes/eglot.el (eglot-client-capabilities)
>
> Here's, you're supposed to add a ':' and then briefly state what you did
> to the definition.
>
> Like this:
>
> * lisp/progmodes/eglot.el
> (eglot-client-capabilities): Advertise :resolveSupport.
>
>
> > ---
> > lisp/progmodes/eglot.el | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> > index 9555d21b00..4193ee0106 100644
> > --- a/lisp/progmodes/eglot.el
> > +++ b/lisp/progmodes/eglot.el
> > @@ -737,6 +737,7 @@ eglot-client-capabilities
> > t
> > :json-false)
> > :deprecatedSupport t
> > + :resolveSupport (:properties
> ["documentation" "details" "additionalTextEdits"])
>
> Can you break this line so that it fits in under 80 columns?
>
> > :tagSupport (:valueSet [1]))
> > :contextSupport t)
> > :hover (list :dynamicRegistration :json-false
>
>
>
[-- Attachment #1.2: Type: text/html, Size: 3001 bytes --]
[-- Attachment #2: Eglot-Advertise-completion.resolveSupport-capabilities.patch --]
[-- Type: text/x-patch, Size: 1465 bytes --]
From b290face2792b254ad9567d007205e715da48283 Mon Sep 17 00:00:00 2001
From: Marcin Pajkowski <marcin.pajkowski@gmail.com>
Date: Sun, 20 Nov 2022 20:03:57 +0100
Subject: [PATCH] Eglot: Advertise completion.resolveSupport capabilities
Some servers avoid reporting completion items that require
"additionalTextEdits" capability. Actually eglot-completion-at-point
function supports such feature so it can be advertised to LSP server.
* lisp/progmodes/eglot.el
(eglot-client-capabilities): Advertise resolveSupport. (bug#59465)
---
lisp/progmodes/eglot.el | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index bbd902c1c7..fab3b1cac0 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -737,7 +737,10 @@ eglot-client-capabilities
t
:json-false)
:deprecatedSupport t
+ :resolveSupport (:properties
+ ["documentation"
+ "details"
+ "additionalTextEdits"])
:tagSupport (:valueSet [1]))
:contextSupport t)
:hover (list :dynamicRegistration :json-false
--
2.38.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-11-22 20:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-12 13:23 [PATCH] Eglot: auto-import completion item Marcin Pajkowski
2022-11-17 9:50 ` Eli Zaretskii
2022-11-17 10:41 ` João Távora
2022-11-20 20:52 ` Marcin Pajkowski
2022-11-21 13:56 ` João Távora
2022-11-22 20:26 ` Marcin Pajkowski
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).