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

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

> From: Augusto Stoffel <[email-masked]>
> Cc: [email-masked],  [email-masked]
> 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?

> > As discussed, position-bytes is incorrect.  You should instead do
> > something like
> >
> >   (length (encode-coding-string
> >            (buffer-substring-no-properties (point)
> >                                            (line-beginning-position))
> >            'utf-8-unix t))
> 
> But it is incorrect only if the buffer contains characters outside of
> the Unicode range, right?  If that happens, we already lost, because a
> few steps later we will serialize the buffer text as JSON to send it to
> the server:

Why should one part of the code depend on what another part does?  In
my book, each part should do its job, and do it right.

> > Also, for 100% reliable results, we should bind
> > inhibit-field-text-motion to t when calling line-beginning-position.
> 
> We should rather be using pos-bol, no?  But how do we keep compatibility
> with older Emacsen?

Exactly.  pos-bol is Emacs 29 and later, whereas Eglot is available
from ELPA for older versions of Emacs.

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