bug#41531: 27.0.91; Better handle asynchronous eldoc backends

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

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

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

>> It is, yes.  The code is clear, not transcendental at all.  But writing
>> docstrings is hard.  In fact, it's possibly the hardest part of the
>> game.  Tell you what: you write the docstring for
>> eldoc-documentation-functions, I'll do the implementation.
>>
>> Deal?
>
> Deal, at the condition that the code comes before I write the docstrings ;-)

Okay. 2 patches attached (my version + your futures).  Just search for
'WORLD CLASS DOCSTRING' and fill in your part.

For my part, really feels like I've reimplemented funcall.

Anyway both approaches lightly tested on this foo.el file:

;;; foo.el --- foobarbaz  -*- lexical-binding:t; -*-
(setq-local eldoc-documentation-function #'eldoc-documentation-compose)

;; Callback approach
;;
(setq-local eldoc-documentation-functions nil)
(defun eldoc-cback-0 (cback &rest _)
  (run-with-timer 0.22 nil
                  (lambda () (funcall cback (symbol-name (gensym "cback-0-"))))))
(defun eldoc-cback-1 (&rest _) (symbol-name (gensym "cback-1-")))
(defun eldoc-cback-2 (cback &rest _)
  (run-with-timer 2.22 nil
                  (lambda () (funcall cback (symbol-name (gensym "cback-2-"))))))

(add-hook 'eldoc-documentation-functions #'eldoc-cback-0 00 t)
(add-hook 'eldoc-documentation-functions #'eldoc-cback-1 10 t)
(add-hook 'eldoc-documentation-functions #'eldoc-cback-2 20 t)

;; Very futuristic approach
;;
(setq-local eldoc-documentation-functions nil)
(defun eldoc-future-0 ()
  (let ((f (eldoc-future-make))) 
    (run-with-timer 0.22 nil (lambda () (eldoc-future-set
                                         f (symbol-name (gensym "future-0-")))))
    f))
(defun eldoc-future-1 () (symbol-name (gensym "future-1-")))
(defun eldoc-future-2 ()
  (let ((f (eldoc-future-make)))
    (run-with-timer 2.22 nil (lambda () (eldoc-future-set
                                         f (symbol-name (gensym "future-2-")))))
    f))

(add-hook 'eldoc-documentation-functions #'eldoc-future-0 00 t)
(add-hook 'eldoc-documentation-functions #'eldoc-future-1 10 t)
(add-hook 'eldoc-documentation-functions #'eldoc-future-2 20 t)

> [ I like this deal: it's always better for someone else to write the
>   docstrings, so at least 2 persons need to agree on what they think the
>   code does (assuming the author of the code checks the resulting docstrings).  ]

Yes, it's called cascade dev, and it gets a bad rep nowadays.  Tho the
person who writes the docstrings usually goes first ;-)

João


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Better-handle-asynchronously-produced-eldoc-docstrin.patch --]
[-- Type: text/x-diff, Size: 11774 bytes --]

From b97c0cfb0cbfea20cbf75ad7ea8abf9f4c51ec54 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 1/2] 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..fa36987014 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)
+                            (when 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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Reimplement-funcall-but-in-the-future.patch --]
[-- Type: text/x-diff, Size: 3905 bytes --]

From c039a4356fa363ef10b8320c3856eb3466eb32f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=A3o=20T=C3=A1vora?= <joaotavora@gmail.com>
Date: Tue, 26 May 2020 19:38:08 +0100
Subject: [PATCH 2/2] Reimplement funcall, but in the future

* lisp/emacs-lisp/eldoc.el (eldoc-future, eldoc-future-set)
(eldoc-future-set-call):  New functions.
(eldoc-documentation-functions): Prepare for Stefan's genius
docstring.
(eldoc-documentation-default, eldoc-documentation-compose): Use
futuristic stuff.
---
 lisp/emacs-lisp/eldoc.el | 54 ++++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index fa36987014..e015076c4d 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -337,18 +337,32 @@ eldoc-display-message-no-interference-p
   (not (or executing-kbd-macro (bound-and-true-p edebug-active))))
 
 \f
+;;;; Futuristic interlude
+(cl-defstruct (eldoc-future
+               (:conc-name eldoc-future--)
+               (:constructor eldoc-future-make ())) ; become Yoda we?
+  "<WORLD CLASS DOCSTRING>"
+  (value 'eldoc-future--unset)
+  callback)
+
+(defun eldoc-future-set (f v)
+  "<WORLD CLASS DOCSTRING>"
+  (cl-assert (eq (eldoc-future--value f) 'eldoc-future--unset))
+  (setf (eldoc-future--value f) v)
+  (when (eldoc-future--callback f)
+    (funcall (eldoc-future--callback f) v)))
+
+(defun eldoc-future-set-callback (f c)
+  "<WORLD CLASS DOCSTRING>"
+  (cl-assert (null (eldoc-future--callback f)))
+  (setf (eldoc-future--callback f) c)
+  (unless (eq (eldoc-future--value f) 'eldoc-future--unset)
+    (funcall c (eldoc-future--value f))))
+
+\f
 (defvar eldoc-documentation-functions nil
   "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.
+<WORLD CLASS DOCSTRING GOES HERE>
 
 Note that this hook is only in effect if the value of
 `eldoc-documentation-function' (notice the singular) is bound to
@@ -372,19 +386,23 @@ eldoc--handle-multiline
 (defun eldoc-documentation-default ()
   "Show first doc string for item at point.
 Default value for `eldoc-documentation-function'."
-  (run-hook-with-args-until-success 'eldoc-documentation-functions
-   eldoc--callback))
+  (let ((x (run-hook-with-args-until-success 'eldoc-documentation-functions)))
+    (if (eldoc-future-p x) (eldoc-future-set-callback x eldoc--callback)
+      x)))
 
 (defun eldoc-documentation-compose ()
   "Show multiple doc string results at once.
 Meant as a value for `eldoc-documentation-function'."
   (let ((res 0))
-    (run-hook-wrapped 'eldoc-documentation-functions
-                      (lambda (f)
-                        (let ((str (funcall f eldoc--callback)))
-                          (if (stringp str) (funcall eldoc--callback str)
-                            (when str (setq res (1+ res))))
-                          nil)))
+    (run-hook-wrapped
+     'eldoc-documentation-functions
+     (lambda (f)
+       (let ((x (funcall f)))
+         (cond ((stringp x) (funcall eldoc--callback x))
+               ((eldoc-future-p x)
+                (eldoc-future-set-callback x eldoc--callback)
+                (setq res (1+ res))))
+         nil)))
     ;; play ball with `eldoc-print-current-symbol-info'
     (if (plusp res) (1- res) "")))
 
-- 
2.20.1