bug#42421: 28.0.50; ElDoc requests too much documentation

bug#42421: 28.0.50; ElDoc requests too much documentation

[João Távora]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Don-t-needlessly-request-docs-from-ElDoc-functions.patch --]
[-- Type: text/x-diff, Size: 5364 bytes --]

From 526dcda35ca2b61ffda7005fb0fd62ae2f80bc06 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
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

bug#42421: 28.0.50; ElDoc requests too much documentation

[João Távora]

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

I've pushed the patch that fixes this.  The commit message hints at the
steps needed to prevent a resurgence of this bug when more developments
in eldoc.el are performed:

Author: João Távora <joaotavora@gmail.com>
Date:   Sun Jul 19 00:48:43 2020 +0100

    Don't needlessly request docs from ElDoc functions

    Fixes: bug#42421

    Do this conservatively for now: if the ElDoc helper buffer (as
    returned by eldoc--doc-buffer) is visible and showing documentation
    for the very same "situation" (as computed by the the new
    eldoc--request-state helper), don't request that documentation from
    sources again.

    Before this change, not only was that request inefficient but if the
    user invoked scroll-other-window to see more of the helper buffer,
    that would eventually cause it to be reformatted and unexpectedly
    recentered.

    Later on, when a customizable list of documentation "sinks" is offered
    to the user, say, something like eldoc-display-functions, this process
    must be consolidated.  In those circumstances, as soon as one of those
    sinks signals that it doesn't have up-to-date documentation for the
    state computed by eldoc--request-state, documentation will have to be
    requested anew from eldoc-documentation-functions via
    eldoc--invoke-strategy.

    * 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.
    (Version): Bump to 1.6.0

On Sun, Jul 19, 2020 at 1:03 AM João Távora <joaotavora@gmail.com> wrote:

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

-- 
João Távora

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

bug#42421: 28.0.50; ElDoc requests too much documentation

[Felician Nemeth]

> Felicián, please try it out.

I don't see this issue with the latest master.  Thank you,
Felicián