Re: Eglot "inlay hints" landed

[João Távora]

On Thu, Feb 23, 2023 at 10:19 PM Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
>
> > Mostly the fact that it's operating on a separate timer, but one
> > that is directly correlated to jit-lock-context-time.
>
> But that's because you're willing to wait for the context refresh to do
> your own.
>
> > So the bookkeeping and the coalescing of the small+large jit chunks
> > should be provided by the jit infrastructure instead.
>
> So far, you're the first to need such a thing.  In my experience the
> needs for "jit display refresh" can be fairly subtly different, so it's
> not clear how generally useful your approach would be.  Maybe we could
> provide some shared infrastructure to maintain a "coalescing set of
> buffer regions", but if so, I suspect that it wouldn't need to be tied
> to `jit-lock.el`.
>
> Also, I'm not sure it gives exactly the info you need/want:
> I suspect that in some languages you can have:
>
>    foo (x)
>    ...
>    function foo (bar : Int)

Yeah, in that case we're frobbed.  But isn't that a problem already
those "some languages" for regular "contextual" fontification?

> > And then no extra timer or logic would be needed.
>
> You mean you'd integrate it into `jit-lock-context`?
> Maybe that could be done.
> > Can we make jit-lock.el a :core ELPA package?
> I have no opinion on that.

That's the only reasonable way that Eglot can ever be changed
to make use of new jit-lock infrastructure.

Anyway, have a look at this patch:

This implementation is much simpler than the one based on
windows-scroll-functions.  It's also conceptually safer, since
jit-lock is guaranteed to refontify the regions that may need
redisplaying.

It's not trivially simple though, as simply adding
'eglot--update-hints-1' to jit-lock-functions, while possible, is
going to request inlay hints from the LSP server for every small
fraction changed by the user.  So we do some bookkeeping on regions
and coalesce them at a suitable time for a larger request.

To do this, we use a jit-lock implementation detail,
jit-lock-context-unfontify-pos, which tells us that the contextual
fontification has just finished.  Not sure how brittle it is, but it
seems to work reasonably.

* doc/misc/eglot.texi (Eglot Variables): Remove mention
to deleted eglot-lazy-inlay-hints.

* lisp/progmodes/eglot.el (eglot-lazy-inlay-hints)
(eglot--inlay-hints-after-scroll)
(eglot--inlay-hints-fully)
(eglot--inlay-hints-lazily): Remove.
(eglot--update-hints): Add function.
(eglot-inlay-hints-mode): Simplify.
---
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index eea8be6d1aa..9955b04574f 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -3489,32 +3489,38 @@ eglot-type-hint-face
 (defface eglot-parameter-hint-face '((t (:inherit eglot-inlay-hint-face)))
   "Face used for parameter inlay hint overlays.")

