[GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Felician Nemeth]

I'd like to submit a new package called eglot-x to ELPA.  Its commentary
starts with:

;; Eglot supports (a subset of) the Language Server Protocol.  However,
;; there are useful protocol extensions that are not part of the
;; official protocol specification.  Eglot-x adds support for some of
;; them.  If you find a bug in Eglot, please, try to reproduce it
;; without Eglot-x, because Eglot-x is substantially modifies Eglot's
;; normal behavior as well.

João intend to support only the standardized protocol features in
eglot.el, but lots of LSP servers extend the protocol in their own way.
(It was João who suggested the package name long ago.)  I considered
eglot-x just an experiment and a learning possibility, but people seem
to use it, so I'd like to make their life easier by this submission.

Currently, it mainly implements extensions for rust-analyzer and taplo,
but some other extensions are supported as well.

The package is hosted at https://github.com/nemethf/eglot-x

Thanks,
Felicián

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Ruijie Yu via Emacs development discussions.]

Didn't look into its sources, but the idea definitely sounds good.  Some
in-line comments below regarding the commentary you attached.

Felician Nemeth <felician.nemeth@gmail.com> writes:

> I'd like to submit a new package called eglot-x to ELPA.  Its commentary
> starts with:
>
> ;; Eglot supports (a subset of) the Language Server Protocol.  However,

I think you should mention (as you did down below) that eglot only
supports standardized protocol features of the LSP, in particular,
_nothing more_ -- hence your package to support commonly-used, yet not
standardized protocol specs.  Thoughts?

> ;; there are useful protocol extensions that are not part of the
> ;; official protocol specification.  Eglot-x adds support for some of
> ;; them.  If you find a bug in Eglot, please, try to reproduce it

New paragraph here at "if"?

> ;; without Eglot-x, because Eglot-x is substantially modifies Eglot's

Syntax: change "is substantially modifies" into either "substantially
modifies" or "has substantially modified".

> ;; normal behavior as well.
>
> João intend to support only the standardized protocol features in
> eglot.el, but lots of LSP servers extend the protocol in their own way.
> (It was João who suggested the package name long ago.)  I considered
> eglot-x just an experiment and a learning possibility, but people seem
> to use it, so I'd like to make their life easier by this submission.
>
> Currently, it mainly implements extensions for rust-analyzer and taplo,
> but some other extensions are supported as well.
>
> The package is hosted at https://github.com/nemethf/eglot-x
>
> Thanks,
> Felicián

-- 
Best,


RY

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Eli Zaretskii]

> From: Felician Nemeth <felician.nemeth@gmail.com>
> Cc: João Távora <joaotavora@gmail.com>
> Date: Fri, 05 May 2023 13:56:21 +0200
> 
> I'd like to submit a new package called eglot-x to ELPA.  Its commentary
> starts with:
> 
> ;; Eglot supports (a subset of) the Language Server Protocol.  However,
> ;; there are useful protocol extensions that are not part of the
> ;; official protocol specification.  Eglot-x adds support for some of
> ;; them.  If you find a bug in Eglot, please, try to reproduce it
> ;; without Eglot-x, because Eglot-x is substantially modifies Eglot's
> ;; normal behavior as well.
> 
> João intend to support only the standardized protocol features in
> eglot.el, but lots of LSP servers extend the protocol in their own way.
> (It was João who suggested the package name long ago.)  I considered
> eglot-x just an experiment and a learning possibility, but people seem
> to use it, so I'd like to make their life easier by this submission.

It sounds strange to me to refuse to support LSP extensions in
eglot.el.  At the very least, eglot.el could benefit from offering a
supported mechanism for adding such extensions; there should be no
need for using advice for that.

João, why would you not consider supporting these extensions as part
of eglot.el?

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> João intend to support only the standardized protocol features in
>> eglot.el, but lots of LSP servers extend the protocol in their own way.
>> (It was João who suggested the package name long ago.)  I considered
>> eglot-x just an experiment and a learning possibility, but people seem
>> to use it, so I'd like to make their life easier by this submission.
>
> It sounds strange to me to refuse to support LSP extensions in
> eglot.el.

Noone "refused" anything.  Some years ago, Felicián and I came to this
agreement.  If there is an actual patch for eglot.el being proposed, I
can review it.  Felicián has contributed much code for eglot.el and they
are very appreciated.

> At the very least, eglot.el could benefit from offering a
> supported mechanism for adding such extensions; there should be no
> need for using advice for that.

I agree.  Eglot has an API.  I haven't looked at eglot-x in depth but I
suspect it's using some of this API in part.  If it's not sufficient,
then Eglot could enhance it.  What part of eglot-x is using advice, if
any?

> João, why would you not consider supporting these extensions as part
> of eglot.el?

