unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: "João Távora" <joaotavora@gmail.com>
To: Chinmay Dalal <dalal.chinmay.0101@gmail.com>,
	Dimitri Belopopsky <dimitri@belopopsky.com>,
	Po Lu <luangruo@yahoo.com>
Cc: Eli Zaretskii <eliz@gnu.org>, 61412@debbugs.gnu.org
Subject: bug#61412: Inlay hints implementation
Date: Tue, 21 Feb 2023 15:13:03 +0000	[thread overview]
Message-ID: <CALDnm53otfeDQGr0dWWUhxGLTSuiWTstLXJz1HXQgWLiAgsk=A@mail.gmail.com> (raw)
In-Reply-To: <2B284D77-97DF-4B3E-89FB-13F0CA93D240@gmail.com>

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

[Eli, I just noticed that this bug should be merged with 61066,
but I don't know how to do that]

Hello,

Attached is my implementation of inlay hints for Eglot.
Two patches are provided, where the first just lays some
basic groundwork to make the actual inlay hint implementation
simpler.

Regarding copyright stuff, I did look at Chinmay's patch,
but I re-started from scratch.  While it was a very good
effort, there were too many idiomatic Elisp and Eglot things
to change.  I did take Chinmay's face definitions, though.
Not sure how to proceed here and if this counts as "derived
work" and if we should wait for Chinmay's copyright assignment.

I gave it some light testing and I kind of like it.  Quite
helpful for C++ with clangd (the only server I tested it with).
You can bind `eglot-inlay-hint-mode` to some keybinding probably.

Documentation in the manual is still missing, but shouldn't
be very hard to do.

Anyway, this is likely not the end of the inlay hint story
because, as the second patch documents, this is likely a
very naive implementation that always requests inlay hints
for the entire buffer even if just a fraction of it is visible.

A better implementation would probably leverage
window-scroll-functions along with the Eglot-specific
idle timer.  That is probably much, much more tricky to get
right, but is also more than likely the way to go.

In the meantime, I'd like your opinion on this patch and
the above topics first.

João

I hope gmail doesn't mess up my attachments...

[-- Attachment #2: 0001-Eglot-simplify-capability-checking-code.patch --]
[-- Type: application/octet-stream, Size: 5051 bytes --]

From cacedd67873bfb094d12c08fc23e017fa4de5df1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Tue, 21 Feb 2023 13:59:04 +0000
Subject: [PATCH 1/2] Eglot: simplify capability-checking code

* lisp/progmodes/eglot.el (eglot--server-capable-or-lose): New helper.
(eglot--signal-textDocument/willSave)
(eglot--signal-textDocument/didSave): Tweak docstring.
(eglot--workspace-symbols, xref-backend-identifier-at-point)
(eglot-format, eglot-completion-at-point, eglot-rename)
(eglot-code-actions): Use new eglot--server-capable-or-lose.
---
 lisp/progmodes/eglot.el | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 82401b685ce..45f00daca1f 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -1641,6 +1641,13 @@ eglot--server-capable
              if (not (listp (cadr probe))) do (cl-return (if more nil (cadr probe)))
              finally (cl-return (or (cadr probe) t)))))
 
