bug#41531: 27.0.91; Better handle asynchronous eldoc backends

["João Távora" <joaotavora@gmail.com>]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> But really: now we have deadlock too?  I just want to solve this
>> problem: please let's commit something, and move on to the next bug.
>
> Can you use the sample `eldoc-future-*` code I sent earlier?
>
>         Stefan

It's not complete, is it?  And how to I use it to solve the
eldoc-documentation-compose problem?  I suppose it's possible, anything
is.  But do we really want to hand-roll futures in eldoc.el when we got
this nice https://github.com/skeeto/emacs-aio/commits/master that we
could be looking into?

Also the latest patch I attach solves the eldoc-documentation-compose
problem decently (should actually be less LOC than previous versions).
I suggest we go with this tried-and-tested well-understood solution and
then adjust as more sophisticated solutions come along and we evaluate
their merits.

João


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-eldoc-async-v3.patch --]
[-- Type: text/x-diff, Size: 11759 bytes --]

From 0227a048cc7f88c5e4d773eac2ec8366056f3a6b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Mon, 25 May 2020 16:39:40 +0100
Subject: [PATCH] Better handle asynchronously produced eldoc docstrings

No longer do clients of eldoc need to call eldoc-message (an internal
function) directly.  They may return any non-nil, non-string value and
call a callback afterwards.  This enables eldoc.el to exert control
over how (and crucially also when) to display the docstrings to the
user.

* lisp/emacs-lisp/eldoc.el (eldoc-documentation-functions):
Overhaul docstring.
(eldoc-documentation-compose, eldoc-documentation-default): Handle
non-nil, non-string values of elements of
eldoc-documentation-functions.
(eldoc-print-current-symbol-info): Redesign.
(eldoc--handle-multiline): New helper.
(eldoc--callback): New internal special var.
(Version): Bump to 1.1.0.

* lisp/hexl.el (hexl-print-current-point-info): Adjust to new
eldoc-documentation-functions protocol.

* lisp/progmodes/cfengine.el (cfengine3-documentation-function):
Adjust to new eldoc-documentation-functions protocol.

* lisp/progmodes/elisp-mode.el
(elisp-eldoc-documentation-function): Adjust to new
eldoc-documentation-functions protocol.

* lisp/progmodes/octave.el (octave-eldoc-function): Adjust to new
eldoc-documentation-functions protocol.

* lisp/progmodes/python.el (python-eldoc-function): Adjust to new
eldoc-documentation-functions protocol.
---
 lisp/emacs-lisp/eldoc.el     | 101 ++++++++++++++++++++++++++---------
 lisp/hexl.el                 |   2 +-
 lisp/progmodes/cfengine.el   |   2 +-
 lisp/progmodes/elisp-mode.el |   6 ++-
 lisp/progmodes/octave.el     |   4 +-
 lisp/progmodes/python.el     |   2 +-
 6 files changed, 84 insertions(+), 33 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index ef5dbf8103..dcd28fb082 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -5,7 +5,7 @@
 ;; Author: Noah Friedman <friedman@splode.com>
 ;; Keywords: extensions
 ;; Created: 1995-10-06
-;; Version: 1.0.0
+;; Version: 1.1.0
 ;; Package-Requires: ((emacs "26.3"))
 
 ;; This is a GNU ELPA :core package.  Avoid functionality that is not