They're non-standard, each server has its own set of extensions, which
in some cases could be volatile and become obsolete.  Furthermore, I
would suspect that if eglot-x.el modifies eglot.el's standard behaviour,
it is not as well tested across different servers and could provide a
surprise for users of other servers.

A guiding principle of eglot.el (and indede LSP) is to not have any
server-specific code.

The only such (reasonably manageable) exception is the
eglot-server-programs variable.  But even this variable should ideally be
dissipated across settings in major modes, as was frequently suggested
for gdscript.el mode with much success.  I plan to spin it off to a new
file in the near future, as it is growing very fast.

Furthermore, I presume some of these extensions need some kind of
user-facing UI, and Eglot is conservative about that.  As frequently
explained, I generally prefer using non-LSP generic UIs that integrate
well with all parts of Emacs.  Fine tuning the design of such UIs takes
time and care.

So in general, I think having eglot-x.el as a kind of laboratory where
authors can experiment freely with new LSP-powered UIs is an excellent
approach.  As these extensions make their way into the official LSP
protocol Eglot can support them.  For example, I believe "inlay hints"
started as some kind of rust-specific protocol extension and is now
well-supported in Eglot for many servers (I use it daily).

We could consider bundling eglot-x.el with the Eglot ELPA package (I do
hope Felicián is aware of the ins and outs of :core packages though).
And I think a separate package isn't really bad either.

João

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: Felician Nemeth <felician.nemeth@gmail.com>,  emacs-devel@gnu.org
> Date: Fri, 05 May 2023 15:20:21 +0100
> 
> > At the very least, eglot.el could benefit from offering a
> > supported mechanism for adding such extensions; there should be no
> > need for using advice for that.
> 
> I agree.  Eglot has an API.  I haven't looked at eglot-x in depth but I
> suspect it's using some of this API in part.  If it's not sufficient,
> then Eglot could enhance it.  What part of eglot-x is using advice, if
> any?
> 
> > João, why would you not consider supporting these extensions as part
> > of eglot.el?
> 
> They're non-standard, each server has its own set of extensions, which
> in some cases could be volatile and become obsolete.  Furthermore, I
> would suspect that if eglot-x.el modifies eglot.el's standard behaviour,
> it is not as well tested across different servers and could provide a
> surprise for users of other servers.

I understand and agree in general, but surely each change should be
judged by itself, and some, maybe many, might not have these
problematic aspects?

> A guiding principle of eglot.el (and indede LSP) is to not have any
> server-specific code.

If "server-specific" behavior is, for example, just some non-standard
capabilities, that doesn't count as "server-specific code" in my book.
After all, a standard-compliant server could very well implement only
a subset of the capabilities documented by the standard.

> So in general, I think having eglot-x.el as a kind of laboratory where
> authors can experiment freely with new LSP-powered UIs is an excellent
> approach.  As these extensions make their way into the official LSP
> protocol Eglot can support them.  For example, I believe "inlay hints"
> started as some kind of rust-specific protocol extension and is now
> well-supported in Eglot for many servers (I use it daily).
> 
> We could consider bundling eglot-x.el with the Eglot ELPA package (I do
> hope Felicián is aware of the ins and outs of :core packages though).
> And I think a separate package isn't really bad either.

But it should not use advice, IMNSHO.

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Felician Nemeth <felician.nemeth@gmail.com>
>> Cc: João Távora <joaotavora@gmail.com>
>> Date: Fri, 05 May 2023 13:56:21 +0200
>> 
>> I'd like to submit a new package called eglot-x to ELPA.  Its commentary
>> starts with:
>> 
>> ;; Eglot supports (a subset of) the Language Server Protocol.  However,
>> ;; there are useful protocol extensions that are not part of the
>> ;; official protocol specification.  Eglot-x adds support for some of
>> ;; them.  If you find a bug in Eglot, please, try to reproduce it
>> ;; without Eglot-x, because Eglot-x is substantially modifies Eglot's
>> ;; normal behavior as well.
>> 
>> João intend to support only the standardized protocol features in
>> eglot.el, but lots of LSP servers extend the protocol in their own way.
>> (It was João who suggested the package name long ago.)  I considered
>> eglot-x just an experiment and a learning possibility, but people seem
>> to use it, so I'd like to make their life easier by this submission.
>
> It sounds strange to me to refuse to support LSP extensions in
> eglot.el.  At the very least, eglot.el could benefit from offering a
> supported mechanism for adding such extensions; there should be no
> need for using advice for that.
>
> João, why would you not consider supporting these extensions as part
> of eglot.el?

