bug#62687: [PATCH] Eglot: eglot--sig-info: Show SigInfo Docs if Markup; fix regex for highlighting; etc

[Mekeor Melire <mekeor@posteo.de>]

[-- Attachment #1: Type: text/plain, Size: 2706 bytes --]

Tags: patch

1. I already reported this issue with Eglot's eglot--sig-info:

    (when (and (stringp documentation)

It does not show the SignatureInformation's documentation if it's 
not a string but rather of type markup.

This issue is addressed by the attached patch.


2. I found another bug while reading the code:

    (concat "\\<" (regexp-quote label) "\\>")

Here, we try to find the ParameterInformation's label within the 
parameters. And that's the regex used for searching. 
Unfortunately, this regex won't match if the label has a non-word 
character at its beginning or end. Take a look at these examples:

    ELISP> (string-match "\\<bar: \"BAR\"" "foo(bar: \"BAR\", 
    baz)")
    4 (#o4, #x4, ?\C-d)

    ELISP> (string-match "\\<bar: \"BAR\"\\>" "foo(bar: \"BAR\", 
    baz)")
    nil

I tried to use "\\(\\`\\|\\W\\|\\=\\)" instead of "\\<" and 
"\\(\\'\\|\\W\\)" instead of "\\>", but that fixes the problem 
only for the first parameter, somehow. Maybe you have an idea 
which regex would fit here?

Until then, the attached patch just removes the "\\<" and "\\>" 
parts.


3. eglot--sig-info does not highlight the active parameter if it 
does not match the <name>(<params>) pattern. Here's the 
responsible code:

    ;; Ad-hoc attempt to parse label as <name>(<params>)
    (when (looking-at "\\([^(]*\\)(\\([^)]+\\))")
      (setq params-start (match-beginning 2) params-end (match-end 
      2))
      ;; ...
      )
    (when params-start
      ;; ...
      (add-face-text-property
        beg end
        'eldoc-highlight-function-argument))

But we are in fact able to highlight the active parameter, if 
ParameterInformation's label is a pair of numbers, denoting the 
active parameter's position. We just need to nest our conditions 
the other way around.

This issue is addressed by the attached patch.


4. The "documentation" field of both SignatureInformation and 
ParameterInformation can be either of type string or of type 
MarkupContent, according to LSP 3.17. I think we should not format 
the documentation as GitHub-Flavored-Markdown (GFM) because it 
might lead to wrong interpretations/displayings of the doc-string.

This is addressed by the attached patch.


5. There is a little overhead on pattern matching on a pair. 
Instead of

    ((`(,beg ,end)
      (if COND
        (list FOO BAR)
        (append CONS-CELL nil))))

we could simply:

    ((`(,beg . ,end)
      (if COND
        (cons FOO BAR)
        CONS-CELL)))

This is also addressed in the attached patch.


6. Using ": " and "\n" to separate information is not enough of 
separation. This is fixed by the attached patch by using "\n\n" 
instead. Should we make it a customizable variable?



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 62687.patch --]
[-- Type: text/patch, Size: 4593 bytes --]

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 10b6c0cc2ca..42989b9e73e 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -3120,6 +3120,7 @@ for which LSP on-type-formatting should be requested."
   (eglot--dbind ((SignatureInformation) label documentation parameters activeParameter)
       sig
     (with-temp-buffer
+      ;; Insert SignatureInformation label
       (save-excursion (insert label))
       (let ((active-param (or activeParameter sig-help-active-param))
             params-start params-end)
@@ -3128,44 +3129,40 @@ for which LSP on-type-formatting should be requested."
           (setq params-start (match-beginning 2) params-end (match-end 2))
           (add-face-text-property (match-beginning 1) (match-end 1)
                                   'font-lock-function-name-face))
-        ;; Decide whether to add one-line-summary to signature line
-        (when (and (stringp documentation)
-                   (string-match "[[:space:]]*\\([^.\r\n]+[.]?\\)"
-                                 documentation))
-          (setq documentation (match-string 1 documentation))
-          (unless (string-prefix-p (string-trim documentation) label)
-            (goto-char (point-max))
-            (insert ": " (eglot--format-markup documentation))))
+        ;; Insert SignatureInformation documentation
+        (goto-char (point-max))
+        (unless (null documentation)
+          (insert "\n\n"
+            (if (stringp documentation)
+              (string-trim documentation)
+              (eglot--format-markup documentation))))
         ;; Decide what to do with the active parameter...
         (when (and active-param (< -1 active-param (length parameters)))
           (eglot--dbind ((ParameterInformation) label documentation)
               (aref parameters active-param)
             ;; ...perhaps highlight it in the formals list
-            (when params-start
-              (goto-char params-start)
-              (pcase-let
-                  ((`(,beg ,end)
-                    (if (stringp label)
-                        (let ((case-fold-search nil))
-                          (and (re-search-forward
-                                (concat "\\<" (regexp-quote label) "\\>")
-                                params-end t)
-                               (list (match-beginning 0) (match-end 0))))
-                      (mapcar #'1+ (append label nil)))))
-                (if (and beg end)
-                    (add-face-text-property
-                     beg end
-                     'eldoc-highlight-function-argument))))
-            ;; ...and/or maybe add its doc on a line by its own.
+            (pcase-let
+              ((`(,beg . ,end)
+                 (if (stringp label)
+                   (if params-start
+                     (let ((case-fold-search nil))
+                       (goto-char params-start)
+                       (and (re-search-forward
+                              (regexp-quote label)
+                              params-end t)
+                         (cons (match-beginning 0) (match-end 0)))))
+                   (mapcar #'1+ label))))
+              (if (and beg end)
+                (add-face-text-property
+                  beg end
+                  'eldoc-highlight-function-argument)))
+            ;; ...and maybe add its doc on a line by its own.
             (when documentation
               (goto-char (point-max))
-              (insert "\n"
-                      (propertize
-                       (if (stringp label)
-                           label
-                         (apply #'buffer-substring (mapcar #'1+ label)))
-                       'face 'eldoc-highlight-function-argument)
-                      ": " (eglot--format-markup documentation))))))
+              (insert "\n\n"
+                (if (stringp documentation)
+                  (string-trim documentation)
+                  (eglot--format-markup documentation)))))))
       (buffer-string))))
 
 (defun eglot-signature-eldoc-function (cb)
@@ -3183,7 +3180,7 @@ for which LSP on-type-formatting should be requested."
                                   (aref signatures (or activeSignature 0)))))
              (if (not active-sig) (funcall cb nil)
                (funcall cb
-                        (mapconcat #'eglot--sig-info signatures "\n")
+                        (mapconcat #'eglot--sig-info signatures "\n\n")
                         :echo (eglot--sig-info active-sig t activeParameter))))))
        :deferred :textDocument/signatureHelp))
     t))