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

[Augusto Stoffel]

On Fri, 24 Feb 2023 at 08:43, Eli Zaretskii wrote:

> It does? then please humor me by walking me through the code and the
> patch to show how that would work after applying the patch.

>From a86601f4e80dfbf21a84230433c431375e3012aa Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Thu, 23 Feb 2023 08:55:58 +0100
Subject: [PATCH] * lisp/progmodes/eglot.el: Support positionEncoding
 capability

---
 lisp/progmodes/eglot.el | 65 +++++++++++++++++++++++++++++------------
 1 file changed, 46 insertions(+), 19 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index b569c03e8c2..0268fbf63a5 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -816,6 +816,9 @@ eglot-client-capabilities
                                        `(:valueSet
                                          [,@(mapcar
                                              #'car eglot--tag-faces)])))
+            :general
+            (list
+             :positionEncodings ["utf-32" "utf-8" "utf-16"])
             :experimental eglot--{})))

We announce our position encoding capabilities, in this order of
preference:

A. counting characters a.k.a. Unicode codepoints
B. counting bytes of the UTF-8 representation
C. counting bytes of the UTF-16 representation divided by two (which is
   currently the default).

 (cl-defgeneric eglot-workspace-folders (server)
@@ -1439,20 +1442,26 @@ eglot--warn
   (let ((warning-minimum-level :error))
     (display-warning 'eglot (apply #'format format args) :warning)))
 
-(defun eglot-current-column () (- (point) (line-beginning-position)))
+(defun eglot-current-column ()
+  "Calculate current column, counting Unicode codepoints."
+  (- (point) (line-beginning-position)))

I added a docstring.

+(defun eglot--current-column-utf-8 ()
+  "Calculate current column, counting bytes."
+  (- (position-bytes (point)) (position-bytes (line-beginning-position))))

I defined a new function to support the style B. of counting offsets.

-(defvar eglot-current-column-function #'eglot-lsp-abiding-column
+(defvar eglot-current-column-function nil
   "Function to calculate the current column.

I changed the default so this variable can be eventually made obsolete.
Note that it is a workaround variable introduced for the sole purpose of
making nonconforming servers work with Eglot.  But this problem should
slowly vanish with the introduction of the :positionEncoding capability.
Hence my suggestion to obsolete this workaround variable.

 This is the inverse operation of
 `eglot-move-to-column-function' (which see).  It is a function of
 no arguments returning a column number.  For buffers managed by