@@ -338,12 +338,24 @@ eldoc-display-message-no-interference-p
 
 \f
 (defvar eldoc-documentation-functions nil
-  "Hook for functions to call to return doc string.
-Each function should accept no arguments and return a one-line
-string for displaying doc about a function etc. appropriate to
-the context around point.  It should return nil if there's no doc
-appropriate for the context.  Typically doc is returned if point
-is on a function-like name or in its arg list.
+  "Hook of functions that produce doc strings.
+Each hook function should accept at least one argument CALLBACK
+and decide whether to display a doc short string about the
+context around point.  If the decision and the doc string can be
+produced quickly, the hook function can ignore CALLBACK and
+immediately return the doc string, or nil if there's no doc
+appropriate for the context.  Otherwise, if its computation is
+expensive or can't be performed directly, the hook function
+should arrange for CALLBACK to be asynchronously called at a
+later time, passing it either nil or the desired doc string.  The
+hook function should then return a non-nil, non-string value.
+
+Note that this hook is only in effect if the value of
+`eldoc-documentation-function' (notice the singular) is bound to
+one of its pre-set values.
+
+Typically doc is returned if point is on a function-like name or
+in its arg list.
 
 Major modes should modify this hook locally, for example:
   (add-hook \\='eldoc-documentation-functions #\\='foo-mode-eldoc nil t)
@@ -351,30 +363,30 @@ eldoc-documentation-functions
 taken into account if the major mode specific function does not
 return any documentation.")
 
+(defun eldoc--handle-multiline (res)
+  "Helper for handling a bit of `eldoc-echo-area-use-multiline-p'."
+  (if eldoc-echo-area-use-multiline-p res
+    (truncate-string-to-width
+     res (1- (window-width (minibuffer-window))))))
+
 (defun eldoc-documentation-default ()
   "Show first doc string for item at point.
 Default value for `eldoc-documentation-function'."
-  (let ((res (run-hook-with-args-until-success 'eldoc-documentation-functions)))
-    (when res
-      (if eldoc-echo-area-use-multiline-p res
-        (truncate-string-to-width
-         res (1- (window-width (minibuffer-window))))))))
+  (run-hook-with-args-until-success 'eldoc-documentation-functions
+   eldoc--callback))
 
 (defun eldoc-documentation-compose ()
   "Show multiple doc string results at once.
 Meant as a value for `eldoc-documentation-function'."
-  (let (res)
-    (run-hook-wrapped
-     'eldoc-documentation-functions
-     (lambda (f)
-       (let ((str (funcall f)))
-         (when str (push str res))
-         nil)))
-    (when res
-      (setq res (mapconcat #'identity (nreverse res) ", "))
-      (if eldoc-echo-area-use-multiline-p res
-        (truncate-string-to-width
-         res (1- (window-width (minibuffer-window))))))))
+  (let ((res 0))
+    (run-hook-wrapped 'eldoc-documentation-functions
+                      (lambda (f)
+                        (let ((str (funcall f eldoc--callback)))
+                          (if (stringp str) (funcall eldoc--callback str)
+                            (setq res (1+ res)))
+                          nil)))
+    ;; play ball with `eldoc-print-current-symbol-info'
+    (if (plusp res) (1- res) "")))
 
 (defcustom eldoc-documentation-function #'eldoc-documentation-default
   "Function to call to return doc string.
@@ -408,6 +420,11 @@ eldoc--supported-p
            ;; there's some eldoc support in the current buffer.
            (local-variable-p 'eldoc-documentation-function))))
 
+;; this variable should be unbound, but that confuses
+;; `describe-symbol' for some reason.
+(defvar eldoc--callback nil
+  "Dynamically bound.  Passed to  `eldoc-documentation-functions'.")
+
 (defun eldoc-print-current-symbol-info ()
   "Print the text produced by `eldoc-documentation-function'."
   ;; This is run from post-command-hook or some idle timer thing,
@@ -417,11 +434,43 @@ eldoc-print-current-symbol-info
         ;; Erase the last message if we won't display a new one.
         (when eldoc-last-message
           (eldoc-message nil))
-      (let ((non-essential t))
+      (let ((non-essential t)
+            (buffer (current-buffer)))
         ;; Only keep looking for the info as long as the user hasn't
         ;; requested our attention.  This also locally disables inhibit-quit.
         (while-no-input
-          (eldoc-message (funcall eldoc-documentation-function)))))))
+          (let* (;; `wanted' and `received' keep track of how many
+                 ;; docstrings we expect from the clients.  negative
+                 ;; `wanted' means store docstring for later but don't
+                 ;; message yet; likewise for a positive value, but we
+                 ;; decrease it by one.  Any other value (including 0)
+                 ;; means the next time the callback is called we're
+                 ;; composing and outputting whatever we got.
+                 (wanted -1) (received '())
+                 (eldoc--callback
+                  (lambda (string)
+                    (with-current-buffer buffer
+                      (cond ((and (numberp wanted) (not (zerop wanted)))
+                             (if (plusp wanted)
+                                 (setq wanted (1- wanted))) ; decf where art thou?
+                             (push string received))
+                            (wanted
+                             (unless (string= string "") (push string received))
+                             (setq wanted nil)
+                             (eldoc-message
+                              (eldoc--handle-multiline
+                               (mapconcat #'identity (nreverse received) ", "))))
+                            (t
+                             ;; For now, silently swallow anything the
+                             ;; client unexpectedly gives us
+                             )))))
+                 (res (funcall eldoc-documentation-function)))
+            (cond (;; we got a string, we should output immediately
+                   (stringp res) (setq wanted t) (funcall eldoc--callback res))
+                  (;; got something else, trust eldoc--callback will be called
+                   res           (setq wanted res))
+                  (;; got nil, clear the echo area
+                   t             (eldoc-message nil)))))))))
 
 ;; If the entire line cannot fit in the echo area, the symbol name may be
 ;; truncated or eliminated entirely from the output to make room for the
diff --git a/lisp/hexl.el b/lisp/hexl.el
index cf7118f208..38eca77e26 100644
--- a/lisp/hexl.el
+++ b/lisp/hexl.el
@@ -515,7 +515,7 @@ hexl-current-address
       (message "Current address is %d/0x%08x" hexl-address hexl-address))
     hexl-address))
 
-(defun hexl-print-current-point-info ()
+(defun hexl-print-current-point-info (&rest _ignored)
   "Return current hexl-address in string.
 This function is intended to be used as eldoc callback."
   (let ((addr (hexl-current-address)))
diff --git a/lisp/progmodes/cfengine.el b/lisp/progmodes/cfengine.el
index f25b3cb9e2..9a6d81ce06 100644
--- a/lisp/progmodes/cfengine.el
+++ b/lisp/progmodes/cfengine.el
@@ -1294,7 +1294,7 @@ cfengine3-make-syntax-cache
                           'symbols))
         syntax)))
 
-(defun cfengine3-documentation-function ()
+(defun cfengine3-documentation-function (&rest _ignored)
   "Document CFengine 3 functions around point.
 Intended as the value of `eldoc-documentation-function', which see.
 Use it by enabling `eldoc-mode'."
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index d37eb8c152..d7865a7319 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -1402,8 +1402,10 @@ elisp--eldoc-last-data
       or argument string for functions.
   2 - `function' if function args, `variable' if variable documentation.")
 
-(defun elisp-eldoc-documentation-function ()
-  "`eldoc-documentation-function' (which see) for Emacs Lisp."
+(defun elisp-eldoc-documentation-function (_ignored &rest _also-ignored)
+  "Contextual documentation function for Emacs Lisp.
+Intended to be placed in `eldoc-documentation-functions' (which
+see)."
   (let ((current-symbol (elisp--current-symbol))
 	(current-fnsym  (elisp--fnsym-in-current-sexp)))
     (cond ((null current-fnsym)
diff --git a/lisp/progmodes/octave.el b/lisp/progmodes/octave.el
index 352c1810d1..2cf305c404 100644
--- a/lisp/progmodes/octave.el
+++ b/lisp/progmodes/octave.el
@@ -1639,8 +1639,8 @@ octave-eldoc-function-signatures
                   (nreverse result)))))
   (cdr octave-eldoc-cache))
 
-(defun octave-eldoc-function ()
-  "A function for `eldoc-documentation-function' (which see)."
+(defun octave-eldoc-function (&rest _ignored)
+  "A function for `eldoc-documentation-functions' (which see)."
   (when (inferior-octave-process-live-p)
     (let* ((ppss (syntax-ppss))
            (paren-pos (cadr ppss))
diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
index 1ca9f01963..404a67ba9f 100644
--- a/lisp/progmodes/python.el
+++ b/lisp/progmodes/python.el
@@ -4571,7 +4571,7 @@ python-eldoc-function-timeout-permanent
   :type 'boolean
   :version "25.1")
 
-(defun python-eldoc-function ()
+(defun python-eldoc-function (&rest _ignored)
   "`eldoc-documentation-function' for Python.
 For this to work as best as possible you should call
 `python-shell-send-buffer' from time to time so context in
-- 
2.20.1