unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#61066: [PATCH] Add inlay hint support to eglot
@ 2023-01-25 22:34 Dimitri Belopopsky
  2023-01-26  6:29 ` Eli Zaretskii
  0 siblings, 1 reply; 5+ messages in thread
From: Dimitri Belopopsky @ 2023-01-25 22:34 UTC (permalink / raw)
  To: 61066


[-- Attachment #1.1: Type: text/plain, Size: 823 bytes --]

Hello,

I've been working on adding support for inlay hints inside eglot using
overlays.
Here is a working patch, but I'm still missing a few things:

- I can't figure out a way to show the hints on a document without causing
lags or timeouts from the lsp server
- I'm currently updating the hints by sending the whole file each time (to
make sure all hints get updated correctly). I'm not sure on how to make
this more efficient (or if it even necessary).

On the implementation side:
- implemented with overlays as a minor model, enabled by default
- shows all hints supported by the protocol
- there is a customisation to disable the minor mode if the user doesn't
want the feature

I'd love to get a few points to finish this patch, and of course any ideas
and feedbacks are welcome!

Kind regards,

Dimitri Belopopsky

[-- Attachment #1.2: Type: text/html, Size: 1048 bytes --]

[-- Attachment #2: 0001-Add-inlay-hints-to-eglot.patch --]
[-- Type: text/x-patch, Size: 5008 bytes --]

From 848fa16eb6257ce454c1810f81be8ed8ed140164 Mon Sep 17 00:00:00 2001
From: Dimitri Belopopsky <dimitri@belopopsky.com>
Date: Wed, 25 Jan 2023 23:24:13 +0100
Subject: [PATCH] Add inlay hints to eglot

---
 lisp/progmodes/eglot.el | 88 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 87 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 8ce1a8b7baf..2d947c377a4 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -404,6 +404,11 @@ eglot-withhold-process-id
 (when (assoc 'flex completion-styles-alist)
   (add-to-list 'completion-category-defaults '(eglot (styles flex basic))))
 
+(defcustom eglot-inlay-hints t
+  "If non-nil, activate inlay hints inside the buffer."
+  :type 'boolean)
+
+
 \f
 ;;; Constants
 ;;;
@@ -1624,7 +1629,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 Hint" :inlayHintProvider)))
 
 (defun eglot--server-capable (&rest feats)
   "Determine if current server is capable of FEATS."
@@ -1845,6 +1851,9 @@ eglot--maybe-activate-editing-mode
     (when (and buffer-file-name (eglot-current-server))
       (setq eglot--diagnostics nil)
       (eglot--managed-mode)
+      (unless (not eglot-inlay-hints)
+        (eglot-inlay-mode)
+        )
       (eglot--signal-textDocument/didOpen))))
 
 (add-hook 'find-file-hook 'eglot--maybe-activate-editing-mode)
@@ -3453,6 +3462,83 @@ eglot-list-connections
       (revert-buffer)
       (pop-to-buffer (current-buffer)))))
 
+(defface eglot-inlay-hint
+  '((t (:height 0.8 :inherit shadow)))
+  "Face used for inlay hint overlays.")
+
+(define-minor-mode eglot-inlay-mode
+  "Mode for displaying inlay hint."
+  :lighter " inlay"
+  (cond
+    (eglot--managed-mode
+      (add-hook 'after-save-hook 'eglot--update-hints nil t))
+    (t
+      (remove-hook 'after-save-hook 'eglot--update-hints t))
+    )
+  ;; Note: the public hook runs before the internal eglot--managed-mode-hook.
+  (run-hooks 'eglot-managed-mode-hook)
+  )
+
+(defun eglot--update-hints()
+  "Refresh inlay hints from LSP server."
+  (unless (eglot--server-capable :inlayHintProvider)
+    (error "Server does not support inlay hint."))
+  (mapc #'delete-overlay (overlays-in (point-min) (point-max)))
+  (let ((read-only-p buffer-read-only)
+	       overlays)
+	    (let ((lens-table (make-hash-table)))
+	      ;; Get the inlay hint objects.
+	      (mapc (lambda (inlayHint)
+		            (when (eglot--server-capable
+			                       :inlayHintProvider :resolveProvider)
+		              (setq inlayHint
+			              (jsonrpc-request (eglot--current-server-or-lose)
+					            :inlayHint/resolve inlayHint)))
+		            (let ((line (thread-first inlayHint
+					                    (plist-get :position)
+					                    (plist-get :line))))
+		              (puthash line
+			              (append (gethash line lens-table) (list inlayHint))
+			              lens-table)))
+		      (jsonrpc-request
+		        (eglot--current-server-or-lose)
+		        :textDocument/inlayHint
+            (list :textDocument (eglot--TextDocumentIdentifier) :range (list :start (list :line 0 :character 0) :end (list :line (count-lines (point-min) (point-max)) :character 0)))
+		        :deferred t))
+
+	      ;; Make overlays for them.
+	      (maphash
+	        (lambda (line values)
+            (cl-loop for value in values
+              do
+              (cl-loop for val being the elements of
+                (if (stringp (plist-get value :label))
+                  (list (plist-get value :label))
+                  (plist-get value :label))
+                do
+	              (eglot--widening
+	                (let ((c (plist-get (plist-get value :position) :character))
+		                     (text (propertize
+                                 (concat
+                                   (if (plist-get value :paddingLeft) " " "")
+                                   (if (stringp val) val (plist-get val :value))
+                                   (if (plist-get value :paddingRight) " " ""))
+                           'mouse-face 'highlight))
+                         )
+	              (goto-char (point-min))
+	              (forward-line line)
+	              (eglot-move-to-column c)
+	              (let ((ov (make-overlay (point) (point))))
+		              (push ov overlays)
+		              (overlay-put ov 'eglot-inlay-hint values)
+		              (overlay-put ov 'before-string (propertize text 'face 'eglot-inlay-hint))
+                  )))
+                )))
+	        lens-table)
+        )
+    ))
+
+
 \f
 ;;; Hacks
 ;;;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* bug#61066: [PATCH] Add inlay hint support to eglot
  2023-01-25 22:34 bug#61066: [PATCH] Add inlay hint support to eglot Dimitri Belopopsky
@ 2023-01-26  6:29 ` Eli Zaretskii
  2023-01-26 12:30   ` João Távora
  2023-01-28  1:58   ` Dmitry Gutov
  0 siblings, 2 replies; 5+ messages in thread
