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

[João Távora]

On Fri, Feb 24, 2023 at 11:27 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Augusto Stoffel <arstoffel@gmail.com>
> > Cc: joaotavora@gmail.com,  61726@debbugs.gnu.org
> > Date: Fri, 24 Feb 2023 10:15:48 +0100
> >
> > >> -(defun eglot-current-column () (- (point) (line-beginning-position)))
> > >> +(defun eglot-current-column ()
> > >> +  "Calculate current column, counting Unicode codepoints."
> > >> +  (- (point) (line-beginning-position)))
> > >
> > > Can we please take this opportunity to get rid of the confusing
> > > "column" terminology?  As became evident from this discussion, we are
> > > not talking columns here, we are talking offsets in characters from
> > > BOL.  So something like "pos" or "linepos" or "line-offset" should be
> > > better.
> > >
> > > João, are you okay with such a sweeping change in all of eglot.el?
> >
> > 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.  Are you sure it isn't better just to add some
clarifying comments?  I fear for my ability to recall details about this
code with such a sweeping rename, accuracy-improving as it may be.
So it's a balancing act, your call.

>
> > >> +              (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@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.

João