From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Mekeor Melire Newsgroups: gmane.emacs.bugs Subject: bug#62687: [PATCH] Eglot: eglot--sig-info: Show SigInfo Docs if Markup; fix regex for highlighting; etc Date: Fri, 07 Apr 2023 22:40:16 +0000 Message-ID: <87fs9bmg1j.fsf@posteo.de> References: <875yaanfuv.fsf@posteo.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="8792"; mail-complaints-to="usenet@ciao.gmane.io" To: 62687@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Apr 08 01:27:19 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pkvUF-00025j-Fd for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 08 Apr 2023 01:27:19 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pkvU2-00056R-8w; Fri, 07 Apr 2023 19:27:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pkvTz-00055s-TE for bug-gnu-emacs@gnu.org; Fri, 07 Apr 2023 19:27:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pkvTy-0007cF-HW for bug-gnu-emacs@gnu.org; Fri, 07 Apr 2023 19:27:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pkvTy-0003FM-Cb for bug-gnu-emacs@gnu.org; Fri, 07 Apr 2023 19:27:02 -0400 X-Loop: help-debbugs@gnu.org In-Reply-To: <875yaanfuv.fsf@posteo.de> Resent-From: Mekeor Melire Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 07 Apr 2023 23:27:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 62687 X-GNU-PR-Package: emacs Original-Received: via spool by 62687-submit@debbugs.gnu.org id=B62687.168091000912462 (code B ref 62687); Fri, 07 Apr 2023 23:27:02 +0000 Original-Received: (at 62687) by debbugs.gnu.org; 7 Apr 2023 23:26:49 +0000 Original-Received: from localhost ([127.0.0.1]:57416 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pkvTk-0003Ev-Oe for submit@debbugs.gnu.org; Fri, 07 Apr 2023 19:26:49 -0400 Original-Received: from mout01.posteo.de ([185.67.36.65]:43359) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pkvTi-0003Eh-1Y for 62687@debbugs.gnu.org; Fri, 07 Apr 2023 19:26:47 -0400 Original-Received: from submission (posteo.de [185.67.36.169]) by mout01.posteo.de (Postfix) with ESMTPS id 87CC4240108 for <62687@debbugs.gnu.org>; Sat, 8 Apr 2023 01:26:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=posteo.de; s=2017; t=1680909999; bh=wnynKRtHzWDb69aw09EFYgIeec7eJXyHZRdLvvX+Hgg=; h=From:To:Subject:Date:From; b=SImQXxpGrRLIDIh1OtYzes9u4+GCs8/X/IFGTJX5Tw/8V/yQoPY6CrPxPBO2kJonC 61GYEhO7hB+faeJcW+S5cn7rvm33P7GduHqu/fEvG31IN3pFk8hgrXUTYZgStAg1X1 yQhvbZDznUhJYN1LPrXlTA3dS+BU/Kh7CKcO1fl4DSYJlZXHddxDPww1M4udZ1CblX z+KqXXOTCQEt3owTJuc9HJho8I3LpH1AEZ0PFCka/E7IqGQ/m5JC3jzDQz7Hg67wRa wN4FxCjW+zek9QvchNLc7q8tZnsLozZCYXkCifiu2qqHdUdtOCXTiZpuc+AagFgRPH OcZhbfXZTu4hQ== Original-Received: from customer (localhost [127.0.0.1]) by submission (posteo.de) with ESMTPSA id 4PtZJG5WzBz6tvd; Sat, 8 Apr 2023 01:26:38 +0200 (CEST) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:259446 Archived-At: --=-=-= Content-Type: text/plain; format=flowed 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 "\\ (string-match "\\" "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 () pattern. Here's the responsible code: ;; Ad-hoc attempt to parse label as () (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? --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=62687.patch 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)) --=-=-=--