Subject: 28.0.50; ElDoc requests too much documentation

From: João Távora <[email-masked]>
Date: Sun, 19 Jul 2020 01:02:07 +0100

Hello, I'm following up on a bug started in
https://github.com/joaotavora/eglot/issues/445.

In the latest ElDoc, when the *eldoc* buffer is showing a lenghty
docstrings for some situation in a source buffer, switching to that
buffer scrolling around, then switching back to the same position in the
source buffer will lead to that very documentation being re-requested,
ultimately undoing the scrolling work done previously by the user.

The attached patch prevents needless, superfluous doc requests when the
requester's state is found to be identical to the state recorded in a
visible *eldoc* buffer.

These kinds of situation must be taken into account when redesigning the
mechanism for allowing multiple outlets for documentation.

For now, the patch should fix the situation.  Felicián, please try it
out.  Notice that it probably needs the Emacs master version of eldoc.el
(where, BTW, I have already fixed the other bug you report in the Github
issue).

João

>From 526dcda35ca2b61ffda7005fb0fd62ae2f80bc06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <[email-masked]>
Date: Sun, 19 Jul 2020 00:48:43 +0100
Subject: [PATCH] Don't needlessly request docs from ElDoc functions

Do this conservatively for now: if the ElDoc doc buffer is visible and
showing documentation for the very same situation the source buffer is
still in, don't request it again.

* lisp/emacs-lisp/eldoc.el (eldoc--request-docs-p): Rework from
eglot-display-message-p.
(eldoc--last-request-state): New variable.
(eldoc--request-state): New helper.
(eldoc--handle-docs): Memorize state of request in doc buffer.
(eldoc-print-current-symbol-info): Pass a token to
eldoc--request-docs-p.
---
 lisp/emacs-lisp/eldoc.el | 59 ++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 18 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 6ed5bff9f4..20b796c529 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -340,16 +340,32 @@ eldoc-pre-command-refresh-echo-area
          ;; for us, but do note that the last-message will be gone.
          (setq eldoc-last-message nil))))
 
-;; Decide whether now is a good time to display a message.
-(defun eldoc-display-message-p ()
-  "Return non-nil when it is appropriate to display an ElDoc message."
-  (and (eldoc-display-message-no-interference-p)
-       ;; If this-command is non-nil while running via an idle
-       ;; timer, we're still in the middle of executing a command,
-       ;; e.g. a query-replace where it would be annoying to
-       ;; overwrite the echo area.
-       (not this-command)
-       (eldoc--message-command-p last-command)))
+(defvar-local eldoc--last-request-state nil
+  "Tuple containing information about last ElDoc request.")
+(defun eldoc--request-state ()
+  "Compute information to store in `eldoc--last-request-state'."
+  (list (current-buffer) (buffer-modified-tick) (point)))
+
+(defun eldoc--request-docs-p (request-state)
+  "Return non-nil when it is appropriate to request docs.
+REQUEST-STATE is a candidate for `eldoc--last-request-state'"
+  (and
+   ;; FIXME: The original idea behind this function is to protect the
+   ;; Echo area from ElDoc interference, but since that is only one of
+   ;; the possible outlets of ElDoc, this must soon be reworked.
+   (eldoc-display-message-no-interference-p)
+   (not (and eldoc--doc-buffer
+             (get-buffer-window eldoc--doc-buffer)
+             (equal request-state
+                    (with-current-buffer
+                        eldoc--doc-buffer
+                      eldoc--last-request-state))))
+   ;; If this-command is non-nil while running via an idle
+   ;; timer, we're still in the middle of executing a command,
+   ;; e.g. a query-replace where it would be annoying to
+   ;; overwrite the echo area.
+   (not this-command)
+   (eldoc--message-command-p last-command)))

 ;; Check various conditions about the current environment that might make
@@ -400,7 +416,8 @@ eldoc-documentation-functions
 taken into account if the major mode specific function does not
 return any documentation.")
 
-(defvar eldoc--doc-buffer nil "Buffer holding latest eldoc-produced docs.")
+(defvar eldoc--doc-buffer nil "Buffer displaying latest ElDoc-produced docs.")
+
 (defun eldoc-doc-buffer (&optional interactive)
   "Get latest *eldoc* help buffer.  Interactively, display it."
   (interactive (list t))
@@ -410,6 +427,7 @@ eldoc-doc-buffer
           (setq eldoc--doc-buffer (get-buffer-create "*eldoc*")))
     (when interactive (display-buffer eldoc--doc-buffer))))
 
+
 (defun eldoc--handle-docs (docs)
   "Display multiple DOCS in echo area.
 DOCS is a list of (STRING PLIST...).  It is already sorted.
@@ -429,9 +447,12 @@ eldoc--handle-docs
                       (integer val)
                       (t 1)))
          (things-reported-on)
+         (request eldoc--last-request-state)
          single-doc single-doc-sym)
       ;; Then, compose the contents of the `*eldoc*' buffer.
       (with-current-buffer (eldoc-doc-buffer)
+        ;; Set doc-buffer's `eldoc--last-request-state', too
+        (setq eldoc--last-request-state request)
         (let ((inhibit-read-only t))
           (erase-buffer) (setq buffer-read-only t)
           (local-set-key "q" 'quit-window)
@@ -741,14 +762,16 @@ eldoc--invoke-strategy
 (defun eldoc-print-current-symbol-info (&optional interactive)
   "Document thing at point."
   (interactive '(t))
-  (cond (interactive
-         (eldoc--invoke-strategy))
-        (t
-         (if (not (eldoc-display-message-p))
-             ;; Erase the last message if we won't display a new one.
-             (when eldoc-last-message
-               (eldoc--message nil))
+  (let ((token (eldoc--request-state)))
+    (cond (interactive
+           (eldoc--invoke-strategy))
+          ((not (eldoc--request-docs-p token))
+           ;; Erase the last message if we won't display a new one.
+           (when eldoc-last-message
+             (eldoc--message nil)))
+          (t
            (let ((non-essential t))
+             (setq eldoc--last-request-state token)
              ;; Only keep looking for the info as long as the user hasn't
              ;; requested our attention.  This also locally disables
              ;; inhibit-quit.
-- 
2.25.1