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

From: Eli Zaretskii <[email-masked]>
Date: Fri, 24 Feb 2023 13:57:57 +0200

> From: João Távora <[email-masked]>
> Date: Fri, 24 Feb 2023 11:43:18 +0000
> Cc: Augusto Stoffel <[email-masked]>, [email-masked]
> 
> > > 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 <[email-masked]>
> 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?