+(defun eglot--server-capable-or-lose (&rest feats)
+  "Like `eglot--server-capable', but maybe error out."
+  (let ((retval (apply #'eglot--server-capable feats)))
+    (unless retval
+      (eglot--error "This LSP server isn't capable of %s" feats))
+    retval))
+
 (defun eglot--range-region (range &optional markers)
   "Return region (BEG . END) that represents LSP RANGE.
 If optional MARKERS, make markers."
@@ -2482,7 +2489,7 @@ eglot--signal-textDocument/didClose
      :textDocument/didClose `(:textDocument ,(eglot--TextDocumentIdentifier)))))
 
 (defun eglot--signal-textDocument/willSave ()
-  "Send textDocument/willSave to server."
+  "Maybe send textDocument/willSave to server."
   (let ((server (eglot--current-server-or-lose))
         (params `(:reason 1 :textDocument ,(eglot--TextDocumentIdentifier))))
     (when (eglot--server-capable :textDocumentSync :willSave)
@@ -2494,7 +2501,7 @@ eglot--signal-textDocument/willSave
                           :timeout 0.5))))))
 
 (defun eglot--signal-textDocument/didSave ()
-  "Send textDocument/didSave to server."
+  "Maybe send textDocument/didSave to server."
   (eglot--signal-textDocument/didChange)
   (when (eglot--server-capable :textDocumentSync :save)
     (jsonrpc-notify
@@ -2591,8 +2598,7 @@ eglot--workspace-symbols
   "Ask for :workspace/symbol on PAT, return list of formatted strings.
 If BUFFER, switch to it before."
   (with-current-buffer (or buffer (current-buffer))
-    (unless (eglot--server-capable :workspaceSymbolProvider)
-      (eglot--error "This LSP server isn't a :workspaceSymbolProvider"))
+    (eglot--server-capable-or-lose :workspaceSymbolProvider)
     (mapcar
      (lambda (wss)
        (eglot--dbind ((WorkspaceSymbol) name containerName kind) wss
@@ -2654,13 +2660,12 @@ eglot--lsp-xref-refs
 
 (cl-defun eglot--lsp-xrefs-for-method (method &key extra-params capability)
   "Make `xref''s for METHOD, EXTRA-PARAMS, check CAPABILITY."
-  (unless (eglot--server-capable
-           (or capability
-               (intern
-                (format ":%sProvider"
-                        (cadr (split-string (symbol-name method)
-                                            "/"))))))
-    (eglot--error "Sorry, this server doesn't do %s" method))
+  (eglot--server-capable-or-lose
+   (or capability
+       (intern
+        (format ":%sProvider"
+                (cadr (split-string (symbol-name method)
+                                    "/"))))))
   (let ((response
          (jsonrpc-request
           (eglot--current-server-or-lose)
@@ -2757,8 +2762,7 @@ eglot-format
                                   :end (eglot--pos-to-lsp-position end)))))
                 (t
                  '(:textDocument/formatting :documentFormattingProvider nil)))))
-    (unless (eglot--server-capable cap)
-      (eglot--error "Server can't format!"))
+    (eglot--server-capable-or-lose cap)
     (eglot--apply-text-edits
      (jsonrpc-request
       (eglot--current-server-or-lose)
@@ -3202,8 +3206,7 @@ eglot-rename
                                          "unknown symbol"))
           nil nil nil nil
           (symbol-name (symbol-at-point)))))
-  (unless (eglot--server-capable :renameProvider)
-    (eglot--error "Server can't rename!"))
+  (eglot--server-capable-or-lose :renameProvider)
   (eglot--apply-workspace-edit
    (jsonrpc-request (eglot--current-server-or-lose)
                     :textDocument/rename `(,@(eglot--TextDocumentPositionParams)
@@ -3230,9 +3233,7 @@ eglot-code-actions
                             '("quickfix" "refactor.extract" "refactor.inline"
                               "refactor.rewrite" "source.organizeImports")))
      t))
-  (unless (or (not interactive)
-              (eglot--server-capable :codeActionProvider))
-    (eglot--error "Server can't execute code actions!"))
+  (eglot--server-capable-or-lose :codeActionProvider)
   (let* ((server (eglot--current-server-or-lose))
          (actions
           (jsonrpc-request
-- 
2.36.1.windows.1


[-- Attachment #3: 0002-Eglot-implement-inlay-hints-bug-61412.patch --]
[-- Type: application/octet-stream, Size: 6591 bytes --]

From fec508d6a8cab778649623d3bbb2954162b4c625 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Tue, 21 Feb 2023 14:14:05 +0000
Subject: [PATCH 2/2] Eglot: implement inlay hints (bug#61412)

This implementation is probably too slow for complex situations and
big files since it requests inlay hints for the whole buffer as which
can possibly mean a large amount of data.

A better implementation will be smart enough to only request inlay
hints for the visible parts of the buffer.

* lisp/progmodes/eglot.el (eglot--lsp-interface-alist): Define
InlayHint.
(eglot-client-capabilities): Announce 'inlayHint' capability.
(eglot-ignored-server-capabilities): Add :inlayHintProvider.
(eglot--document-changed-hook): New helper hook.
(eglot--after-change): Use it.
(eglot-inlay-hint-face, eglot-type-hint-face)
(eglot-parameter-hint-face): New faces.
(eglot--update-hints, eglot--paint-hint): New helpers.
(eglot-inlay-hints-mode): New minor mode.
---
 lisp/progmodes/eglot.el | 71 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 68 insertions(+), 3 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 45f00daca1f..c0c5dea29be 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -483,7 +483,9 @@ eglot--executable-find
       (VersionedTextDocumentIdentifier (:uri :version) ())
       (WorkDoneProgress (:kind) (:title :message :percentage :cancellable))
       (WorkspaceEdit () (:changes :documentChanges))
-      (WorkspaceSymbol (:name :kind) (:containerName :location :data)))
+      (WorkspaceSymbol (:name :kind) (:containerName :location :data))
+      (InlayHint (:position :label) (:kind :textEdits :tooltip :paddingLeft
+                                           :paddingRight :data)))
     "Alist (INTERFACE-NAME . INTERFACE) of known external LSP interfaces.
 
 INTERFACE-NAME is a symbol designated by the spec as
@@ -803,6 +805,7 @@ eglot-client-capabilities
              :formatting         `(:dynamicRegistration :json-false)
              :rangeFormatting    `(:dynamicRegistration :json-false)
              :rename             `(:dynamicRegistration :json-false)
+             :inlayHint          `(:dynamicRegistration :json-false)
              :publishDiagnostics (list :relatedInformation :json-false
                                        ;; TODO: We can support :codeDescription after
                                        ;; adding an appropriate UI to
@@ -1625,7 +1628,8 @@ eglot-ignored-server-capabilities
           (const :tag "Highlight links in document" :documentLinkProvider)
           (const :tag "Decorate color references" :colorProvider)
           (const :tag "Fold regions of buffer" :foldingRangeProvider)
-          (const :tag "Execute custom commands" :executeCommandProvider)))
+          (const :tag "Execute custom commands" :executeCommandProvider)
+          (const :tag "Inlay hints" :inlayHintProvider)))
 
 (defun eglot--server-capable (&rest feats)
   "Determine if current server is capable of FEATS."
@@ -2291,6 +2295,9 @@ eglot--before-change
             (,end . ,(copy-marker end t)))
           eglot--recent-changes)))
 
+(defvar eglot--document-changed-hook '(eglot--signal-textDocument/didChange)
+  "Internal hook for doing things when the document changes.")
+
 (defun eglot--after-change (beg end pre-change-length)
   "Hook onto `after-change-functions'.
 Records BEG, END and PRE-CHANGE-LENGTH locally."
@@ -2331,7 +2338,7 @@ eglot--after-change
            eglot-send-changes-idle-time
            nil (lambda () (eglot--when-live-buffer buf
                             (when eglot--managed-mode
-                              (eglot--signal-textDocument/didChange)
+                              (run-hooks 'eglot--document-changed-hook)
                               (setq eglot--change-idle-timer nil))))))))
 
 ;; HACK! Launching a deferred sync request with outstanding changes is a
@@ -3459,6 +3466,64 @@ eglot-list-connections
       (revert-buffer)
       (pop-to-buffer (current-buffer)))))
 
+\f
+;;; Inlay hints
+(defface eglot-inlay-hint-face '((t (:height 0.8 :inherit shadow)))
+  "Face used for inlay hint overlays.")
+
+(defface eglot-type-hint-face '((t (:inherit eglot-inlay-hint-face)))
+  "Face used for type inlay hint overlays.")
+
+(defface eglot-parameter-hint-face '((t (:inherit eglot-inlay-hint-face)))
+  "Face used for parameter inlay hint overlays.")
+
+(defun eglot--paint-hint (hint)
+  "Paint inlay hint HINT in current buffer."
+  (eglot--dbind ((InlayHint) position kind label paddingLeft paddingRight) hint
+    (goto-char (eglot--lsp-position-to-point position))
+    (let ((ov (make-overlay (point) (point))))
+      (overlay-put ov 'before-string
+                   (propertize
+                    (concat (and paddingLeft " ")
+                            (if (stringp label) label (plist-get label :value))
+                            (and paddingRight " "))
+                    'face (pcase kind
+                            (1 'eglot-type-hint-face)
+                            (2 'eglot-parameter-hint-face)
+                            (_ 'eglot-inlay-hint-face))))
+      (overlay-put ov 'eglot--inlay-hint t))))
+
+(defun eglot--update-hints ()
+  "Request inlay hints for current buffer and paint them."
+  (interactive)
+  (eglot--server-capable-or-lose :inlayHintProvider)
+  (let ((buffer (current-buffer)))
+    (jsonrpc-async-request
+      (eglot--current-server-or-lose)
+      :textDocument/inlayHint
+      (list :textDocument (eglot--TextDocumentIdentifier)
+            :range (list :start (eglot--pos-to-lsp-position (point-min))
+                         :end (eglot--pos-to-lsp-position (point-max))))
+      :success-fn (lambda (hints)
+                    (with-current-buffer buffer
+                      (eglot--widening
+                       (remove-overlays nil nil 'eglot--inlay-hint t)
+                       (mapc #'eglot--paint-hint hints))))
+      :deferred t)))
+
+(define-minor-mode eglot-inlay-hints-mode
+  "Minor mode annotating buffer with LSP inlay hints."
+  :global nil
+  (if eglot-inlay-hints-mode
+      (if (not eglot--managed-mode)
+          (eglot--error "Can't turn on `eglot-inlay-hints' mode.")
+        (add-hook 'eglot--document-changed-hook
+                  'eglot--update-hints nil t)
+        (eglot--update-hints))
+    (remove-hook 'eglot--document-changed-hook
+                 'eglot--update-hints t)
+    (remove-overlays nil nil 'eglot--inlay-hint t)))
+
 \f
 ;;; Hacks
 ;;;
-- 
2.36.1.windows.1


  reply	other threads:[~2023-02-21 15:13 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-11  8:13 bug#61412: [PATCH] Add inlay hints to eglot Chinmay Dalal
2023-02-11 11:50 ` Chinmay Dalal
2023-02-11 15:23 ` bug#61412: Question about implementation Chinmay Dalal
2023-02-13 15:35 ` bug#61412: Hover github discussion Chinmay Dalal
2023-02-15 13:58   ` Eli Zaretskii
2023-02-15 15:05     ` João Távora
2023-02-15 15:38       ` Eli Zaretskii
2023-02-15 12:56 ` bug#61412: [PATCH v2] Add inlay hints to eglot Chinmay Dalal
2023-02-15 13:08 ` bug#61412: [PATCH v3] " Chinmay Dalal
2023-02-15 16:24 ` bug#61412: Inlay activation Chinmay Dalal
2023-02-15 18:09   ` Eli Zaretskii
2023-02-15 18:48     ` Chinmay Dalal
2023-02-15 19:01       ` João Távora
2023-02-15 19:17         ` Chinmay Dalal
2023-02-15 19:41       ` Eli Zaretskii
2023-02-15 20:17         ` Chinmay Dalal
2023-02-21 15:13           ` João Távora [this message]
2023-02-21 15:21             ` bug#61412: Inlay hints implementation Eli Zaretskii
2023-02-21 18:42             ` Dimitri Belopopsky
2023-02-21 21:26               ` João Távora
2023-02-25  0:21                 ` João Távora
2023-02-25  7:59                   ` Eli Zaretskii
2023-02-25 10:19                     ` João Távora
2023-02-21 15:33 ` Chinmay Dalal
2023-02-21 15:57 ` Chinmay Dalal
2023-02-22 15:26 ` Chinmay Dalal
2023-02-22 16:51   ` Chinmay Dalal
2023-02-22 23:17   ` João Távora

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CALDnm53otfeDQGr0dWWUhxGLTSuiWTstLXJz1HXQgWLiAgsk=A@mail.gmail.com' \
    --to=joaotavora@gmail.com \
    --cc=61412@debbugs.gnu.org \
    --cc=dalal.chinmay.0101@gmail.com \
    --cc=dimitri@belopopsky.com \
    --cc=eliz@gnu.org \
    --cc=luangruo@yahoo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).