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

From: João Távora <[email-masked]>
Date: Sun, 26 Feb 2023 14:17:39 +0000

Eli Zaretskii <[email-masked]> writes:

>> From: João Távora <[email-masked]>
>> Date: Sun, 26 Feb 2023 13:13:16 +0000
>> Cc: Augusto Stoffel <[email-masked]>, [email-masked]
>> 
>> * Pushed a renaming/redocumenting obsoletion-establishing patch.  Now
>> Eglot talks of "linepos" instead of "column".
>
> I've corrected a few doc strings to use a more widely-familiar
> terminology.

Thanks, but at one of the corrections is mistaken, so I've fixed it like
this.

     (defvar eglot-current-linepos-function #'eglot-utf-16-linepos
    -  "Function calculating number of UTF-16 code units from line beginning.
    +  "Function calculating number of code units from line beginning.
     
     This is the inverse operation of
     `eglot-move-to-linepos-function' (which see).	It is a function of

This variable is set by default to a function that indeed does UTF-16
calculations.  But it can and will be set to one that does UTF-32 or
UTF-8 codepoint calculations, depending on the server.

I also have a few nits.  In your patch:

     (defun eglot-utf-32-linepos ()
    -  "Calculate number of code units to line beginning using UTF-32."
    +  "Calculate number of Unicode codepoints from line beginning."
       (- (point) (line-beginning-position)))

This is strictly true, because in UTF-32 it's one code point per code
unit.  But the protocol of the variable
'eglot-current-linepos-function', where this function is to be plugged,
a code-unit counting function is demanded.  I would predict that
encoding-challenged programmers (like this package's maintainer in about
2 week's time) might not know about that equivalence and will find it
slightly confusing to plug this function into that variable.

But as I said this is just a nit.

João