bug#42421: closed (28.0.50; ElDoc requests too much documentation)

[GNU bug Tracking System]

Your message dated Thu, 23 Jul 2020 12:04:02 +0100
with message-id 
<CALDnm51gu0-gKe_P6JRdS1PXy_xLVSt1aTe-ypdAc9k=Dd3p2A@mail.gmail.com>
and subject line Re: bug#42421: 28.0.50; ElDoc requests too much documentation
has caused the debbugs.gnu.org bug report #42421,
regarding 28.0.50; ElDoc requests too much documentation
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
42421: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=42421
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems
> --- Begin Message ---
>
>
>
> Subject: 
>
> 28.0.50; ElDoc requests too much documentation
>
>
>
>
> 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?= <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
>
>
> --- End Message ---

--- Begin Message --- Subject: Re: bug#42421: 28.0.50; ElDoc requests too much documentation Date: Thu, 23 Jul 2020 12:04:02 +0100

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

--- End Message ---