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

Previous Next

Full log

Message #92 received at 61726 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: João Távora <joaotavora <at> gmail.com>
Cc: 61726 <at> debbugs.gnu.org, arstoffel <at> gmail.com
Subject: Re: bug#61726: [PATCH] Eglot: Support positionEncoding capability
Date: Fri, 24 Feb 2023 13:57:57 +0200

> From: João Távora <joaotavora <at> gmail.com>
> Date: Fri, 24 Feb 2023 11:43:18 +0000
> Cc: Augusto Stoffel <arstoffel <at> gmail.com>, 61726 <at> debbugs.gnu.org
> 
> > > I like linepos, if João is fine with not making the absolute minimal
> > > amount of changes to the code.
> >
> > João?
> 
> eglot-current-column is a user-visible function.  would need
> obsoletion aliases.

Yes.  But that is not a problem, from my POV.

> Are you sure it isn't better just to add some clarifying comments?

I'm okay with that as well, but the clarifications should be in doc
strings of public functions as well, if we go that way.

> > > >> +              (tab-width 1)
> > > >                   ^^^^^^^^^^^
> > > > This last part shouldn't be necessary: we should move by characters,
> > > > not by columns.  Why is it necessary?
> > >
> > > Maybe João can clarify, but I'm pretty sure this is there to support the
> > > UTF-16 way of counting offsets, so this ideally should move to
> > > eglot-move-to-lsp-abiding-column.
> >
> > Then perhaps the UTF-16 way of counting offsets should be changed as
> > well.
> 
> I've vc-region-history'ed it to:
> 
> commit 2cf7905887f2137869f44c3383a55636e38b4b81
> Author: Michal Krzywkowski <k.michal <at> zoho.com>
> Date:   Mon Nov 19 21:22:14 2018 +0100
> 
>     Treat tab characters as 1 column wide in position conversion functions
> 
>     Fixes https://github.com/joaotavora/eglot/issues/158.
> 
>     * eglot.el (eglot--pos-to-lsp-position): Call
>       eglot-current-column-function with tab-width bound to 1.
>     (eglot--lsp-position-to-point): Call eglot-move-to-column-function
>     with tab-width bound to 1.
> 
> Following the link, I read this there:
> 
>   "This is because move-to-column and current-column count each tab
> character as
>    tab-width chars."
> 
> And, as far as I remember, at the time we were indeed using move-to-column and
> current-column.  But now we aren't anymore.
> 
> So maybe, just maybe, this can be removed.  And the full test suite must
> run afterwards.  And then probably more tests should be added.

How about removing it on master?

Previous Next

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