From: Eli Zaretskii @ 2023-01-26  6:29 UTC (permalink / raw)
  To: Dimitri Belopopsky, João Távora; +Cc: 61066

> From: Dimitri Belopopsky <dimitri@belopopsky.com>
> Date: Wed, 25 Jan 2023 23:34:02 +0100
> 
> I've been working on adding support for inlay hints inside eglot using overlays.
> Here is a working patch, but I'm still missing a few things:
> 
> - I can't figure out a way to show the hints on a document without causing lags or timeouts from the lsp
> server
> - I'm currently updating the hints by sending the whole file each time (to make sure all hints get updated
> correctly). I'm not sure on how to make this more efficient (or if it even necessary).
> 
> On the implementation side:
> - implemented with overlays as a minor model, enabled by default
> - shows all hints supported by the protocol
> - there is a customisation to disable the minor mode if the user doesn't want the feature
> 
> I'd love to get a few points to finish this patch, and of course any ideas and feedbacks are welcome!

Thank you for working on this important feature.

AFAIU, inlay hints provide information of the same kind as ElDoc and
in similar manner from the display and UX POV.  So I think this
feature should work via ElDoc, not as a separate from-the-scratch
implementation.  ElDoc is already capable of using Eglot-supplied
information, so perhaps the only feature we need to add is the
capability of ElDoc to (optionally) display the information in
overlays near point.  (I thought we already had such a capability in
eldoc.el, but it looks like I was dreaming, because I cannot find it
there.)

The advantage of basing this on ElDoc is that then we will be able to
provide similar features from information sources other than Eglot.

João, WDYT?





