Subject: Re: bug#61726: [PATCH] Eglot: Support positionEncoding capability

From: João Távora <[email-masked]>
Date: Fri, 24 Feb 2023 11:18:11 +0000

On Fri, Feb 24, 2023 at 11:01 AM Augusto Stoffel <[email-masked]> 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