GNU bug report logs - #61726
[PATCH] Eglot: Support positionEncoding capability

Previous Next

Package: emacs;

Reported by: Augusto Stoffel <arstoffel <at> gmail.com>

Date: Thu, 23 Feb 2023 08:06:01 UTC

Severity: normal

Tags: patch

Done: João Távora <joaotavora <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: João Távora <joaotavora <at> gmail.com>
To: Augusto Stoffel <arstoffel <at> gmail.com>
Cc: 61726 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: bug#61726: [PATCH] Eglot: Support positionEncoding capability
Date: Fri, 24 Feb 2023 11:18:11 +0000
On Fri, Feb 24, 2023 at 11:01 AM Augusto Stoffel <arstoffel <at> gmail.com> wrote:
>
> On Fri, 24 Feb 2023 at 10:20, João Távora wrote:
>
> > The second thing I don't like is also due to the late-binding idea.
> > This is a hotspot in Eglot, some of these functions are called
> > many many times, for each LSP server interaction depending
> > on how many document positions are exchanged (and they can
> > be a lot).  I do remember benchmarking strategies at the time
> > and seeing a perceptible difference.  Plus, this late-binding is
> > really useless as a server will guaranteedly _not_ change its
> > column-counting standard during the LSP session.
>
> `eglot-lsp-abiding-column' allocates a new string!  I doubt that looking
> up a few plists makes much of a difference compared to that.  But when
> using the better offset counting styles, I think it might indeed make a
> difference.  OTOH it might as well be premature optimization.

And this is why we measure.  You can find some scripts for doing that
and some discussion in:

https://github.com/joaotavora/eglot/pull/125

The 2018 issue where this all started.

> > Finally, here's a patch that doesn't use late-binding, doesn't
> > introduce new strategies and supports "utf-32" and "utf-16"
> > today.  As you can see, the patch is nearly trivial.
>
> I'm fine with that way of doing things, but please respond to my concern
> from the other message: do you really want to store a server capability
> in a buffer-local variable?  What about your plans to support multiple
> servers?

The capability is stored in the server object and reflects in the buffer-local
variable which will be restored when the session ends.  I don't see
any problem with that.

My idea of supporting multiple servers is to create a proxy program
that invokes multiple processes and negotiates a common set
of capabilities, so I don't see it as a problem either.  If server A supports
utf-32 and utf-16 and server B only supports utf-16, the multiplexing
server C only declares support for utf-16.

> I suggest you to guard against future headaches.  We can store the
> offset functions in two slots of the server class if you don't like to
> traverse the capabilities plist each time.

No, this is exactly the type of complexity that I strive to avoid in Eglot,
especially when the added value is small.

João




This bug report was last modified 2 years and 138 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.