^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#61066: [PATCH] Add inlay hint support to eglot
  2023-01-26  6:29 ` Eli Zaretskii
@ 2023-01-26 12:30   ` João Távora
  2023-01-27 18:44     ` Dimitri Belopopsky
  2023-01-28  1:58   ` Dmitry Gutov
  1 sibling, 1 reply; 5+ messages in thread
From: João Távora @ 2023-01-26 12:30 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Dimitri Belopopsky, 61066

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

On Thu, Jan 26, 2023 at 6:28 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Dimitri Belopopsky <dimitri@belopopsky.com>
> > Date: Wed, 25 Jan 2023 23:34:02 +0100
> >
> > I've been working on adding support for inlay hints inside eglot using
> overlays.
> > Here is a working patch, but I'm still missing a few things:
> >
> > - I can't figure out a way to show the hints on a document without
> causing lags or timeouts from the lsp
> > server
> > - I'm currently updating the hints by sending the whole file each time
> (to make sure all hints get updated
> > correctly). I'm not sure on how to make this more efficient (or if it
> even necessary).
> >
> > On the implementation side:
> > - implemented with overlays as a minor model, enabled by default
> > - shows all hints supported by the protocol
> > - there is a customisation to disable the minor mode if the user doesn't
> want the feature
> >
> > I'd love to get a few points to finish this patch, and of course any
> ideas and feedbacks are welcome!
>
> Thank you for working on this important feature.
>
> AFAIU, inlay hints provide information of the same kind as ElDoc and
> in similar manner from the display and UX POV.  So I think this
> feature should work via ElDoc, not as a separate from-the-scratch
> implementation.  ElDoc is already capable of using Eglot-supplied
> information, so perhaps the only feature we need to add is the
> capability of ElDoc to (optionally) display the information in
> overlays near point.  (I thought we already had such a capability in
> eldoc.el, but it looks like I was dreaming, because I cannot find it
> there.)
>
> The advantage of basing this on ElDoc is that then we will be able to
> provide similar features from information sources other than Eglot.
>
> João, WDYT?
>

I think you're mostly right in your analysis.  I also think this should be
provided
via some LSP-agnostic infrastructure and not as a separate from-the-scratch
implementation, as you put it. This is (and was) also my stance in other
similar
matters.

The devil is in the details, of course.  Is eldoc.el the right
infrastructure
for it. What augmentation does it need, if any?  Are these augmentations
practical given the relative size, complexity and history of eldoc.el?
Isn't
an "inlay.el" or a "little-hint.el" support library a better choice? Or
maybe
flymake.el with it's use of overlay-based annotations is also acceptable
here?  I don't (yet) have answers to these questions.

Of course the existence of this prototype by Dimitri is certainly
No Bad Thing and a good starting point.

João