-fully LSP-compliant servers, this should be set to
-`eglot-lsp-abiding-column' (the default), and
-`eglot-current-column' for all others.")
+fully LSP-compliant servers, this should be nil.  For others, it
+can be set to `eglot-current-colum' or
+`eglot--current-column-utf-8'.")
 
 (defun eglot-lsp-abiding-column (&optional lbp)
-  "Calculate current COLUMN as defined by the LSP spec.
+  "Calculate current column, counting UTF-16 code units as in the original LSP 
spec.
 LBP defaults to `line-beginning-position'."
   (/ (- (length (encode-coding-region (or lbp (line-beginning-position))
                                       ;; Fix github#860

The LSP spec now describes 3 ways of counting offsets, hence the
documenation clarification.

@@ -1462,13 +1471,19 @@ eglot-lsp-abiding-column
 
 (defun eglot--pos-to-lsp-position (&optional pos)
   "Convert point POS to LSP position."
-  (eglot--widening
-   ;; LSP line is zero-origin; emacs is one-origin.
-   (list :line (1- (line-number-at-pos pos t))
-         :character (progn (when pos (goto-char pos))
-                           (funcall eglot-current-column-function)))))
-
+  (let ((columnfn (or eglot-current-column-function
+                      (pcase (plist-get (eglot--capabilities 
(eglot-current-server))
+                                        :positionEncoding)
+                        ("utf-32" #'eglot-current-column)
+                        ("utf-8" #'eglot--current-column-utf-8)
+                        (_ #'eglot-lsp-abiding-column)))))
+    (eglot--widening
+     ;; LSP line is zero-origin; emacs is one-origin.
+     (list :line (1- (line-number-at-pos pos t))
+           :character (progn (when pos (goto-char pos))
+                             (funcall columnfn))))))
+

This is the heart of the patch.

A “good” server will provide :positionEncoding "utf-32", and we'll keep
the workaround variable `eglot-current-column-function' at its new default
value of nil.  So columnfn, which is called in the last line of the
chunck, will be bound to `eglot-current-column'.

A “bad” server will provide provide :positionEncoding "utf-16" or nil,
and then we will call `eglot-lsp-abiding-column' near the end.

An “inbetween” server will provide :positionEncoding "utf-8" and we will
call the newly added `eglot--current-column-utf-8' near the end.

If the user sets `eglot-current-column-function' to work around an
issue, nothing changes in relation to the original version.

-(defvar eglot-move-to-column-function #'eglot-move-to-lsp-abiding-column
+(defvar eglot-move-to-column-function nil
   "Function to move to a column reported by the LSP server.
 
I changed the default so this variable can be eventually made obsolete.
Note that it is a workaround variable introduced for the sole purpose of
making nonconforming servers work with Eglot.  But this problem should
slowly vanish with the introduction of the :positionEncoding capability.
Hence my suggestion to obsolete this workaround variable.

@@ -1478,11 +1493,11 @@ eglot-move-to-column-function
 `c'. However, many servers don't follow the spec this closely.
 
 For buffers managed by fully LSP-compliant servers, this should
-be set to `eglot-move-to-lsp-abiding-column' (the default), and
-`eglot-move-to-column' for all others.")
+be letft nil.  For others, it can be set to
+`eglot-move-to-column' or `eglot--move-to-column-utf-8'.")
 
 (defun eglot-move-to-column (column)
-  "Move to COLUMN without closely following the LSP spec."
+  "Move to COLUMN, counting Unicode codepoints."
   ;; We cannot use `move-to-column' here, because it moves to *visual*
   ;; columns, which can be different from LSP columns in case of
   ;; `whitespace-mode', `prettify-symbols-mode', etc.  (github#296,

The LSP spec now describes 3 ways of counting offsets, hence the
documenation clarification.

@@ -1490,8 +1505,14 @@ eglot-move-to-column
   (goto-char (min (+ (line-beginning-position) column)
                   (line-end-position))))
 
+(defun eglot--move-to-column-utf-8 (column)
+  "Move to COLUMN, regarded as a byte offset."
+  (goto-char (min (byte-to-position
+                   (+ (position-bytes (line-beginning-position)) column))
+                  (line-end-position))))
+

I defined a new function to support the style B. of counting offsets.

 (defun eglot-move-to-lsp-abiding-column (column)
-  "Move to COLUMN abiding by the LSP spec."
+  "Move to COLUMN, counting UTF-16 code units as in the original LSP spec."
   (save-restriction
     (cl-loop
      with lbp = (line-beginning-position)
@@ -1515,14 +1536,20 @@ eglot--lsp-position-to-point
       (forward-line (min most-positive-fixnum
                          (plist-get pos-plist :line)))
       (unless (eobp) ;; if line was excessive leave point at eob
-        (let ((tab-width 1)
+        (let ((movefn (or eglot-move-to-column-function
+                          (pcase (plist-get (eglot--capabilities 
(eglot-current-server))
+                                            :positionEncoding)
+                            ("utf-32" #'eglot-move-to-column)
+                            ("utf-8" #'eglot--move-to-column-utf-8)
+                            (_ #'eglot-move-to-lsp-abiding-column))))
+              (tab-width 1)
               (col (plist-get pos-plist :character)))
           (unless (wholenump col)
             (eglot--warn
              "Caution: LSP server sent invalid character position %s. Using 0 
instead."
              col)
             (setq col 0))
-          (funcall eglot-move-to-column-function col)))
+          (funcall movefn col)))
       (if marker (copy-marker (point-marker)) (point)))))

This is the second heart of the patch.

A “good” server will provide :positionEncoding "utf-32", and we'll keep
the workaround variable `eglot-move-to-column-function' at its new default
value of nil.  So columnfn, which is called in the last line of the
chunck, will be bound to `eglot-move-to-column'.

A “bad” server will provide provide :positionEncoding "utf-16" or nil,
and then we will call `eglot-lsp-abiding-column' near the end.

An “inbetween” server will provide :positionEncoding "utf-8" and we will
call the newly added `eglot--move-to-column-utf-8' near the end.

If the user sets `eglot-move-to-column-function' to work around an
issue, nothing changes in relation to the original version.



I hope this helps clarifying things.