-(defcustom eglot-lazy-inlay-hints 0.3
-  "If non-nil, restrict LSP inlay hints to visible portion of the buffer.
-
-Value is a number specifying how many seconds to wait after a
-window has been (re)scrolled before requesting new inlay hints
-for the now-visible portion of the buffer shown in the window.
-
-If nil, then inlay hints are requested for the entire buffer.
-This could be slow.
-
-This value is only meaningful if the minor mode
-`eglot-inlay-hints-mode' is turned on in a buffer."
-  :type 'number
-  :version "29.1")
-
-(defun eglot--inlay-hints-fully ()
-  (eglot--widening (eglot--update-hints-1 (point-min) (point-max))))
-
-(cl-defun eglot--inlay-hints-lazily (&optional (buffer (current-buffer)))
-  (eglot--when-live-buffer buffer
-    (when eglot--managed-mode
-      (dolist (window (get-buffer-window-list nil nil 'visible))
-        (eglot--update-hints-1 (window-start window) (window-end window))))))
+(defvar-local eglot--outstanding-inlay-regions nil
+  "List of regions that weren't yet \"contextually\" fontified.")
+
+(defun eglot--update-hints (from to)
+  "Jit-lock function for Eglot inlay hints."
+  ;; HACK: Comparing `jit-lock-context-unfontify-pos' to `point-max'
+  ;; is our way of telling whether this call to `jit-lock-functions'
+  ;; happens after `jit-lock-context-timer' has just run.  This is the
+  ;; best time to coalesce the regions in
+  ;; `eglot--outstanding-inlay-regions' and contact the LSP server.  Any
+  ;; other call to this function just adds another region to that list.
+  (if (= jit-lock-context-unfontify-pos (point-max))
+      (let* ((sorted (sort (cons (cons from to)
+                                 eglot--outstanding-inlay-regions)
+                           (pcase-lambda (`(,a1 . ,a2) `(,b1 . ,b2))
+                             (if (= a1 b1) (< a2 b2) (< a1 b1)))))
+             (coalesced (cl-reduce (lambda (acc x)
+                                     (cond ((and acc (>= (cdr (car acc))
+                                                         (car x)))
+                                            (setcdr (car acc) (cdr x))
+                                            acc)
+                                           (t (push x acc) acc)))
+                                   sorted
+                                   :initial-value nil)))
+        (dolist (r coalesced)
+          (eglot--update-hints-1 (max (car r) (point-min))
+                                 (min (cdr r) (point-max))))
+        (setq eglot--outstanding-inlay-regions nil))
+    (push (cons from to) eglot--outstanding-inlay-regions)))

 (defun eglot--update-hints-1 (from to)
-  "Request LSP inlay hints and annotate current buffer from FROM to TO."
+  "Do most work for `eglot--update-hints', including LSP request."
   (let* ((buf (current-buffer))
          (paint-hint
           (eglot--lambda ((InlayHint) position kind label paddingLeft
paddingRight)
@@ -3545,67 +3551,16 @@ eglot--update-hints-1
                       (mapc paint-hint hints))))
      :deferred 'eglot--update-hints-1)))

-(defun eglot--inlay-hints-after-scroll (window display-start)
-  (cl-macrolet ((wsetq (sym val) `(set-window-parameter window ',sym ,val))
-                (wgetq (sym) `(window-parameter window ',sym)))
-    (let ((buf (window-buffer window))
-          (timer (wgetq eglot--inlay-hints-timer))
-          (last-display-start (wgetq eglot--last-inlay-hint-display-start)))
-      (when (and eglot-lazy-inlay-hints
-                 ;; FIXME: If `window' is _not_ the selected window,
-                 ;; then for some unknown reason probably related to
-                 ;; the overlays added later to the buffer, the scroll
-                 ;; function will be called indefinitely.  Not sure if
-                 ;; an Emacs bug, but prevent useless duplicate calls
-                 ;; by saving and examining `display-start' fixes it.
-                 (not (eql last-display-start display-start)))
-        (when timer (cancel-timer timer))
-        (wsetq eglot--last-inlay-hint-display-start
-               display-start)
-        (wsetq eglot--inlay-hints-timer
-               (run-at-time
-                eglot-lazy-inlay-hints
-                nil (lambda ()
-                      (eglot--when-live-buffer buf
-                        (when (eq buf (window-buffer window))
-                          (eglot--update-hints-1 (window-start window)
-                                                 (window-end window))
-                          (wsetq eglot--inlay-hints-timer nil))))))))))
-
-(defun eglot--inlay-hints-after-window-config-change ()
-  (eglot--update-hints-1 (window-start) (window-end)))
-
 (define-minor-mode eglot-inlay-hints-mode
   "Minor mode for annotating buffers with LSP server's inlay hints."
   :global nil
   (cond (eglot-inlay-hints-mode
-         (cond
-          ((not (eglot--server-capable :inlayHintProvider))
+         (if (eglot--server-capable :inlayHintProvider)
+             (jit-lock-register #'eglot--update-hints 'contextual)
            (eglot--warn
-            "No :inlayHintProvider support. Inlay hints will not work."))
-          (eglot-lazy-inlay-hints
-           (add-hook 'eglot--document-changed-hook
-                     #'eglot--inlay-hints-lazily t t)
-           (add-hook 'window-scroll-functions
-                     #'eglot--inlay-hints-after-scroll nil t)
-           (add-hook 'window-configuration-change-hook
-                     #'eglot--inlay-hints-after-window-config-change nil t)
-           ;; Maybe there isn't a window yet for current buffer,
-           ;; so `run-at-time' ensures this runs after redisplay.
-           (run-at-time 0 nil #'eglot--inlay-hints-lazily))
-          (t
-           (add-hook 'eglot--document-changed-hook
-                     #'eglot--inlay-hints-fully nil t)
-           (eglot--inlay-hints-fully))))
+            "No :inlayHintProvider support. Inlay hints will not work.")))
         (t
-         (remove-hook 'window-configuration-change-hook
-                      #'eglot--inlay-hints-after-window-config-change)
-         (remove-hook 'eglot--document-changed-hook
-                      #'eglot--inlay-hints-lazily t)
-         (remove-hook 'eglot--document-changed-hook
-                      #'eglot--inlay-hints-fully t)
-         (remove-hook 'window-scroll-functions
-                      #'eglot--inlay-hints-after-scroll t)
+         (jit-lock-unregister #'eglot--update-hints)
          (remove-overlays nil nil 'eglot--inlay-hint t))))