I think that it makes sense to have non-standard and perhaps not even
well specified (?) extensions to LSP as part of a separate package that
is not part of the core, so that it can adjust to changes in the
implementations that provide these extensions more quickly.  I don't
know what the working procedure is for LSP, but if they do get added to
the standard, it should be easy to move the code from eglot-x to eglot,
if the package is on GNU ELPA.

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

> I understand and agree in general, but surely each change should be
> judged by itself, and some, maybe many, might not have these
> problematic aspects?

Right.  Each change can be judged by itself.

>> A guiding principle of eglot.el (and indede LSP) is to not have any
>> server-specific code.
>
> If "server-specific" behavior is, for example, just some non-standard
> capabilities, that doesn't count as "server-specific code" in my book.
> After all, a standard-compliant server could very well implement only
> a subset of the capabilities documented by the standard.

Right, again.  Each change should be judged on its own merits.

> But it should not use advice, IMNSHO.

Agree.

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Felician Nemeth]

I can surely try to submit these extension one-by-one as patches to
Eglot, but my first attempt was friendly rejected, so I think a package
collecting these rejected extension is a good thing.  Also, even my
patches implementing standard LSP features take a long time to be
processed, so temporarily collecting some extensions in the eglot-x
package allows me to work in my own pace.

Sometime ago, I collected the internal variables, functions, and advised
functions that eglot-x uses from Eglot in order to help to refine
Eglot's public API:

  https://github.com/joaotavora/eglot/discussions/802#discussioncomment-2171239

If it helps, I can explain one by one why it was necessary to use them.

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Felician Nemeth]

Thanks for your comments.  I'll update the README based on this
feedback.

Ruijie Yu <ruijie@netyu.xyz> writes:

> Didn't look into its sources, but the idea definitely sounds good.  Some
> in-line comments below regarding the commentary you attached.
>
> Felician Nemeth <felician.nemeth@gmail.com> writes:
>
>> I'd like to submit a new package called eglot-x to ELPA.  Its commentary
>> starts with:
>>
>> ;; Eglot supports (a subset of) the Language Server Protocol.  However,
>
> I think you should mention (as you did down below) that eglot only
> supports standardized protocol features of the LSP, in particular,
> _nothing more_ -- hence your package to support commonly-used, yet not
> standardized protocol specs.  Thoughts?
>
>> ;; there are useful protocol extensions that are not part of the
>> ;; official protocol specification.  Eglot-x adds support for some of
>> ;; them.  If you find a bug in Eglot, please, try to reproduce it
>
> New paragraph here at "if"?
>
>> ;; without Eglot-x, because Eglot-x is substantially modifies Eglot's
>
> Syntax: change "is substantially modifies" into either "substantially
> modifies" or "has substantially modified".
>
>> ;; normal behavior as well.
>>
>> João intend to support only the standardized protocol features in
>> eglot.el, but lots of LSP servers extend the protocol in their own way.
>> (It was João who suggested the package name long ago.)  I considered
>> eglot-x just an experiment and a learning possibility, but people seem
>> to use it, so I'd like to make their life easier by this submission.
>>
>> Currently, it mainly implements extensions for rust-analyzer and taplo,
>> but some other extensions are supported as well.
>>
>> The package is hosted at https://github.com/nemethf/eglot-x
>>
>> Thanks,
>> Felicián

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Eli Zaretskii]

> From: Felician Nemeth <felician.nemeth@gmail.com>
> Cc: João Távora <joaotavora@gmail.com>,
>   emacs-devel@gnu.org
> Date: Sat, 06 May 2023 15:12:14 +0200
> 
> I can surely try to submit these extension one-by-one as patches to
> Eglot, but my first attempt was friendly rejected, so I think a package
> collecting these rejected extension is a good thing.  Also, even my
> patches implementing standard LSP features take a long time to be
> processed, so temporarily collecting some extensions in the eglot-x
> package allows me to work in my own pace.

Since Eglot is now part of Emacs, the situation has changed.  IMO, it
is no longer reasonable to have extensions that cannot be possibly
supported by the version of Eglot that comes with Emacs.  IOW, if
someone wants to download and install your extensions, we should try
not to force them to also download and install a newer version of
Eglot from ELPA.  I hope this is possible.

> Sometime ago, I collected the internal variables, functions, and advised
> functions that eglot-x uses from Eglot in order to help to refine
> Eglot's public API:
> 
>   https://github.com/joaotavora/eglot/discussions/802#discussioncomment-2171239
> 
> If it helps, I can explain one by one why it was necessary to use them.

What mechanisms aside of advice can be used to extend Eglot so that it
could optionally support those extensions?

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[João Távora]

On Sat, May 6, 2023 at 2:12 PM Felician Nemeth
<felician.nemeth@gmail.com> wrote:
>
> I can surely try to submit these extension one-by-one as patches to
> Eglot, but my first attempt was friendly rejected, so I think a package
> collecting these rejected extension is a good thing.  Also, even my
> patches implementing standard LSP features take a long time to be
> processed, so temporarily collecting some extensions in the eglot-x
> package allows me to work in my own pace.