[-- Attachment #2: Type: text/html, Size: 3369 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#61066: [PATCH] Add inlay hint support to eglot
  2023-01-26 12:30   ` João Távora
@ 2023-01-27 18:44     ` Dimitri Belopopsky
  0 siblings, 0 replies; 5+ messages in thread
From: Dimitri Belopopsky @ 2023-01-27 18:44 UTC (permalink / raw)
  To: João Távora; +Cc: Eli Zaretskii, 61066

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

Hello,

I don't have much experience with hacking emacs, but I was thinking the
current code could also be reused
for the code lens LSP feature. Maybe there are other features that could be
done with the same kind
of setup?

This (to me anyway) would probably mean eldoc isn't the best option for a
more general solution.
Though I expect having overlays for eldoc would be beneficial as well.

I don't really know which is best between a library or integration inside
flymake. I might try out both
ideas, as I have no experience with either of them. My guess flymake sounds
easier to implement, but
a support library could probably be more flexible for reuse?

Dimitri

On Thu, 26 Jan 2023 at 13:29, João Távora <joaotavora@gmail.com> wrote:

> On Thu, Jan 26, 2023 at 6:28 AM Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Dimitri Belopopsky <dimitri@belopopsky.com>
>> > Date: Wed, 25 Jan 2023 23:34:02 +0100
>> >
>> > I've been working on adding support for inlay hints inside eglot using
>> overlays.
>> > Here is a working patch, but I'm still missing a few things:
>> >
>> > - I can't figure out a way to show the hints on a document without
>> causing lags or timeouts from the lsp
>> > server
>> > - I'm currently updating the hints by sending the whole file each time
>> (to make sure all hints get updated
>> > correctly). I'm not sure on how to make this more efficient (or if it
>> even necessary).
>> >
>> > On the implementation side:
>> > - implemented with overlays as a minor model, enabled by default
>> > - shows all hints supported by the protocol
>> > - there is a customisation to disable the minor mode if the user
>> doesn't want the feature
>> >
>> > I'd love to get a few points to finish this patch, and of course any
>> ideas and feedbacks are welcome!
>>
>> Thank you for working on this important feature.
>>
>> AFAIU, inlay hints provide information of the same kind as ElDoc and
>> in similar manner from the display and UX POV.  So I think this
>> feature should work via ElDoc, not as a separate from-the-scratch
>> implementation.  ElDoc is already capable of using Eglot-supplied
>> information, so perhaps the only feature we need to add is the
>> capability of ElDoc to (optionally) display the information in
>> overlays near point.  (I thought we already had such a capability in
>> eldoc.el, but it looks like I was dreaming, because I cannot find it
>> there.)
>>
>> The advantage of basing this on ElDoc is that then we will be able to
>> provide similar features from information sources other than Eglot.
>>
>> João, WDYT?
>>
>
> I think you're mostly right in your analysis.  I also think this should be
> provided
> via some LSP-agnostic infrastructure and not as a separate
> from-the-scratch
> implementation, as you put it. This is (and was) also my stance in other
> similar
> matters.
>
> The devil is in the details, of course.  Is eldoc.el the right
> infrastructure
> for it. What augmentation does it need, if any?  Are these augmentations
> practical given the relative size, complexity and history of eldoc.el?
> Isn't
> an "inlay.el" or a "little-hint.el" support library a better choice? Or
> maybe
> flymake.el with it's use of overlay-based annotations is also acceptable
> here?  I don't (yet) have answers to these questions.
>
> Of course the existence of this prototype by Dimitri is certainly
> No Bad Thing and a good starting point.
>
> João
>

[-- Attachment #2: Type: text/html, Size: 4548 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#61066: [PATCH] Add inlay hint support to eglot
  2023-01-26  6:29 ` Eli Zaretskii
  2023-01-26 12:30   ` João Távora
@ 2023-01-28  1:58   ` Dmitry Gutov
  1 sibling, 0 replies; 5+ messages in thread
From: Dmitry Gutov @ 2023-01-28  1:58 UTC (permalink / raw)
  To: Eli Zaretskii, Dimitri Belopopsky, João Távora; +Cc: 61066

On 26/01/2023 08:29, Eli Zaretskii wrote:
> AFAIU, inlay hints provide information of the same kind as ElDoc and
> in similar manner from the display and UX POV.  So I think this
> feature should work via ElDoc, not as a separate from-the-scratch
> implementation.

IIUC the "inlay hints" are about different information.

Information that is not determined by the position of point.

For example, annotating variable declarations with specific types in a 
language that infers types, here's a screenshot: 
https://user-images.githubusercontent.com/2771466/71774757-54269200-2fc8-11ea-8505-ac2f326761ca.png

Or adding parameter types (or names) inside function calls, example: 
https://user-images.githubusercontent.com/2303841/35807174-43bc0c3c-0a82-11e8-8c99-591fb650e1e0.png

So I don't think this fits Eldoc's protocol.

You were probably thinking of signature help (that was my initial 
impression as well).





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-01-28  1:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 22:34 bug#61066: [PATCH] Add inlay hint support to eglot Dimitri Belopopsky
2023-01-26  6:29 ` Eli Zaretskii
2023-01-26 12:30   ` João Távora
2023-01-27 18:44     ` Dimitri Belopopsky
2023-01-28  1:58   ` Dmitry Gutov

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).