I don't recall the details (or the discussion, do you have the link?)
but looking at eglot-x I see around 10-20 new things, I don't remember
you submitting 10-20 new separate things.  So start with the one you think
is the most valuable, most popular, and has the  highest chance of
integration.

João

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Felician Nemeth]

Eli Zaretskii <eliz@gnu.org> writes:

> IMO, it is no longer reasonable to have extensions that cannot be
> possibly supported by the version of Eglot that comes with Emacs.
> IOW, if someone wants to download and install your extensions, we
> should try not to force them to also download and install a newer
> version of Eglot from ELPA.  I hope this is possible.

Yes, I think so.  I'll keep this in mind when I modify eglot-x.

> What mechanisms aside of advice can be used to extend Eglot so that it
> could optionally support those extensions?

1. Eglot support editing remote files when the source files and the LSP
   server are on the same remote system.  There is an extension that
   makes possible to edit files when the files and the server are on
   different systems.

   Eglot's own remote file support rewrites filenames, which interfered
   with my implementation.  My solution was basically to selective
   advice some Eglot functions to make file-remote-p always return nil.

   An alternative approach could be an Eglot configuration variable to
   optionally disable its tramp support.


2. There's a clangd extension for encoding negotiation.  The server
   sends the result of the negotiation in an InitializeResult response.
   Eglot saves the capabilities part of this response, which is usually
   enough, but this extension puts the negotiated encoding elsewhere.

   Eglot-x uses a series of add-advices calls to copy the encoding
   string into the exposed capabilities variable.

   An alternative approach for Eglot could be to save the whole
   InitializeResult response as well.

   However, the encoding negotiation extension has been standardized (in
   a slightly different way).  Eglot does support the standardized
   feature.  So maybe it is enough to wait a bit until clangd implements
   the new standard and then this feature can be removed from eglot-x.


3. Eglot uses the eglot--apply-text-edits defun to apply server
   initiated edits.  There is an extension that allows the server to
   send the edits in a different format (snippet-text-edits).

   Eglot-x puts an advise on eglot--apply-text-edits to check the format
   of the edits and act accordingly.

   I don't know how to avoid this advice.


4. Eglot-x puts an advice on eglot--mode-line-format in order to display
   the server's status (which the server sends if the client supports an
   LSP extension.)

   I think eglot-x should just use its own space in the mode line.  I
   think I just wasn't able to figure out how it could be put next to
   Eglot's mode-line.


5. Finally, eglot-x stores its own state in an instance of the
   eglot-lsp-server.  eglot-lsp-server is defined with defclass, so it
   is possible to add new slots to it without any problem.

   For some reason I just wanted to hide these eglot-x specific slots
   from Eglot.  It think the solution is to simply not use an advice
   here.

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Eli Zaretskii]

> From: Felician Nemeth <felician.nemeth@gmail.com>
> Cc: joaotavora@gmail.com,  emacs-devel@gnu.org
> Date: Sat, 06 May 2023 17:17:14 +0200
> 
> 3. Eglot uses the eglot--apply-text-edits defun to apply server
>    initiated edits.  There is an extension that allows the server to
>    send the edits in a different format (snippet-text-edits).
> 
>    Eglot-x puts an advise on eglot--apply-text-edits to check the format
>    of the edits and act accordingly.
> 
>    I don't know how to avoid this advice.

Some hook or function variable, perhaps?  If they don't exist, perhaps
they could be added?

Re: [GNU ELPA] eglot-x.el: Protocol extensions for Eglot

[Felician Nemeth]

João Távora <joaotavora@gmail.com> writes:

> On Sat, May 6, 2023 at 2:12 PM Felician Nemeth
> <felician.nemeth@gmail.com> wrote:
>>
>> I can surely try to submit these extension one-by-one as patches to
>> Eglot, but my first attempt was friendly rejected, so I think a package
>> collecting these rejected extension is a good thing.  Also, even my
>> patches implementing standard LSP features take a long time to be
>> processed, so temporarily collecting some extensions in the eglot-x
>> package allows me to work in my own pace.
>
> I don't recall the details (or the discussion, do you have the link?)

All I was able to find is the following PR, which is not very helpful in
this regard: https://github.com/joaotavora/eglot/pull/211

> but looking at eglot-x I see around 10-20 new things, I don't remember
> you submitting 10-20 new separate things.  So start with the one you think
> is the most valuable, most popular, and has the  highest chance of
> integration.

Sure, but firstly my plan is to nudge you to deal with my patches
concerning standard LSP features and Eglot-enhancements :)