bug#41531: 27.0.91; Better handle asynchronous eldoc backends

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

Hi Stefan, Dmitry, Andrii and maintainers,

Moving the discussion that started in
https://github.com/joaotavora/eglot/pull/459 to the bug tracker, and
attaching the two patches that contain what I think is a decent
short-term solution to the eldoc/async problems.

It makes eldoc-diagnostic-functions have a very similar interface to
flymake-diagnostic-functions.  Flymake's handling of the multiple
backends and async is more sophisticated, and we could extend eldoc to
try similarly heroic stuff, if we do find there's a demand for it.

The main thing you probably want to read if
`eldoc-documentation-functions`'s new docstring.

This was tested summarily with Eglot, by the way, and seems to work OK.

Enjoy!
João

PS: How do I mark that the bug report contains a patch in the mail
message itself?


[-- 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: 6825 bytes --]

From 9ca84e5482b2e7ef40f80679ec508afb008293ae 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.  Use eldoc--handle-multiline.
(eldoc-print-current-symbol-info): Honour non-nil, non-string
values returned by eldoc-documentation-callback.
(eldoc--handle-multiline): New helper.
(Version): Bump to 1.1.0.
---
 lisp/emacs-lisp/eldoc.el | 73 ++++++++++++++++++++++++++++++----------
 1 file changed, 56 insertions(+), 17 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index ef5dbf8103..f5dcdb4ea0 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,26 @@ 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 no arguments 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 should 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 save the value bound to
+`eldoc-documentation-callback', and arrange for that callback
+function 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.
+
+A current limitation of the asynchronous case is that it is only
+guaranteed to work correctly if the value of
+`eldoc-documentation-function' (notice the singular) is
+`eldoc-documentation-default'.
+
+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,14 +365,18 @@ 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))))))))
+    (cond ((stringp res) (eldoc--handle-multiline res))
+          (t res))))
 
 (defun eldoc-documentation-compose ()
   "Show multiple doc string results at once.
@@ -368,13 +386,11 @@ eldoc-documentation-compose
      'eldoc-documentation-functions
      (lambda (f)
        (let ((str (funcall f)))
-         (when str (push str res))
+         (when (stringp 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))))))))
+      (eldoc--handle-multiline res))))
 
 (defcustom eldoc-documentation-function #'eldoc-documentation-default
   "Function to call to return doc string.
@@ -408,6 +424,12 @@ 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-documentation-callback nil
+  "Dynamically bound.  Accessible to `eldoc-documentation-functions'.
+See that function for details.")
+
 (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 +439,28 @@ 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*
+              ((waiting-for-callback nil)
+               (eldoc-documentation-callback
+                (lambda (string)
+                  (with-current-buffer buffer
+                    ;; JT@2020-05-25: Currently, we expect one single
+                    ;; docstring from the client, we silently swallow
+                    ;; anything the client unexpectedly gives us,
+                    ;; including updates.  This could change.
+                    (when waiting-for-callback
+                      (eldoc-message (eldoc--handle-multiline string))
+                      (setq waiting-for-callback nil)))))
+               (res
+                (funcall eldoc-documentation-function)))
+            (cond ((stringp res) (eldoc-message res))
+                  (res (setq waiting-for-callback t))
+                  (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
-- 
2.20.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Adjust-eldoc-documentation-functions-protocol-for-be.patch --]
[-- Type: text/x-diff, Size: 8347 bytes --]

From a6ba9972ee0e3305c7b41fd380a88dd18a6626a1 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 17:38:23 +0100
Subject: [PATCH 2/2] Adjust eldoc-documentation-functions protocol for better
 async

Instead of exposing a eldoc-documentation-callback variable to
clients, just pass it the callback as the first argument.

* lisp/emacs-lisp/eldoc.el (eldoc-documentation-functions): Rewrite docstring.
(eldoc-documentation-default, eldoc-documentation-compose): Use
internal eldoc--callback.
(eldoc-documentation-callback).
(eldoc---callback): Rename from eldoc-documentation-callback.
(eldoc-print-current-symbol-info): Bind eldoc--callback.

* 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     | 45 ++++++++++++++++++------------------
 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, 32 insertions(+), 29 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index f5dcdb4ea0..a4e1e460ac 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -339,22 +339,22 @@ eldoc-display-message-no-interference-p
 \f
 (defvar eldoc-documentation-functions nil
   "Hook of functions that produce doc strings.
-Each hook function should accept no arguments 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 should 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 save the value bound to
-`eldoc-documentation-callback', and arrange for that callback
-function 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.
-
-A current limitation of the asynchronous case is that it is only
-guaranteed to work correctly if the value of
-`eldoc-documentation-function' (notice the singular) is
-`eldoc-documentation-default'.
+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.  Furthermore, the asynchronous
+mechanism described above is only guaranteed to work correctly if
+that value is `eldoc-documentation-default'.
 
 Typically doc is returned if point is on a function-like name or
 in its arg list.
@@ -374,7 +374,9 @@ eldoc--handle-multiline
 (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)))
+  (let ((res (run-hook-with-args-until-success
+              'eldoc-documentation-functions
+              eldoc--callback)))
     (cond ((stringp res) (eldoc--handle-multiline res))
           (t res))))
 
@@ -385,7 +387,7 @@ eldoc-documentation-compose
     (run-hook-wrapped
      'eldoc-documentation-functions
      (lambda (f)
-       (let ((str (funcall f)))
+       (let ((str (funcall f eldoc--callback)))
          (when (stringp str) (push str res))
          nil)))
     (when res
@@ -426,9 +428,8 @@ eldoc--supported-p
 
 ;; this variable should be unbound, but that confuses
 ;; `describe-symbol' for some reason.
-(defvar eldoc-documentation-callback nil
-  "Dynamically bound.  Accessible to `eldoc-documentation-functions'.
-See that function for details.")
+(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'."
@@ -446,7 +447,7 @@ eldoc-print-current-symbol-info
         (while-no-input
           (let*
               ((waiting-for-callback nil)
-               (eldoc-documentation-callback
+               (eldoc--callback
                 (lambda (string)
                   (with-current-buffer buffer
                     ;; JT@2020-05-25: Currently, we expect one single
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

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

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

On 25.05.2020 20:04, João Távora wrote:
> Hi Stefan, Dmitry, Andrii and maintainers,
> 
> Moving the discussion that started in
> https://github.com/joaotavora/eglot/pull/459  to the bug tracker, and
> attaching the two patches that contain what I think is a decent
> short-term solution to the eldoc/async problems.

Here's a modified approach that doesn't use a global var and should make 
it easier to transition to using futures.

Patch attached. Example of usage:

(add-hook 'eldoc-documentation-functions
           #'test-eldoc-async 0 t)

(defun test-eldoc-async ()
   (cons :async
         (lambda (cb)
           (funcall cb "doc here!"))))

If you like, we could simplify the returned objects to be just FETCHER 
(as documented in the patch) rather than (:async . FETCHER). But the 
latter seems more explicit.

There also exist a possible modification of this patch with a bit of 
functional programming where both calls to eldoc--handle-multiline 
happen from inside of eldoc-documentation-default's definition.

[-- Attachment #2: eldoc-async-with-lexical-callback.diff --]
[-- Type: text/x-patch, Size: 5129 bytes --]

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index ef5dbf8103..8909e8d431 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,26 @@ 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 no arguments and decide whether
+to display a doc short string about the context around point.
+
+Typically doc is returned if point is on a function-like name or
+in its arg list.
+
+If the decision and the doc string can be produced quickly, the
+hook function should 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
+function can instead return a cons (:async . FETCHER) where where
+FETCHER is a function of one argument, CALLBACK.  When the result
+arrives, FETCHER must call CALLBACK and pass it the doc string to
+be displayed.
+
+A current limitation of the asynchronous case is that it is only
+guaranteed to work correctly if the value of
+`eldoc-documentation-function' (notice the singular) is
+`eldoc-documentation-default'.
 
 Major modes should modify this hook locally, for example:
   (add-hook \\='eldoc-documentation-functions #\\='foo-mode-eldoc nil t)
@@ -351,14 +365,18 @@ 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))))))))
+    (cond ((stringp res) (eldoc--handle-multiline res))
+          (t res))))
 
 (defun eldoc-documentation-compose ()
   "Show multiple doc string results at once.
@@ -368,13 +386,11 @@ eldoc-documentation-compose
      'eldoc-documentation-functions
      (lambda (f)
        (let ((str (funcall f)))
-         (when str (push str res))
+         (when (stringp 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))))))))
+      (eldoc--handle-multiline res))))
 
 (defcustom eldoc-documentation-function #'eldoc-documentation-default
   "Function to call to return doc string.
@@ -417,11 +433,30 @@ 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*
+              ((waiting-for-callback t)
+               (callback
+                (lambda (string)
+                  (with-current-buffer buffer
+                    ;; JT@2020-05-25: Currently, we expect one single
+                    ;; docstring from the client, we silently swallow
+                    ;; anything the client unexpectedly gives us,
+                    ;; including updates.  This could change.
+                    (when waiting-for-callback
+                      (setq waiting-for-callback nil)
+                      (eldoc-message (eldoc--handle-multiline string))))))
+               (res
+                (funcall eldoc-documentation-function)))
+            (cond ((stringp res) (eldoc-message res))
+                  ((and (consp res)
+                        (eq (car res) :async))
+                   (funcall (cdr res) callback))
+                  (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

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 25.05.2020 20:04, João Távora wrote:
> (add-hook 'eldoc-documentation-functions
>           #'test-eldoc-async 0 t)
>
> (defun test-eldoc-async ()
>   (cons :async
>         (lambda (cb)
>           (funcall cb "doc here!"))))

Thanks.  As I told you, it's not bad.  These aren't exactly "futures"
though, they're a way to simulate argument passing.  I prefer my
version, which matches what flymake.el, url.el, jsonrpc.el, sly.el and
others do.

> If you like, we could simplify the returned objects to be just FETCHER
> (as documented in the patch) rather than (:async . FETCHER). But the 
> latter seems more explicit.

Yes, if we do go with your version, I'd like that.  The latter is hard
to read and understand from the docstring.

Before I go any further I'd like Stefan and Andrey (or others) to weigh
in.  I don't have a lot of time to invest here, so if there is another
vote for your proposal, I'm not going to wrestle here.

To simplify, hopefully you agree that your proposal can be summarized
as:

  "return a function that receives a function that you
   should call with the docstring"

whereas my proposal can be summarized as:

  "receive a function that you should call with the docstring"

> There also exist a possible modification of this patch with a bit of
> functional programming where both calls to eldoc--handle-multiline 
> happen from inside of eldoc-documentation-default's definition.

Yes, that's independent of the shape of the callback we want to use.
I'll leave that for later.  Specifically, eldoc--handle-multiline has to
do quite a bit more handling (to satisfy Andrey's expectations).

Replying to parts from the other discussion in the Github tracker now.

> OK, another question: if the result still /valid/?
                        ^^ Assuming you mean "is".

Well, if the client discovers the result to be invalid, it can not call
the callback, or signal an error.  If it is valid, call the callback.

> No idea, a hypothetical one. It's an open API, not everybody is or
> will be using LSP until the end of time. And it doesn't really have to
> be huge. A saving is a saving.

There's no free lunch.  A very small saving in time for more complexity
is bad.  That's what overengineered means.

> You can certainly kill the external process outside of it. Saving on
> CPU expenses in general.

The future's creditor is the only one who could do that to any useful
effect.  Does it have access to the process?  Probably not.  You would
have to return a complex future with a kill switch.  That's possible,
but tricky, because you'd then have to be ready in the sentinel for
another source of unexpected kills.

> > For a poor man's async primitive, I prefer my version

> So even the code savings didn't convince you? Both in eldoc.el,

I do see minimal code savings in eldoc.  You do remove a special
variable (which is _not_ the same as a global variable, btw).

I do see a much more complicated docstring, where the reader has to wrap
his head around a 2-deep functional conundrum, whereas my version was
1-deep.  Nothing special, but a VERY common source of headaches.

Let's take your trivial example:

    (defun test-eldoc-async ()
       (cons :async
            (lambda (cb)
               (funcall cb "doc here!"))))

It isn't really representative of what a function that needs async would
have to do, is it?  Because, if you really wanted this very example,
then it's much better to do the one-liner:

   (defun test-eldoc-async () "doc here!")

Rather, presumably you would use this to fetch something from an HTTP
server or so:

    (defun test-eldoc-async ()
      (cons :async
        (lambda (cb)
           (url-retrieve-thingy "http://test-signature" cb))))

Where url-retrieve-thingy is very similar to our url-retrieve. Right?
But why have that awkward :async there when a function is a first class
object that we can identify with the functionp predicate?  Let's just:

    (defun test-eldoc-async ()
       (lambda (cb)
          (url-retrieve-thingy "http://test-signature" cb)))

And at this point one wonders why the heck not

  (defun test-eldoc-async (cb)
     (url-retrieve-thingy "http://test-signature" cb))

?

> and likely in doc functions as well

No.  Unless I am completely mistaken (I might be), in the "doc function"
not only are there no savings, but complication.  This makes sense
because you just inverted the responsibility: the doc function now has
to "beg" for the argument that used to be given to it naturally.

So, it's just a functional gimmick.  As good as the next one, but a
gimmick all the same.  Until the "futures" are here, people will
potentially bang heads in an anguished "WHY??".

Why indeed?  Your other argument, that this makes the transition to
proper futures (such as the ones in https://github.com/skeeto/emacs-aio)
easier, is questionable, too.  There are two scenarios here:

- You want to keep backward compatibility to this API published in eldoc
  1.1.0 until the time of the Emacs 28 release:

  This is something that I -- and Stefan, if I'm not mistaken, -- don't
  think we should worry about.  Just because a package is :core GNU ELPA
  doesn't necessarily mean we guarantee stability of its API.

  But if we do, then we'll have to explain in the docstring that there
  is a fourth return value for the hook functions.  In my version we'll
  have to do exactly the same.

- You don't want to keep backward compatibility until Emacs 28:

  Then, when the super-futures are here, you can just kill the CALLBACK
  arg if we find it doesn't fit and rewrite the docstring without
  concerns.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

> Here's a modified approach that doesn't use a global var and should make it
> easier to transition to using futures.

I think at this stage, we should either start using futures or we should
forget about ever using them.


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

>> Here's a modified approach that doesn't use a global var and should make it
>> easier to transition to using futures.
> I think at this stage, we should either start using futures or we should
> forget about ever using them.

<DRAMATIC MUSIC PLAYS>

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.  It
can be functions that return functions that receive functions with the
airplane.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 26.05.2020 05:38, Stefan Monnier wrote:
>> Here's a modified approach that doesn't use a global var and should make it
>> easier to transition to using futures.
> 
> I think at this stage, we should either start using futures or we should
> forget about ever using them.

We should certainly do it before Emacs 28 is released (so we won't have 
to support whatever ad-hoc option is currently being proposed).

I just don't have the time to work on futures right now. If this bug can 
wait until this weekend, at least, we can reconvene after.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 26.05.2020 04:21, João Távora wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 25.05.2020 20:04, João Távora wrote:
>> (add-hook 'eldoc-documentation-functions
>>            #'test-eldoc-async 0 t)
>>
>> (defun test-eldoc-async ()
>>    (cons :async
>>          (lambda (cb)
>>            (funcall cb "doc here!"))))
> 
> Thanks.  As I told you, it's not bad.  These aren't exactly "futures"
> though, they're a way to simulate argument passing.

It's a simple stand-in, easy to replace.

> I prefer my
> version, which matches what flymake.el, url.el, jsonrpc.el, sly.el and
> others do.

They also use *special* variables? Flymake's API looks fairly clean (you 
call back to REPORTER-FN, that's very reasonable), but I haven't looked 
under the covers yet.

>> If you like, we could simplify the returned objects to be just FETCHER
>> (as documented in the patch) rather than (:async . FETCHER). But the
>> latter seems more explicit.
> 
> Yes, if we do go with your version, I'd like that.  The latter is hard
> to read and understand from the docstring.

I think it's more explicit, and once you understand it, it clicks. But 
that choice is not important to me.

> To simplify, hopefully you agree that your proposal can be summarized
> as:
> 
>    "return a function that receives a function that you
>     should call with the docstring"
> 
> whereas my proposal can be summarized as:
> 
>    "receive a function that you should call with the docstring"

To clarify, you actually included two proposals (behavior with patch 1, 
and with both patch 1 and patch 2 applied). The first one is the one I 
really didn't like. The second one is better, API-wise, but will 
conflict with the futures-based approach.

>> There also exist a possible modification of this patch with a bit of
>> functional programming where both calls to eldoc--handle-multiline
>> happen from inside of eldoc-documentation-default's definition.
> 
> Yes, that's independent of the shape of the callback we want to use.
> I'll leave that for later.  Specifically, eldoc--handle-multiline has to
> do quite a bit more handling (to satisfy Andrey's expectations).

I'm not so such it's independent. The complexity of implementing that 
would certainly vary.

BTW, maybe eldoc-message should handle this after all? Since it's the 
last link in the chain. Or even do that in the default value of 
eldoc-message-function (create a wrapper for minibuffer-message).

> Replying to parts from the other discussion in the Github tracker now.
> 
>> OK, another question: if the result still /valid/?
>                          ^^ Assuming you mean "is".
> 
> Well, if the client discovers the result to be invalid, ...

So the client will have to save and then compare the current buffer, the 
value of point and buffer-chars-modification-tick now?

>> No idea, a hypothetical one. It's an open API, not everybody is or
>> will be using LSP until the end of time. And it doesn't really have to
>> be huge. A saving is a saving.
> 
> There's no free lunch.  A very small saving in time for more complexity
> is bad.  That's what overengineered means.

Futures are not particularly more complex to use.

>> You can certainly kill the external process outside of it. Saving on
>> CPU expenses in general.
> 
> The future's creditor is the only one who could do that to any useful
> effect.  Does it have access to the process?  Probably not.

It can (barring any complex abstractions). It created the process, after 
all.

> You would
> have to return a complex future with a kill switch.  That's possible,
> but tricky, because you'd then have to be ready in the sentinel for
> another source of unexpected kills.

That would be none of ElDoc's business, though. But the implementation 
will get to be as complex as it needs.

> Why indeed?  Your other argument, that this makes the transition to
> proper futures (such as the ones in https://github.com/skeeto/emacs-aio)
> easier, is questionable, too.  There are two scenarios here:
> 
> - You want to keep backward compatibility to this API published in eldoc
>    1.1.0 until the time of the Emacs 28 release:

The one thing I want to avoid doing is changing the callsig of every 
documentation function, and then changing them back when we switch to 
futures.

>    This is something that I -- and Stefan, if I'm not mistaken, -- don't
>    think we should worry about.  Just because a package is :core GNU ELPA
>    doesn't necessarily mean we guarantee stability of its API.

Um, I'm pretty sure we guarantee a fair amount of stability.

>    But if we do, then we'll have to explain in the docstring that there
>    is a fourth return value for the hook functions.  In my version we'll
>    have to do exactly the same.
> 
> - You don't want to keep backward compatibility until Emacs 28:
> 
>    Then, when the super-futures are here, you can just kill the CALLBACK
>    arg if we find it doesn't fit and rewrite the docstring without
>    concerns.

Kill the arg in all built-in functions, as well as in external consumers?

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

> 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

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

> It's not complete, is it?

Don't know.  I have the impression that it's complete enough to give you
the same "power" as the callback argument.

I.e. instead of (funcall BACKEND CALLBACK)
you (eldoc-future-set-callback (funcall BACKEND) CALLBACK) and instead
of (funcall CALLBACK VALUE) you (eldoc-future-set-value FUT VALUE).

> And how to I use it to solve the
> eldoc-documentation-compose problem?

AFAIK this is orthogonal to the technique we use for the backend to run
eldoc's callback code.

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

I'm not familiar with that package, so I can't judge.  It might be an
even better option, indeed.

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

The use of futures has been discussed forever in the context of
several packages.  That's why at this stage, I think either we decide to
drop the idea or to start using it.
I'm OK with either option.


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 26.05.2020 04:21, João Távora wrote:
>> Dmitry Gutov <dgutov@yandex.ru> writes:
> They also use *special* variables? Flymake's API looks fairly clean
> (you call back to REPORTER-FN, that's very reasonable), but I haven't
> looked under the covers yet.

REPORTER-FN, is just another name for CALLBACK.  Flymake doesn't need
these special variables because its API doesn't go through an old,
argless shim like eldoc-documentation-function, singular.  If it did, it
would need a special variable, which is not such a ghoulish thing
either.

>> To simplify, hopefully you agree that your proposal can be summarized
>> as:
>>    "return a function that receives a function that you
>>     should call with the docstring"
>> whereas my proposal can be summarized as:
>>    "receive a function that you should call with the docstring"
>
> To clarify, you actually included two proposals (behavior with patch
> 1, and with both patch 1 and patch 2 applied). The first one is the
> one I really didn't like. The second one is better, API-wise, but will 
> conflict with the futures-based approach.

It's not a conflict at all.  When you do the futures-based approach
you're going to rewrite the docstring to augment the API.  You do the
same here: just add this to the docstring:

   "If you prefer, ignore CALBACK and return a `future-future' object from
    this function".

How is that different from:

   "If you prefer, instead of returning a (:ASYNC . FUNCTION) cons,
   return a `future-future' object from this function""

???

>> I'll leave that for later.  Specifically, eldoc--handle-multiline has to
>> do quite a bit more handling (to satisfy Andrey's expectations).
> I'm not so such it's independent. The complexity of implementing that
> would certainly vary.

It is quite independent.  Just look at the last patch I sent Stefan.
There's only one call to eldoc--handle-multiline (in fact we could ditch
it entirely).

> BTW, maybe eldoc-message should handle this after all? Since it's the
> last link in the chain. Or even do that in the default value of 
> eldoc-message-function (create a wrapper for minibuffer-message).

Doesn't work well becasue eldoc-message is called by both the produced
_and_ the backend, as I showed you earlier.  And it certainly doesn't
work with eldoc-documentation-compose.

>> Replying to parts from the other discussion in the Github tracker now.
>> 
>>> OK, another question: if the result still /valid/?
>>                          ^^ Assuming you mean "is".
>> Well, if the client discovers the result to be invalid, ...
>
> So the client will have to save and then compare the current buffer,
> the value of point and buffer-chars-modification-tick now?

Ah, _that_ validity.  No, no, never do that.  The client should have no
idea of it.  In the framework you either make the callback a noop, or
you set it aborted for the client to save some work.  Or both.

>>> No idea, a hypothetical one. It's an open API, not everybody is or
>>> will be using LSP until the end of time. And it doesn't really have to
>>> be huge. A saving is a saving.
>> There's no free lunch.  A very small saving in time for more
>> complexity
>> is bad.  That's what overengineered means.
>
> Futures are not particularly more complex to use.

Sure, but they are _more_ complex.  And you're mixing up two things:
futures _aren't_ the only way -- or even a particularly easy way -- to
signal to the clients that thety can give up their efforts.  All the
client needs to have is access to an object that's shared between it and
the framework.  That object _needn't_ be a first-class CLOS object or
struct.  It can be a function.

>>> You can certainly kill the external process outside of it. Saving on
>>> CPU expenses in general.
>> The future's creditor is the only one who could do that to any
>> useful
>> effect.  Does it have access to the process?  Probably not.
>
> It can (barring any complex abstractions). It created the process,
> after all.

Not really, it asked a client to solve a problem, doesn't know how the
client if the client is doing by async process or cointoss.

>> You would have to return a complex future with a kill switch.  That's
>> possible, but tricky, because you'd then have to be ready in the
>> sentinel for another source of unexpected kills.
>
> That would be none of ElDoc's business, though. But the implementation
> will get to be as complex as it needs.

_That's_ why an abort switch whose contract is "kill this immediately"
is a bad idea.  An abort switch whose contract is "just letting you know
I don't need this anymore" is better.  But again, in eldoc, not so
useful.  And again, nothing to do with futures here.

>> Why indeed?  Your other argument, that this makes the transition to
>> proper futures (such as the ones in https://github.com/skeeto/emacs-aio)
>> easier, is questionable, too.  There are two scenarios here:
>> - You want to keep backward compatibility to this API published in
>> eldoc
>>    1.1.0 until the time of the Emacs 28 release:
>
> The one thing I want to avoid doing is changing the callsig of every
> documentation function, and then changing them back when we switch to 
> futures.

So _don't_ change the "callsig".  If you implement futures you _are_
going to change the protocol anyway, i.e. write new stuff in the
docstring.

>>    This is something that I -- and Stefan, if I'm not mistaken, -- don't
>>    think we should worry about.  Just because a package is :core GNU ELPA
>>    doesn't necessarily mean we guarantee stability of its API.
>
> Um, I'm pretty sure we guarantee a fair amount of stability.

That's not what Stefan said here:

   https://github.com/joaotavora/eglot/pull/459#issuecomment-633634034

And that's not what the Dmitry from 2016 wrote in xref.el

   +;; NOTE: The xref API is still experimental and can change in major,
   +;; backward-incompatible ways.  Everyone is encouraged to try it, and
   +;; report to us any problems or use cases we hadn't anticiated, by
   +;; sending an email to emacs-devel, or `M-x report-emacs-bug'.

That has been sitting there for almost three Emacs major versions (in
fact you added it after the initial 25 release).  All I'm asking is for
a little such flexibility with eldoc.

>>    But if we do, then we'll have to explain in the docstring that there
>>    is a fourth return value for the hook functions.  In my version we'll
>>    have to do exactly the same.
>> - You don't want to keep backward compatibility until Emacs 28:
>>    Then, when the super-futures are here, you can just kill the
>> CALLBACK
>>    arg if we find it doesn't fit and rewrite the docstring without
>>    concerns.
>
> Kill the arg in all built-in functions, as well as in external
> consumers?

Yes, if we discover there aren't so many.  Currently there are 5 users
in Emacs proper, 0 in GNU ELPA and quite sure 0 elsewhere in the world.
So 0 external consumers.  Do you really think if I push my patch there
will be hordes of eldoc-documentation-functions users out there using
this horrible futureless API that I'm proposing?  There won't, and I'm
happy to add a big disclaimer to the top of the eldoc.el file.

But if you really think we wouldn't be able to bring ourselves to kill
the arg, then just reintroduce eldoc-documentation-callback, and kill
_that_ later on.  Or just don't kill the arg, period.

It just looks like you're holding this problem hostage to introducing
some kind of rushed futures solution.  I don't agree with either of
these things.  I think this particular problem shouldn't be held hostage
to rearchitecting async in Emacs, laudable and useful a goal as that
might be.  And I think a futures library should be well thought out: I'd
like to discuss this in emacs-devel, at least get some opinions on
Christopher Wellon's solution, which seems very elegant.

In the meantime, I have a working patch that follows current coding
practice, solves this problem and in no way prevents or interferes with
future work.

Anyway, see the patch I sent to Stefan.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

[  Christopher, I'm CC-ing you in this discussion becasue we've been
discussing adding futures to Emacs, and I came across your aio.el
library which seems very promising.  (I am, however, proposing a
pragmatic intermediate solution to the particular problem in the subject) ]

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

>> It's not complete, is it?
>
> Don't know.  I have the impression that it's complete enough to give you
> the same "power" as the callback argument.

> I.e. instead of (funcall BACKEND CALLBACK)
> you (eldoc-future-set-callback (funcall BACKEND) CALLBACK) and instead
> of (funcall CALLBACK VALUE) you (eldoc-future-set-value FUT VALUE).

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?

>> And how to I use it to solve the
>> eldoc-documentation-compose problem?
>
> AFAIK this is orthogonal to the technique we use for the backend to run
> eldoc's callback code.

Not really.  A "proper" futures library will have primitives that handle
this very elegantly.  I.e. a future that depends on multiple futures, a
future that applies a function to the first available future.  The kind
of stuff I'm sure you'll love, academically.  That would be the thing to
use there, otherwise we're just playing legos with structs and
functions.

>> 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?
>
> I'm not familiar with that package, so I can't judge.  It might be an
> even better option, indeed.

I believe it has such functionality that I mentioned before.  And then
some.  I don't understand it filly but it seems nicely coded, has two
authors that have (99% sure) assigned copyright.  The library calls
futures "promises", by the way.

>> 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.
> The use of futures has been discussed forever in the context of
> several packages.  That's why at this stage, I think either we decide
> to drop the idea or to start using it.

> I'm OK with either option.

Black and white, do or die? Nah, I don't buy it :-)

Let me be honest: the thing I'm not crazy about in futures is that if we
go the handrolled eldoc.el route that creates another distraction to
maintain separately.  Eldoc's problem lie elsewhere.

Let's use futures in the new completion API thingy, or in an elisp json
library, where they might brutally speed up parsing, by doing parsing
only parts of the document that users access.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

On Tue, May 26, 2020 at 2:32 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 26.05.2020 05:38, Stefan Monnier wrote:
> >> Here's a modified approach that doesn't use a global var and should make it
> >> easier to transition to using futures.
> > I think at this stage, we should either start using futures or we should
> > forget about ever using them.
> We should certainly do it before Emacs 28 is released (so we won't have
> to support whatever ad-hoc option is currently being proposed).

Agree.  By my estimate we have ~2 years to go, tho.  And ad-hoc
is a value judgement that you could apply to every part of Emacs
could be improved (then someone else might call _that_ ad-hoc).

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

> 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 ;-)
[ 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).  ]

> I believe it has such functionality that I mentioned before.  And then
> some.  I don't understand it filly but it seems nicely coded, has two
> authors that have (99% sure) assigned copyright.  The library calls
> futures "promises", by the way.

I'm willing to tweak my vocabulary.


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 26.05.2020 19:03, João Távora wrote:

>>> To simplify, hopefully you agree that your proposal can be summarized
>>> as:
>>>     "return a function that receives a function that you
>>>      should call with the docstring"
>>> whereas my proposal can be summarized as:
>>>     "receive a function that you should call with the docstring"
>>
>> To clarify, you actually included two proposals (behavior with patch
>> 1, and with both patch 1 and patch 2 applied). The first one is the
>> one I really didn't like. The second one is better, API-wise, but will
>> conflict with the futures-based approach.
> 
> It's not a conflict at all.  When you do the futures-based approach
> you're going to rewrite the docstring to augment the API.  You do the
> same here: just add this to the docstring:
> 
>     "If you prefer, ignore CALBACK and return a `future-future' object from
>      this function".
> 
> How is that different from:
> 
>     "If you prefer, instead of returning a (:ASYNC . FUNCTION) cons,
>     return a `future-future' object from this function""
> 
> ???

We're not going to do either. The :async option is only for those tho 
need async _right this second_. It would be fully replaced by the 
"futures" option. Or we can implement it right now.

And hopefully we won't have to maintain the eldoc-specific futures 
"forever" either.

>> BTW, maybe eldoc-message should handle this after all? Since it's the
>> last link in the chain. Or even do that in the default value of
>> eldoc-message-function (create a wrapper for minibuffer-message).
> 
> Doesn't work well becasue eldoc-message is called by both the produced
> _and_ the backend, as I showed you earlier.  And it certainly doesn't
> work with eldoc-documentation-compose.

We could split it apart, I guess. Or make two versions.

>>> Replying to parts from the other discussion in the Github tracker now.
>>>
>>>> OK, another question: if the result still /valid/?
>>>                           ^^ Assuming you mean "is".
>>> Well, if the client discovers the result to be invalid, ...
>>
>> So the client will have to save and then compare the current buffer,
>> the value of point and buffer-chars-modification-tick now?
> 
> Ah, _that_ validity.  No, no, never do that.  The client should have no
> idea of it.  In the framework you either make the callback a noop, or
> you set it aborted for the client to save some work.  Or both.

So the abort thing. In pre-command-hook?

>>>> No idea, a hypothetical one. It's an open API, not everybody is or
>>>> will be using LSP until the end of time. And it doesn't really have to
>>>> be huge. A saving is a saving.
>>> There's no free lunch.  A very small saving in time for more
>>> complexity
>>> is bad.  That's what overengineered means.
>>
>> Futures are not particularly more complex to use.
> 
> Sure, but they are _more_ complex.  And you're mixing up two things:
> futures _aren't_ the only way -- or even a particularly easy way -- to
> signal to the clients that thety can give up their efforts.  All the
> client needs to have is access to an object that's shared between it and
> the framework.  That object _needn't_ be a first-class CLOS object or
> struct.  It can be a function.

It's good to have a well-documented contract. Functions do _everything_. 
They can't be optimal for everything.

And this way we don't add extra arguments, so whoever doesn't need 
async, doesn't need to change a thing.

>>>> You can certainly kill the external process outside of it. Saving on
>>>> CPU expenses in general.
>>> The future's creditor is the only one who could do that to any
>>> useful
>>> effect.  Does it have access to the process?  Probably not.
>>
>> It can (barring any complex abstractions). It created the process,
>> after all.
> 
> Not really, it asked a client to solve a problem, doesn't know how the
> client if the client is doing by async process or cointoss.

Seems like we're miscommunicating.

>>> You would have to return a complex future with a kill switch.  That's
>>> possible, but tricky, because you'd then have to be ready in the
>>> sentinel for another source of unexpected kills.
>>
>> That would be none of ElDoc's business, though. But the implementation
>> will get to be as complex as it needs.
> 
> _That's_ why an abort switch whose contract is "kill this immediately"
> is a bad idea.  An abort switch whose contract is "just letting you know
> I don't need this anymore" is better.  But again, in eldoc, not so
> useful.  And again, nothing to do with futures here.

Here as well.

>>> Why indeed?  Your other argument, that this makes the transition to
>>> proper futures (such as the ones in https://github.com/skeeto/emacs-aio)
>>> easier, is questionable, too.  There are two scenarios here:
>>> - You want to keep backward compatibility to this API published in
>>> eldoc
>>>     1.1.0 until the time of the Emacs 28 release:
>>
>> The one thing I want to avoid doing is changing the callsig of every
>> documentation function, and then changing them back when we switch to
>> futures.
> 
> So _don't_ change the "callsig".  If you implement futures you _are_
> going to change the protocol anyway, i.e. write new stuff in the
> docstring.

See above about not having to change anything.

>>>     This is something that I -- and Stefan, if I'm not mistaken, -- don't
>>>     think we should worry about.  Just because a package is :core GNU ELPA
>>>     doesn't necessarily mean we guarantee stability of its API.
>>
>> Um, I'm pretty sure we guarantee a fair amount of stability.
> 
> That's not what Stefan said here:
> 
>     https://github.com/joaotavora/eglot/pull/459#issuecomment-633634034
> 
> And that's not what the Dmitry from 2016 wrote in xref.el
> 
>     +;; NOTE: The xref API is still experimental and can change in major,
>     +;; backward-incompatible ways.  Everyone is encouraged to try it, and
>     +;; report to us any problems or use cases we hadn't anticiated, by
>     +;; sending an email to emacs-devel, or `M-x report-emacs-bug'.
> 
> That has been sitting there for almost three Emacs major versions (in
> fact you added it after the initial 25 release).  All I'm asking is for
> a little such flexibility with eldoc.

I wrote that for xref.el because that was the state of affairs, and xref 
was relatively new. Especially compared to eldoc.

But we're probably miscommunicating here as well.

>>>     But if we do, then we'll have to explain in the docstring that there
>>>     is a fourth return value for the hook functions.  In my version we'll
>>>     have to do exactly the same.
>>> - You don't want to keep backward compatibility until Emacs 28:
>>>     Then, when the super-futures are here, you can just kill the
>>> CALLBACK
>>>     arg if we find it doesn't fit and rewrite the docstring without
>>>     concerns.
>>
>> Kill the arg in all built-in functions, as well as in external
>> consumers?
> 
> Yes, if we discover there aren't so many.  Currently there are 5 users
> in Emacs proper, 0 in GNU ELPA and quite sure 0 elsewhere in the world.

OK, I see your point: eldoc-documentation-functions is new. And 
apparently you don't intend to add this feature to the variable without "s".

> It just looks like you're holding this problem hostage to introducing
> some kind of rushed futures solution.  I don't agree with either of
> these things.

Who's holding what hostage? I showed a smoother approach, you didn't 
like it. No big surprise about that.

> I think this particular problem shouldn't be held hostage
> to rearchitecting async in Emacs, laudable and useful a goal as that
> might be.  And I think a futures library should be well thought out: I'd
> like to discuss this in emacs-devel, at least get some opinions on
> Christopher Wellon's solution, which seems very elegant.

We should certainly take a look at it. But the built-in futures don't 
have to provide all the options. Just be functionally compatible. 
Christopher could pick up from that.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> no idea of it.  In the framework you either make the callback a noop,
>> or you set it aborted for the client to save some work.  Or both.
>
> So the abort thing. In pre-command-hook?

No, the creditor of the future or issuer of the callback aborts or
invalidates the previous version just before issuing a new one.  Nothing
pre-command-hook here.  Invalidation may or may not entail letting the
holder of the callback know that the previous call became invalid.
Flymake does this: by invoking a backend again with a new callback
instance it is signalling that the preceding one became invalid.  If the
backend tries to call the previous callback, it is an error and the
backend is disabled.

> It's good to have a well-documented contract. Functions do
> _everything_. They can't be optimal for everything.

You're missing a Lisp point here.  It doesn't matter if it's an CLOS
object, a struct, a function or my beautiful singing voice: it just has
to be an object which you can make unique instances of and can respond
to funcall, still-wanted-p, (setf still-wanted-p), errored-p, and (setf
errored-p).  That's the contract.  A function fits perfectly.

>>>> The future's creditor is the only one who could do that to any
>>>> useful effect.  Does it have access to the process?  Probably not.
>>> It can (barring any complex abstractions). It created the process,
>>> after all.
>> Not really, it asked a client to solve a problem, doesn't know how
>> the client if the client is doing by async process or cointoss.
> Seems like we're miscommunicating.

Well you implied that the creditor of the future (the caller who
received) created the process.  It does not.  See the patch to Stefan.

>> is a bad idea.  An abort switch whose contract is "just letting you know
>> I don't need this anymore" is better.  But again, in eldoc, not so
>> useful.  And again, nothing to do with futures here.
>
> Here as well.

OK. The takeaway is that "aborting a future" doesn't need any constructs
specific to futures, is all.

> See above about not having to change anything.

But then we don't have to change anything in any case!  I already
changed EVERY user of eldoc-documentation-functions: every single one of
the 5 in existence in the entire world.  So we're all good.

>>     +;; NOTE: The xref API is still experimental and can change in
>> major,
>>     +;; backward-incompatible ways.  Everyone is encouraged to try it, and
>>     +;; report to us any problems or use cases we hadn't anticiated, by
>>     +;; sending an email to emacs-devel, or `M-x report-emacs-bug'.
>> That has been sitting there for almost three Emacs major versions
>> (in
>> fact you added it after the initial 25 release).  All I'm asking is for
>> a little such flexibility with eldoc.
>
> I wrote that for xref.el because that was the state of affairs, and
> xref was relatively new. Especially compared to eldoc.

This part of eldoc.el is probably newer than xref.el was when you wrote
that.

> But we're probably miscommunicating here as well.

Doubt it.

> OK, I see your point: eldoc-documentation-functions is new. And
> apparently you don't intend to add this feature to the variable
> without "s".

Yes, exactly.  eldoc-documentation-function should be obsoleted IMO.

>> It just looks like you're holding this problem hostage to introducing
>> some kind of rushed futures solution.  I don't agree with either of
>> these things.
>
> Who's holding what hostage? I showed a smoother approach, you didn't
> like it. No big surprise about that.

Let me explain. First: it's clearly not "smoother", your're asking users
to wrap their heads around a function that returns a function taking a
function.  That's not what I want to present to Eglot contributors, for
instance.  And I'm not too crazy with presenting them this "future
thing" that is completely different from Eglot's use of Flymake,
jsonrpc.el, completion-at-point, etc...  In other words, my ambition is
consistency and you seem to be denying it for reasons I can't
understand, because nothing in the steps I am taking denies _your_
ambitions, which seem to be futures.  That's why I speak of "hostage".

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 26.05.2020 23:00, João Távora wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>>> no idea of it.  In the framework you either make the callback a noop,
>>> or you set it aborted for the client to save some work.  Or both.
>>
>> So the abort thing. In pre-command-hook?
> 
> No, the creditor of the future or issuer of the callback aborts or
> invalidates the previous version just before issuing a new one. Nothing
> pre-command-hook here.

Where/when would eldoc-mode do it?

> Invalidation may or may not entail letting the
> holder of the callback know that the previous call became invalid.

Letting know the issuer of the future, you mean.

> Flymake does this: by invoking a backend again with a new callback
> instance it is signalling that the preceding one became invalid.  If the
> backend tries to call the previous callback, it is an error and the
> backend is disabled.

Worse is sometimes better, we know.

>> It's good to have a well-documented contract. Functions do
>> _everything_. They can't be optimal for everything.
> 
> You're missing a Lisp point here.  It doesn't matter if it's an CLOS
> object, a struct, a function or my beautiful singing voice: it just has
> to be an object which you can make unique instances of and can respond
> to funcall, still-wanted-p, (setf still-wanted-p), errored-p, and (setf
> errored-p).  That's the contract.  A function fits perfectly.

That would be my "alternative" suggestion: for 
eldoc-documentation-functions elements to return a function (denoted as 
FETCHER in the docstring) if they want the async convention.

>>>>> The future's creditor is the only one who could do that to any
>>>>> useful effect.  Does it have access to the process?  Probably not.
>>>> It can (barring any complex abstractions). It created the process,
>>>> after all.
>>> Not really, it asked a client to solve a problem, doesn't know how
>>> the client if the client is doing by async process or cointoss.
>> Seems like we're miscommunicating.
> 
> Well you implied that the creditor of the future (the caller who
> received) created the process.  It does not.  See the patch to Stefan.

Okay, creditor != creator. But what you've said a few messages back 
(seen at the top of the quotes chain above) doesn't make sense: the 
creditor will call (future-abort fut), and the issuer of the future can 
make sure that this operation will indeed kill the process.

That's the main idea behind aborting futures. Or canceling. Whatever 
term we're going to pick.

>> See above about not having to change anything.
> 
> But then we don't have to change anything in any case!  I already
> changed EVERY user of eldoc-documentation-functions: every single one of
> the 5 in existence in the entire world.  So we're all good.

And then we'll need to change them back. And in the meantime, the new 
convention could get external users (some people do live on the bleeding 
edge), and this will get messier.

>> OK, I see your point: eldoc-documentation-functions is new. And
>> apparently you don't intend to add this feature to the variable
>> without "s".
> 
> Yes, exactly.  eldoc-documentation-function should be obsoleted IMO.

Perhaps. I'm also not buying the usefulness of eldoc-documentation-compose.

>>> It just looks like you're holding this problem hostage to introducing
>>> some kind of rushed futures solution.  I don't agree with either of
>>> these things.
>>
>> Who's holding what hostage? I showed a smoother approach, you didn't
>> like it. No big surprise about that.
> 
> Let me explain. First: it's clearly not "smoother", your're asking users
> to wrap their heads around a function that returns a function taking a
> function.  That's not what I want to present to Eglot contributors, for
> instance.

Would they need to? As soon as an existing Eglot's implementation is in 
place, that exact part of the code wouldn't need to be touched often.

In any case, you are over-exaggerating. This exact design has been a 
part of "asynchronous" backend calling convention in Company for years. 
And not once have I seen a complaint that it's overly complex.

> And I'm not too crazy with presenting them this "future
> thing" that is completely different from Eglot's use of Flymake,
> jsonrpc.el, completion-at-point, etc...

Didn't you say that Flymake could use futures as well?

> In other words, my ambition is
> consistency and you seem to be denying it for reasons I can't
> understand, because nothing in the steps I am taking denies _your_
> ambitions, which seem to be futures.  That's why I speak of "hostage".

See above. But perhaps we should after all suspend this discussion until 
we had a chance to reach a better mutual understanding of the futures 
API, and how we expect it to help (or not). I promise to show some 
proposals in the near future.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

On Wed, May 27, 2020 at 10:14 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> > No, the creditor of the future or issuer of the callback aborts or
> > invalidates the previous version just before issuing a new one. Nothing
> > pre-command-hook here.
>
> Where/when would eldoc-mode do it?

In the idle timer function.  That's when it decides it wants
new info, and so any old info that hasn't arrived yet is probably
out of date.

But I think I understnad what you mean pre-command-hook.
You suggested pre-command-hook because that's where you
can check if point is far enough away from the place where
the original request originated?  That could work, but I'd prefer
that the info to _do_ come in, because after that info is about
the  last time my point hung out at a particular position long
enough to trigger a request.  This is how Eglot, SLY and
SLIME work, btw.

> > Invalidation may or may not entail letting the
> > holder of the callback know that the previous call became invalid.
>
> Letting know the issuer of the future, you mean.

Or the holder of the callback depending on which abstraction
we're talking about.  I define the holder of the callback as
he who is about to call it.

> > Flymake does this: by invoking a backend again with a new callback
> > instance it is signalling that the preceding one became invalid.  If the
> > backend tries to call the previous callback, it is an error and the
> > backend is disabled.
>
> Worse is sometimes better, we know.

That is a very confusing statement.

> >> It's good to have a well-documented contract. Functions do
> >> _everything_. They can't be optimal for everything.
> >
> > You're missing a Lisp point here.  It doesn't matter if it's an CLOS
> > object, a struct, a function or my beautiful singing voice: it just has
> > to be an object which you can make unique instances of and can respond
> > to funcall, still-wanted-p, (setf still-wanted-p), errored-p, and (setf
> > errored-p).  That's the contract.  A function fits perfectly.
>
> That would be my "alternative" suggestion: for
> eldoc-documentation-functions elements to return a function (denoted as
> FETCHER in the docstring) if they want the async convention.

They need to _receive_ an object produced externally, somhow.
If they return a function as youv suggest, they are only doing so
they can later _receive_ another object.  This is needlessly
complicated.  To receive objects in some place, just use argument
the argument list of that place.
>
> >>>>> The future's creditor is the only one who could do that to any
> >>>>> useful effect.  Does it have access to the process?  Probably not.
> >>>> It can (barring any complex abstractions). It created the process,
> >>>> after all.
> >>> Not really, it asked a client to solve a problem, doesn't know how
> >>> the client if the client is doing by async process or cointoss.
> >> Seems like we're miscommunicating.
> >
> > Well you implied that the creditor of the future (the caller who
> > received) created the process.  It does not.  See the patch to Stefan.
>
> Okay, creditor != creator. But what you've said a few messages back
> (seen at the top of the quotes chain above) doesn't make sense: the
> creditor will call (future-abort fut), and the issuer of the future can
> make sure that this operation will indeed kill the process.

No, it does make sense. Read it again. What you're saying is what I
meant. But that still means that the process sentinel will have to deal
with out-of-band kills that it must distinguish from other out-of-band
kills (such as, say, a kill -9 from the shell). That is added complexity.

It is better, in my opinion, to make this softer.  Let the creditor
signal, I don't need this anymore, and the issuer will take appropriate
measures synchronously, i.e. in the process filter and not in the
process sentinel.

> That's the main idea behind aborting futures. Or canceling. Whatever
> term we're going to pick.

But, again, nothing you're describing here can't be implemented
with passing a callback in the arglist. It's independent.  Futures
particularly the versions you and Stefan are proposing are just
other places to type basically the same sexps.  They're a stylistic
change over callbacks, but nothing more, fundamentally.

> >> See above about not having to change anything.
> >
> > But then we don't have to change anything in any case!  I already
> > changed EVERY user of eldoc-documentation-functions: every single one of
> > the 5 in existence in the entire world.  So we're all good.
>
> And then we'll need to change them back.

Nothing of the sort.

> And in the meantime, the new
> convention could get external users (some people do live on the bleeding
> edge), and this will get messier.

That's only if you assume you'll break compatibility, because you
dislike callbacks so much that you want to close off the arglist
forever.  But not only may users may not dislike them, they may
even prefer them. And keeping the arglist open, not closed, is a
very good idea for extensibility functional API's anyway.

> >> OK, I see your point: eldoc-documentation-functions is new. And
> >> apparently you don't intend to add this feature to the variable
> >> without "s".
> >
> > Yes, exactly.  eldoc-documentation-function should be obsoleted IMO.
>
> Perhaps. I'm also not buying the usefulness of eldoc-documentation-compose.

Yeah,I don't get it particularly, either.  I mean, I can see its uses.
but I'm glad you're finally getting the overengineered feeling :-)
And it's waay more useful than futures here.

> >>> It just looks like you're holding this problem hostage to introducing
> >>> some kind of rushed futures solution.  I don't agree with either of
> >>> these things.
> >>
> >> Who's holding what hostage? I showed a smoother approach, you didn't
> >> like it. No big surprise about that.
> >
> > Let me explain. First: it's clearly not "smoother", your're asking users
> > to wrap their heads around a function that returns a function taking a
> > function.  That's not what I want to present to Eglot contributors, for
> > instance.
>
> Would they need to? As soon as an existing Eglot's implementation is in
> place, that exact part of the code wouldn't need to be touched often.

Code is read much, much more than is is written.  And WTF per minute
_are_ a measure of code quality.  I would like to avoid this particular
WTF please.

> In any case, you are over-exaggerating. This exact design has been a

"over-exaggerating".  Very meta.

> part of "asynchronous" backend calling convention in Company for years.

People will use what you give them, if you they have no other option.

> And not once have I seen a complaint that it's overly complex.

Anyway, count one now.  Or don't.  I mean, I really dislike
company-backends throughout, but I don't use it, either. I mean
I know you inherited it and I had to hack on it to add flex support
for the capf thing, but it's a strange re-implementation of functions
to have one single function that does wildly different things based
on one argument.   I guess at the time there weren't generic
functions. And the cons :async thing is equally confusing to me.
Sorry to be frank. But I guess some people will love it, for some
equally legitimate reason: it will seem "right" to them, or "clever".

> > And I'm not too crazy with presenting them this "future
> > thing" that is completely different from Eglot's use of Flymake,
> > jsonrpc.el, completion-at-point, etc...
>
> Didn't you say that Flymake could use futures as well?

It could, sure, especially if you have a thousand futuristic devs
itching to write backends but not grokking callbacks.  But let's
not kill the existing API, please.

> > In other words, my ambition is
> > consistency and you seem to be denying it for reasons I can't
> > understand, because nothing in the steps I am taking denies _your_
> > ambitions, which seem to be futures.  That's why I speak of "hostage".
>
> See above. But perhaps we should after all suspend this discussion until
> we had a chance to reach a better mutual understanding of the futures
> API, and how we expect it to help (or not). I promise to show some
> proposals in the near future.

Suspend this discussion?  Sure, this discussion yes, if your want.
But not this bugfix: that would be exactly what "holding hostage"
means. Don't hold this bugfix hostage: it has nothing to do with
futures.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 28.05.2020 01:13, João Távora wrote:
> On Wed, May 27, 2020 at 10:14 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>>> No, the creditor of the future or issuer of the callback aborts or
>>> invalidates the previous version just before issuing a new one. Nothing
>>> pre-command-hook here.
>>
>> Where/when would eldoc-mode do it?
> 
> In the idle timer function.  That's when it decides it wants
> new info, and so any old info that hasn't arrived yet is probably
> out of date.

Then eldoc might shows some info when it's no longer pertaining to the 
context around the point.

> But I think I understnad what you mean pre-command-hook.
> You suggested pre-command-hook because that's where you
> can check if point is far enough away from the place where
> the original request originated?

Or always abort, as soon as the user invokes the next command.

>>> Flymake does this: by invoking a backend again with a new callback
>>> instance it is signalling that the preceding one became invalid.  If the
>>> backend tries to call the previous callback, it is an error and the
>>> backend is disabled.
>>
>> Worse is sometimes better, we know.
> 
> That is a very confusing statement.

It's a somewhat incorrect behavior that is, however, easier to implement.

Which is fully along the lines of Richard P. Gabriel's "The Rise of 
Worse Is Better".

>> That would be my "alternative" suggestion: for
>> eldoc-documentation-functions elements to return a function (denoted as
>> FETCHER in the docstring) if they want the async convention.
> 
> They need to _receive_ an object produced externally, somhow.
> If they return a function as youv suggest, they are only doing so
> they can later _receive_ another object.  This is needlessly
> complicated.  To receive objects in some place, just use argument
> the argument list of that place.

"Returning a value" is meaningful semantic. Even when that value is a 
function.

>> Okay, creditor != creator. But what you've said a few messages back
>> (seen at the top of the quotes chain above) doesn't make sense: the
>> creditor will call (future-abort fut), and the issuer of the future can
>> make sure that this operation will indeed kill the process.
> 
> No, it does make sense. Read it again. What you're saying is what I
> meant.

Perhaps you could have agreed then.

> But that still means that the process sentinel will have to deal
> with out-of-band kills that it must distinguish from other out-of-band
> kills (such as, say, a kill -9 from the shell). That is added complexity.

It's their choice. Some processes might run too long in certain cases. 
So that would be a safeguard.

> It is better, in my opinion, to make this softer.  Let the creditor
> signal, I don't need this anymore, and the issuer will take appropriate
> measures synchronously, i.e. in the process filter and not in the
> process sentinel.

Either way, that would require an additional way to signal. Try to fit 
this into your proposal. It won't match so well.

>> That's the main idea behind aborting futures. Or canceling. Whatever
>> term we're going to pick.
> 
> But, again, nothing you're describing here can't be implemented
> with passing a callback in the arglist. It's independent.  Futures
> particularly the versions you and Stefan are proposing are just
> other places to type basically the same sexps.  They're a stylistic
> change over callbacks, but nothing more, fundamentally.

Hence my request to wait a little.

Stefan suggested the simplest version because it both fits your current 
requirements, and it can be extended without breaking that usage.

>> Perhaps. I'm also not buying the usefulness of eldoc-documentation-compose.
> 
> Yeah,I don't get it particularly, either.  I mean, I can see its uses.
> but I'm glad you're finally getting the overengineered feeling :-)

No "finally", that was my opinion of it from the outset.

> And it's waay more useful than futures here.

Way to extend the bridge and then kick it down.

>> Would they need to? As soon as an existing Eglot's implementation is in
>> place, that exact part of the code wouldn't need to be touched often.
> 
> Code is read much, much more than is is written.  And WTF per minute
> _are_ a measure of code quality.  I would like to avoid this particular
> WTF please.

Okay, here's another argument: "Promise", or a "Future", or "Deferred" 
or whatever, are common notions across many programming languages now.

When a programmer encounters an idea familiar to them, they understand a 
program more easily. No WTFs.

>> In any case, you are over-exaggerating. This exact design has been a
> 
> "over-exaggerating".  Very meta.

Plain English.

>> And not once have I seen a complaint that it's overly complex.
> 
> Anyway, count one now.  Or don't.  I mean, I really dislike
> company-backends throughout, but I don't use it, either.

Noted.

> I mean
> I know you inherited it and I had to hack on it to add flex support
> for the capf thing, but it's a strange re-implementation of functions
> to have one single function that does wildly different things based
> on one argument.   I guess at the time there weren't generic
> functions. And the cons :async thing is equally confusing to me.
> Sorry to be frank. But I guess some people will love it, for some
> equally legitimate reason: it will seem "right" to them, or "clever".

It's very simple. Much simpler than generic functions or c-a-p-f.

So I'm surprised to see you disparage both ideas (one simpler than what 
you do, another a tiny bit more complex) as complex and outlandish.

>> Didn't you say that Flymake could use futures as well?
> 
> It could, sure, especially if you have a thousand futuristic devs
> itching to write backends but not grokking callbacks.  But let's
> not kill the existing API, please.

Probably not. Unless we find a good use in it for the extra capabilities 
provided by futures.

> Suspend this discussion?  Sure, this discussion yes, if your want.
> But not this bugfix: that would be exactly what "holding hostage"
> means. Don't hold this bugfix hostage: it has nothing to do with
> futures.

It's a new feature, not a bugfix.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

On Thu, May 28, 2020 at 12:35 AM Dmitry Gutov <dgutov@yandex.ru> wrote:

> It's a somewhat incorrect behavior that is, however, easier to implement.

In the futures, how would you prevent a client from giving you
a stale future? You have the very same problem. Callbacks
aren't "easier" to implement than futures, either.  I'm not saying
they are "superior", as you seem to be pretending with futures,
They just happen to be the style that's most used in Emacs.

Futures are really good when you bring in continuations:
i.e. functions that can halt and resume their processing, so
you can write normally, as if there was no async, but still have
it happen automatically. But that requires an evaluator, which
is what generator.el does, and that's what enables proper
futures like the ones in emacs-aio. Anything other than
that is just moving objects and funcalls around, for style
(or lack thereof, to some people)

> Either way, that would require an additional way to signal. Try to fit
> this into your proposal. It won't match so well.

I've already shown you're mistaken.

> > Suspend this discussion?  Sure, this discussion yes, if your want.
> > But not this bugfix: that would be exactly what "holding hostage"
> > means. Don't hold this bugfix hostage: it has nothing to do with
> > futures.
>
> It's a new feature, not a bugfix.

No it's not.  Async clients are using internal functions.
eldoc-message is an internal function. eldoc is broken
for async clients, always has been. You know this, of
course. I worked on a fix and your're holding it up because
of a longing for an abstraction that you kinda like but
are still having doubts about.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

> --- 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>"
"Future object."
> +  (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>

  "Special hook run to get the documentation string at point.
Each function is called with no argument and should return either nil
or an `eldoc-future` object that should have its `value` set as soon
as possible via `eldoc-future-set-value` (it can be set before
returning the future or at a later time).
This value should be a string, typically occupying only a single line.
In case the function ends up finding no information it is allowed
not to `eldoc-future-set-value` at all."

> +  (let ((x (run-hook-with-args-until-success 'eldoc-documentation-functions)))
> +    (if (eldoc-future-p x) (eldoc-future-set-callback x eldoc--callback)
> +      x)))

I'd simplify this to:

    (let ((x (run-hook-with-args-until-success 'eldoc-documentation-functions)))
      (when x (eldoc-future-set-callback x eldoc--callback)))

But I'd expect that there would be no `eldoc--callback` and that it's
each `eldoc-documentation-function` which chooses its own callback
rather than being chosen by their caller.


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

> "Future object."
>> +  (value 'eldoc-future--unset)
>> +  callback)
>> +
>> +(defun eldoc-future-set (f v)
>> +  "<WORLD CLASS DOCSTRING>"

Ahem.

>> +  (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>"

Ahem hem  :-)

>> +<WORLD CLASS DOCSTRING GOES HERE>
>
>   "Special hook run to get the documentation string at point.
> Each function is called with no argument and should return either nil
> or an `eldoc-future` object that should have its `value` set as soon
> as possible via `eldoc-future-set-value` (it can be set before
> returning the future or at a later time).
> This value should be a string, typically occupying only a single line.

Sometimes clients want to return more than one value, i.e. set more than
one value.  In this case it would be, for instance, the type of the
docstring being returns (function signature, value of a variable, etc).
The callback strategy makes it easy because there are lambda lists of
all shapes and sizes.  How does the future approach handle this?  Do
clients return structured lists of things?

> In case the function ends up finding no information it is allowed
> not to `eldoc-future-set-value` at all."

This is problematic.  In the eldoc-documentation-compose situation we
need to wait for every backend to reply before composing.  In other
situation, indeed we can be greedy.  But I don't think you can just say
to clients they dont'need to do anything.  If they returned the future
they are making _a promise_.  If they don't fullfil it something goes
wrong.

The callback case is the same, if you return non-nil non-string, you,
the eldoc function, are _required_, by Eldoc law, to call it, even if
with nil.

The equivalent in futures is just to say clients can set the value nil,
or some other application-specific indication of "sorry, I failed".

> each `eldoc-documentation-function` which chooses its own callback
> rather than being chosen by their caller.

For this to work, i.e. to be able to handle multiple responses, I think
it has to be set by their caller, i.e. eldoc-print-current-symbol-info:
that's the central "hub" that maintains information about how many
backends have responded and how many haven't.

But indeed that's only an implementation detail.  If you can make the
defined behaviour work work in some other simpler way, I have no
objections, of course.

João

bug#41531: 28.0.50; proper Eldoc async support

[João Távora]

Hello again gang,

As you might have seen in emacs-diffs, I have pushed a new
scratch/eldoc-async branch to the Savannah repo.  If you don't have this
repo setup you can find the commits here in the meantime:

   https://github.com/emacs-mirror/emacs/tree/scratch/eldoc-async

The code, which touches mostly lisp/emacs-lisp/eldoc.el contains the
logical continuation of the improvements I started building two weeks
ago.

When applied, these improvements result in a substantial reduction of
Eglot's documentation handling code.  Yes, Andrii, that means all our
hand-crafted, hard work of doc handling functions is soon gone,
including the awkward eglot-put-doc-in-help-buffer,
eglot-auto-display-help-buffer and the *eglot-help* buffer.

Feels sad but also good, because deleted code is good code.  And it's
not in vain because your feedback and testing was fundamental here.  And
the good news is that the logic problems about blinking and giving
priority to some docs is gone.

There are 6 commits in total, that build on top of each other.  In
reverse order

    10834f20ec * lisp/emacs-lisp/eldoc.el (Version): Bump to 1.1.0
    fe93e5b9d5 Make eldoc-print-current-symbol-info a command
    0612bb7ab5 Introduce eldoc-prefer-doc-buffer defcustom
    40d45067ba Overhaul and handle (most of) eldoc-echo-area-use-multiline-p in Eldoc itself
    600b9c0a71 New eldoc-documentation-eager value for eldoc-documentation-function
    cde8a6ab98 Better handle asynchronously produced eldoc docstrings

And the new version of Eglot that makes uses of this eldoc is here.

    https://github.com/joaotavora/eglot/tree/scratch/work-with-new-eldoc

It's important to note that Eldoc remains backward-compatible to all its
previous users.

In eldoc.el, things build on top of existing work, which includes the
reasonably new eldoc-documentation-functions (plural).  Earlier, I
thought of this variable somewhat useless, but it's really not.  In
fact, it's exactly what Eglot, SLY and other modes are after.  Even
emacs-lisp-mode itself could benefit as it's often the case that one
loses the documentation for a special variable just because one happen
to be inside a function call, or vice versa.
eldoc-documentation-functions allows us to split up these two competing
sources of documentation, so thank you Mark, or whomever had this great
idea.

An orthogonal question is how to display the documentation we gather
from multiple sources.  That is handled by eldoc-documentation-function
(singular), another variable I had underestimated.  In my changes, I
have added a third option to the two already existing ones.

  :type '(radio (function-item eldoc-documentation-default)
                (function-item eldoc-documentation-compose)
                (function-item eldoc-documentation-eager)
                (function :tag "Other function"))
  :version "28.1")

Eglot defaults to eldoc-documentation-eager, which simulates its
previous behavior.  I suggest you see the docstring for each.
Certainly we can have more strategies to combine documentation sources.

Another important development is that now that the display is
centralized, the formatiing can also be.  Thus a big part of
eldoc-echo-area-use-multiline-p can be easily honoured in eldoc.el
itself.

But the most complex changes pertain to async backends, such as Eglot's
and some (but not all) of SLY.  This is also covered.

On this point, it is also extremely super-important to note that even
though I have NOT used "futures" or "promises" in these patches, the
very same things can be achieved with them, give or take some code here
and there and some head-wrapping around different debugging techniques.
I have NOT focused on the particular async programming technique: I just
used the simplest and most commonly used in Emacs.

Finally, there is a bit of future-proof unused API.  Backends can call
the documentation-reporting callback with keyword arguments, such as
`:hint`.  But they're not really used yet for anything yet.  Some more
sophisticated text formatting in the *eldoc doc* buffer, including
renaming it like Eglot used to do to its *Eglot doc* buffer, is a
possibility.

Another possibility yet, also left unexplored, is for Flymake diagnostic
messages at point to be reported as independent documentation sources.
This is a frequent complain about Eglot.  But in fact I think it should
be Flymake-mode itself that adds some function to
eldoc-documentation-functions.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

>>> +  "<WORLD CLASS DOCSTRING>"
> Ahem.
>>> +  "<WORLD CLASS DOCSTRING>"
> Ahem hem  :-)

Not sure what you mean ;-)

>>   "Special hook run to get the documentation string at point.
>> Each function is called with no argument and should return either nil
>> or an `eldoc-future` object that should have its `value` set as soon
>> as possible via `eldoc-future-set-value` (it can be set before
>> returning the future or at a later time).
>> This value should be a string, typically occupying only a single line.
> Sometimes clients want to return more than one value, i.e. set more than
> one value.

You mean a single call could return first a function signature and
a while later a docstring?  Or at the same time.
If at the same time, then it should return them combined into a single
value (concatenation of strings, list, you name it).

The infrastructure so far only accepts strings as far as I know, so
until that is changed the question seems moot.

> The callback strategy makes it easy because there are lambda lists of
> all shapes and sizes.

It's trivial to use a list to bring the number back down to 1, so it's
not much of a difference.

> How does the future approach handle this?
> Do clients return structured lists of things?

If we want to extend it that way, that would be the natural thing to do,
I guess.  Tho without knowing what the different elements represent
(i.e. some kind of semantic information about that list), I'm not sure
what the callback can do with it other than presume that all elements
are strings and concatenate them.

>> In case the function ends up finding no information it is allowed
>> not to `eldoc-future-set-value` at all."
> This is problematic.  In the eldoc-documentation-compose situation we
> need to wait for every backend to reply before composing.

We definitely don't want to wait: if a backend responds quickly we
should show that info immediately, and update the info if/when another
backend gives additional info.

OTOH indeed for the non-composing situation, we'd need to know that the
1st backend finished unsuccessfully before being able to use the
second backend.  So I guess you're right: we shouldn't allow backends to
drop requests :-(

>> each `eldoc-documentation-function` which chooses its own callback
>> rather than being chosen by their caller.
> For this to work, i.e. to be able to handle multiple responses, I think
> it has to be set by their caller, i.e. eldoc-print-current-symbol-info:
> that's the central "hub" that maintains information about how many
> backends have responded and how many haven't.

I don't think this structure can work well: the "hub" needs to work
differently for compose than for "return first".


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

>>>> +  "<WORLD CLASS DOCSTRING>"
>> Ahem.
>>>> +  "<WORLD CLASS DOCSTRING>"
>> Ahem hem  :-)
>
> Not sure what you mean ;-)
>
>>>   "Special hook run to get the documentation string at point.
>>> Each function is called with no argument and should return either nil
>>> or an `eldoc-future` object that should have its `value` set as soon
>>> as possible via `eldoc-future-set-value` (it can be set before
>>> returning the future or at a later time).
>>> This value should be a string, typically occupying only a single line.
>> Sometimes clients want to return more than one value, i.e. set more than
>> one value.
>
> You mean a single call could return first a function signature and
> a while later a docstring?

No, that's not what I mean.  Those should be two different members of
eldoc-documentation-functions (plural).  I meant producing metadata
about the string just returned.  Much like string properties, I suppose,
but not specifically attached to regions of the string.

> The infrastructure so far only accepts strings as far as I know, so
> until that is changed the question seems moot.

Very soon that might change, but yes.

>> The callback strategy makes it easy because there are lambda lists of
>> all shapes and sizes.
> It's trivial to use a list to bring the number back down to 1, so it's
> not much of a difference.

Yes, I agree, but it's IMO easier to read (funcall cb :foo 42 :baz 23)
than (set-value fut (list :foo 42 :baz 23)).

>> How does the future approach handle this?
>> Do clients return structured lists of things?
>
> If we want to extend it that way, that would be the natural thing to do,
> I guess.  Tho without knowing what the different elements represent
> (i.e. some kind of semantic information about that list), I'm not sure
> what the callback can do with it other than presume that all elements
> are strings and concatenate them.

See my most recent patches, there are many ways to handle differently
formated and differently timed reports.

>>> In case the function ends up finding no information it is allowed
>>> not to `eldoc-future-set-value` at all."
>> This is problematic.  In the eldoc-documentation-compose situation we
>> need to wait for every backend to reply before composing.
>
> We definitely don't want to wait: if a backend responds quickly we
> should show that info immediately, and update the info if/when another
> backend gives additional info.

That depends.  Depends on the strategy.  I don't take
eldoc-documentation-compose to mean that, for example.  But we could
easily have eldoc-documentation-compose-wait and
eldoc-documentation-compose-eager.

> OTOH indeed for the non-composing situation, we'd need to know that the
> 1st backend finished unsuccessfully before being able to use the
> second backend.  So I guess you're right: we shouldn't allow backends to
> drop requests :-(

I also don't take eldoc-documentation-default to mean that.  I take it
to mean: focus on the first guy that promised he would produce
something.

But we could indeed have a "focus on the first guy that actually
produced something".

>>> each `eldoc-documentation-function` which chooses its own callback
>>> rather than being chosen by their caller.
>> For this to work, i.e. to be able to handle multiple responses, I think
>> it has to be set by their caller, i.e. eldoc-print-current-symbol-info:
>> that's the central "hub" that maintains information about how many
>> backends have responded and how many haven't.
>
> I don't think this structure can work well: the "hub" needs to work
> differently for compose than for "return first".

Yes, of course it works differently.  How "well" is another question,
whose details I don't fuilly understand.  In the last patches I showed
it works decently well, i.e. I don't see anything wrong with it.  I
implemented eldoc-documentation-default, eldoc-documentation-compose,
and something eldoc-documentation-eager which is like "default" but will
show the first one, and potentially replace with a slower, but more
important one.  These were all done "in the hub", without lots of code.
I'm fairly confident other strategies can be implemented "in the hub"
easily.

In fact, a much better name for eldoc-documentation-function (singular)
is eldoc-documentation-strategy, not least because it relieves us from
this silly singular/plural confusion.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

>> You mean a single call could return first a function signature and
>> a while later a docstring?
> No, that's not what I mean.  Those should be two different members of
> eldoc-documentation-functions (plural).

Great.

>>> The callback strategy makes it easy because there are lambda lists of
>>> all shapes and sizes.
>> It's trivial to use a list to bring the number back down to 1, so it's
>> not much of a difference.
> Yes, I agree, but it's IMO easier to read (funcall cb :foo 42 :baz 23)
> than (set-value fut (list :foo 42 :baz 23)).

I find the difference largely irrelevant.  Much more important is
what kinds of :foo and bar are allowed and what they do.

> In fact, a much better name for eldoc-documentation-function (singular)
> is eldoc-documentation-strategy, not least because it relieves us from
> this silly singular/plural confusion.

Sounds very good.  Changing its name will make it possible to fix the
current backward-incompatibility (which we'd fix by re-introducing a(n
obsolete) eldoc-documentation-function).


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 03.06.2020 21:07, João Távora wrote:
> The equivalent in futures is just to say clients can set the value nil,
> or some other application-specific indication of "sorry, I failed".

They should call 'eldoc-future-set-error'.

One design point I'm not sure of, is whether the argument should be a 
string (coming from the error message), or a "proper" exception/error 
object, previous captured inside a condition-case.

We might make that choice just based on whether url-retrieve passes the 
same kind of data as, say, error-message-string expects. The same 
"proper" object, that is.

bug#41531: 28.0.50; proper Eldoc async support

[Andrii Kolomoiets]

João Távora <joaotavora@gmail.com> writes:

> Hello again gang,

Hi João,

> When applied, these improvements result in a substantial reduction of
> Eglot's documentation handling code.  Yes, Andrii, that means all our
> hand-crafted, hard work of doc handling functions is soon gone,
> including the awkward eglot-put-doc-in-help-buffer,
> eglot-auto-display-help-buffer and the *eglot-help* buffer.
>
> Feels sad but also good, because deleted code is good code.  And it's
> not in vain because your feedback and testing was fundamental here.  And
> the good news is that the logic problems about blinking and giving
> priority to some docs is gone.

I for one completely support moving documentation handling code to
eldoc.

I was planning to remove the eglot-put-doc-in-help-buffer variable in
the near future PR as well as the use of the eglot--message function for
the documentation display ;-)

However, after briefly using new Eldoc and Eglot I found some issues
that, I hope, we can fix:

1. Display only first line of the hover info.  Again :-)

2. The hover info is sometimes displayed right before the signature info
making the echo area to "blink".  I suppose this must be fixed on Eglot
side by not requesting both the hover and the signature infos at the
same time.

3. That IMO useless "...truncated, see *help* buffer" message is moved
to Eldoc.  Do we really need to show this message every time?  That one
last line can be used to show additional documentation.

Hadn't a chance to take a closer look at the code, so reporting those
issues is the most I can do for now.

Thanks!

bug#41531: 28.0.50; proper Eldoc async support

[Dmitry Gutov]

On 04.06.2020 19:20, Andrii Kolomoiets wrote:
> 3. That IMO useless "...truncated, see*help*  buffer" message is moved
> to Eldoc.  Do we really need to show this message every time?  That one
> last line can be used to show additional documentation.

The truncation can be indicated as ellipsis at the end of the (first) 
line. Maybe ellipsis in parentheses (...).

Whether to use multiple lines of not, seems like individual preference.

bug#41531: 28.0.50; proper Eldoc async support

[Andrii Kolomoiets]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04.06.2020 19:20, Andrii Kolomoiets wrote:
>> 3. That IMO useless "...truncated, see*help*  buffer" message is moved
>> to Eldoc.  Do we really need to show this message every time?  That one
>> last line can be used to show additional documentation.
>
> The truncation can be indicated as ellipsis at the end of the (first)
> line. Maybe ellipsis in parentheses (...).

Good idea.

> Whether to use multiple lines of not, seems like individual preference.

Absolutely.  That's why there are customizable variables so anyone can
tweak behavior to their likes.  Current version of Eglot pays attention
to the eldoc-echo-area-use-multiline-p variable and proposed one do
not.

bug#41531: 28.0.50; proper Eldoc async support

[João Távora]

[ Theodor and Fredrik, adding you since you were also interested in this
Eglot/Eldoc matter.  You can review the messages in the bug list if
you're interested:  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41531]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

> I was planning to remove the eglot-put-doc-in-help-buffer variable in
> the near future PR as well as the use of the eglot--message function for
> the documentation display ;-)

I'm guess I'm happy to have shot these plans into the depths of the
ocean ;-)

> However, after briefly using new Eldoc and Eglot I found some issues
> that, I hope, we can fix:
>
> 1. Display only first line of the hover info.  Again :-)

You should be able to do this with either

   (setq eldoc-echo-area-use-multiline-p 1)

or

   (setq eldoc-echo-area-use-multiline-p nil)

Did you try this? If so, what exactly didn't work for you when you did?
I'm sorry if you've already given me this information in the multiple
PR's you opened about this, but let's have it again.

> 2. The hover info is sometimes displayed right before the signature info
> making the echo area to "blink".  I suppose this must be fixed on Eglot
> side by not requesting both the hover and the signature infos at the
> same time.

Not something to be fixed in Eglot, definitely, it's not its fault or
responsibility: it just reports whatever it has.  I've fixed this in
Eldoc, in the last commit.  It only affected the "eager" strategy (which
should really be called the "enthusiast" strategy).  I'll post a commit
soon using better names for strategies, I'm thinking:

eldoc-documentation-function  -> eldoc-strategy         (with obsolete alias)
eldoc-documentation-functions -> eldoc-functions        (maybe)

eldoc-documentation-default  -> eldoc-patient
eldoc-documentation-compose  -> eldoc-compose-patiently
eldoc-documentation-eager    -> eldoc-enthusiast        (Eglot uses this)
  <doesn't exist yet>        -> eldoc-compose-eagerly   (Stefan mentioned this)

> 3. That IMO useless "...truncated, see *help* buffer" message is moved
> to Eldoc.  Do we really need to show this message every time?

I see.  Maybe not _every time_ but at least _once_, I'd say.  Once per
Eldoc session (but what is an Eldoc session)?  Once per x truncated
messages?  Customization variable? (I hate those, but maybe).

Or maybe never show it?

> That one last line can be used to show additional documentation.
> Hadn't a chance to take a closer look at the code, so reporting those
> issues is the most I can do for now.

Yes, and that's fine for now, though a second pair of eyes in the code
is certainly appreciated too.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

>>> You mean a single call could return first a function signature and
>>> a while later a docstring?
>> No, that's not what I mean.  Those should be two different members of
>> eldoc-documentation-functions (plural).
> Great.

Indeed, I can see 3 eldoc sources for emacs-lisp mode, in this order:

- function signature
- docstring
- special variable value

If emacs-lisp-mode one uses the default display strategy
(eldoc-documentation-default or some rename of that) and sets
eldoc-echo-area-use-multiline-p to 1, it should be fully backward
compatible to the current behaviour.

Alternatively, we can have emacs-lisp-mode keep out of
eldoc-echo-area-use-multiline-p and use 4 sources:

- function signature
- one-line docstring
- special variable value
- remaining paragraphs of docstring

( At some point, if we compose all of these bits of information in the
volatile *eldoc* buffer, it will start looking a lot like *Help* for C-h
o, There's some integration work to do there, but I'd rather not open
that can of worms just now. )

>>>> The callback strategy makes it easy because there are lambda lists of
>>>> all shapes and sizes.
>>> It's trivial to use a list to bring the number back down to 1, so it's
>>> not much of a difference.
>> Yes, I agree, but it's IMO easier to read (funcall cb :foo 42 :baz 23)
>> than (set-value fut (list :foo 42 :baz 23)).
>
> I find the difference largely irrelevant.  Much more important is
> what kinds of :foo and bar are allowed and what they do.

I agree.  Content is more important than style.  Of course style
matters, too.  Occasionally, it matters overwhelmingly.  But not
here. I'd say.  The promises-vs-callbacks discussion is a matter of
style.

>> In fact, a much better name for eldoc-documentation-function (singular)
>> is eldoc-documentation-strategy, not least because it relieves us from
>> this silly singular/plural confusion.
>
> Sounds very good.  Changing its name will make it possible to fix the
> current backward-incompatibility (which we'd fix by re-introducing a(n
> obsolete) eldoc-documentation-function).

Yes, that's the plan.

João

bug#41531: 28.0.50; proper Eldoc async support

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hello!

Thanks for pinging me -

I tried it, and it looks pretty cool. However, I noticed a few bugs.

(When using both the new eldoc and eglot):

Bug 1:      
1. Open a file
2. Start eglot
3. Hover something with "C-h ."
4. Switch to the new window that popped up.
5. C-x k (kill buffer)
6. Repeat step 3.
7. "invalid buffer" and eldoc is dead. 
8. Starting and stopping eldoc doesn't work   

This seems to happen since it uses the new buffer and updates it. When that buffer is deleted, it gets confused.


Bug 2:
1. (setq eldoc-echo-area-use-multiline-p nil)
2. eglot spits the whole thing on one line. This looks a bit weird since it uses both the signature and the documentation.

This may not be a bug, but right now IMO it looks a bit strange.

Bug 3:
1. "M-:"
2. Type "("
3. Minibuffer shows: eldoc-error: (wrong number of arguments (0 . 0) 1)

I think the first bug is the most problematic one though :)

However, I've started to take a liking to the no options set, with this version. I think I like the "truncate, press M-x ..." message more than the previous one.

All the best,

Theodor

João Távora <joaotavora@gmail.com> writes:

> [ Theodor and Fredrik, adding you since you were also interested in this
> Eglot/Eldoc matter.  You can review the messages in the bug list if
> you're interested:  https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41531]
>
> Andrii Kolomoiets <andreyk.mad@gmail.com> writes:
>
>> I was planning to remove the eglot-put-doc-in-help-buffer variable in
>> the near future PR as well as the use of the eglot--message function for
>> the documentation display ;-)
>
> I'm guess I'm happy to have shot these plans into the depths of the
> ocean ;-)
>
>> However, after briefly using new Eldoc and Eglot I found some issues
>> that, I hope, we can fix:
>>
>> 1. Display only first line of the hover info.  Again :-)
>
> You should be able to do this with either
>
>    (setq eldoc-echo-area-use-multiline-p 1)
>
> or
>
>    (setq eldoc-echo-area-use-multiline-p nil)
>
> Did you try this? If so, what exactly didn't work for you when you did?
> I'm sorry if you've already given me this information in the multiple
> PR's you opened about this, but let's have it again.
>
>> 2. The hover info is sometimes displayed right before the signature info
>> making the echo area to "blink".  I suppose this must be fixed on Eglot
>> side by not requesting both the hover and the signature infos at the
>> same time.
>
> Not something to be fixed in Eglot, definitely, it's not its fault or
> responsibility: it just reports whatever it has.  I've fixed this in
> Eldoc, in the last commit.  It only affected the "eager" strategy (which
> should really be called the "enthusiast" strategy).  I'll post a commit
> soon using better names for strategies, I'm thinking:
>
> eldoc-documentation-function  -> eldoc-strategy         (with obsolete alias)
> eldoc-documentation-functions -> eldoc-functions        (maybe)
>
> eldoc-documentation-default  -> eldoc-patient
> eldoc-documentation-compose  -> eldoc-compose-patiently
> eldoc-documentation-eager    -> eldoc-enthusiast        (Eglot uses this)
>   <doesn't exist yet>        -> eldoc-compose-eagerly   (Stefan mentioned this)
>
>> 3. That IMO useless "...truncated, see *help* buffer" message is moved
>> to Eldoc.  Do we really need to show this message every time?
>
> I see.  Maybe not _every time_ but at least _once_, I'd say.  Once per
> Eldoc session (but what is an Eldoc session)?  Once per x truncated
> messages?  Customization variable? (I hate those, but maybe).
>
> Or maybe never show it?
>
>> That one last line can be used to show additional documentation.
>> Hadn't a chance to take a closer look at the code, so reporting those
>> issues is the most I can do for now.
>
> Yes, and that's fine for now, though a second pair of eyes in the code
> is certainly appreciated too.
>
> João

bug#41531: 28.0.50; proper Eldoc async support

[João Távora]

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>> On 04.06.2020 19:20, Andrii Kolomoiets wrote:
>>> 3. That IMO useless "...truncated, see*help*  buffer" message is moved
>>> to Eldoc.  Do we really need to show this message every time?  That one
>>> last line can be used to show additional documentation.
>> The truncation can be indicated as ellipsis at the end of the (first)
>> line. Maybe ellipsis in parentheses (...).
>
> Good idea.

I think is is easily missed, and will not inform of the keybinding for
`eldoc-doc-buffer'.  But it's clearly better than nothing for the
strictly-one-line people.

>> Whether to use multiple lines of not, seems like individual preference.
>
> Absolutely.  That's why there are customizable variables so anyone can
> tweak behavior to their likes.  Current version of Eglot pays attention
> to the eldoc-echo-area-use-multiline-p variable and proposed one do
> not.

It does not because most of that variable (except for the very special
"truncate-sym-name-if-fit" value) is now handled in eldoc.el.  Modulo
bugs, of course, which I will endeavour to fix.  I also agree, of course
this is individual preference.

bug#41531: 28.0.50; proper Eldoc async support

[João Távora]

Theodor Thornhill <theothornhill@pm.me> writes:

> Bug 1:      
> 1. Open a file
> 2. Start eglot
> 3. Hover something with "C-h ."
> 4. Switch to the new window that popped up.
> 5. C-x k (kill buffer)
> 6. Repeat step 3.
> 7. "invalid buffer" and eldoc is dead. 
> 8. Starting and stopping eldoc doesn't work   
>
> This seems to happen since it uses the new buffer and updates it. When that buffer is deleted, it gets confused.

Thanks.  This is easy to fix.  It's a buffer-live-p call somewhere.

> Bug 2:
> 1. (setq eldoc-echo-area-use-multiline-p nil)
> 2. eglot spits the whole thing on one line. This looks a bit weird since it uses both the signature and the documentation.
>
> This may not be a bug, but right now IMO it looks a bit strange.

It was intended.  But I agree it's not good.  I will change it: instead
of joining all lines, then trucating, I will just truncate the first
line if needed.

> Bug 3:
> 1. "M-:"
> 2. Type "("
> 3. Minibuffer shows: eldoc-error: (wrong number of arguments (0 . 0) 1)

I couldn't reproduce this one.

> I think the first bug is the most problematic one though :)
>
> However, I've started to take a liking to the no options set, with
> this version. I think I like the "truncate, press M-x ..." message
> more than the previous one.

Yes, and if `eldoc-doc-buffer` is bound to something shorter, then it
will be even simpler to read.  I will eventually recommend we rebind
`C-h .` (display-local-help) to it.  But in the meantime, I think I will
make Eglot bind it to that key, as an exception to Eglot's
no-keybindings policy.

João

bug#41531: 28.0.50; proper Eldoc async support

[João Távora]

Theodor Thornhill <theothornhill@pm.me> writes:

> Bug 1:      
> 1. Open a file
> 2. Start eglot
> 3. Hover something with "C-h ."
> 4. Switch to the new window that popped up.
> 5. C-x k (kill buffer)
> 6. Repeat step 3.
> 7. "invalid buffer" and eldoc is dead. 
> 8. Starting and stopping eldoc doesn't work   
>
> This seems to happen since it uses the new buffer and updates it. When that buffer is deleted, it gets confused.

Thanks.  This is easy to fix.  It's a buffer-live-p call somewhere.

> Bug 2:
> 1. (setq eldoc-echo-area-use-multiline-p nil)
> 2. eglot spits the whole thing on one line. This looks a bit weird since it uses both the signature and the documentation.
>
> This may not be a bug, but right now IMO it looks a bit strange.

It was intended.  But I agree it's not good.  I will change it: instead
of joining all lines, then trucating, I will just truncate the first
line if needed.

> Bug 3:
> 1. "M-:"
> 2. Type "("
> 3. Minibuffer shows: eldoc-error: (wrong number of arguments (0 . 0) 1)

I couldn't reproduce this one.

> I think the first bug is the most problematic one though :)
>
> However, I've started to take a liking to the no options set, with
> this version. I think I like the "truncate, press M-x ..." message
> more than the previous one.

Yes, and if `eldoc-doc-buffer` is bound to something shorter, then it
will be even simpler to read.  I will eventually recommend we rebind
`C-h .` (display-local-help) to it.  But in the meantime, I think I will
make Eglot bind it to that key, as an exception to Eglot's
no-keybindings policy.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

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

Hi!

On 26.05.2020 17:53, Stefan Monnier wrote:
>> 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?

How about the attached file for a rough, but a largely feature complete 
first version of futures?

To remind from previous discussions, we wanted futures:

- To be cancelable, so that the issuer could abort their calculations, 
if they so desire.
- Force-able, meaning the consumer should be able to "realize" the 
future synchronously, with the future's creator being able to support 
their, most optimal, version of that logic. The default needs to be 
useful and reliable enough, though, that callers could use it in 99% of 
cases anyway.

The error callback probably wasn't mentioned, but it seems logical to 
have it anyway.

For the first two features, I also considered using cl-generic, but 
result might turn out to be clunkier, and we need overridability only 
for two of these functions. But suggestions welcome.

TBD:

- Probably rename to "promises" in the end.
- It would be nice to have a certain degree of compatibility with 
Christopher Wellons's emacs-aio, so that its promises could be accepted 
by code that expects "our" futures/promises. Not sure if we can do that 
without making the API more complex (aio-resolve's signature comes to 
mind), or if we adopt its approach, importing the package wholesale 
might make more sense.

[-- Attachment #2: future.el --]
[-- Type: text/x-emacs-lisp, Size: 2549 bytes --]

;;; future.el --- Futures, a concurrency primitive   -*- lexical-binding: t; -*-

;; Copyright (C) 2020  Dmitry Gutov

;; Author: Dmitry Gutov <dgutov@yandex.ru>
;; Keywords: lisp

;; This program is free software; you can redistribute it and/or modify
;; it under the terms of the GNU General Public License as published by
;; the Free Software Foundation, either version 3 of the License, or
;; (at your option) any later version.

;; This program is distributed in the hope that it will be useful,
;; but WITHOUT ANY WARRANTY; without even the implied warranty of
;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;; GNU General Public License for more details.

;; You should have received a copy of the GNU General Public License
;; along with this program.  If not, see <https://www.gnu.org/licenses/>.

;;; Commentary:

;;

;;; Code:

(require 'cl-lib)

(cl-defstruct (future
               (:conc-name future--)
               (:constructor future-make (&key force-fn
                                               cancel-fn)))
  "This structure represents a \"future\" value."
  value
  error
  (status 'pending)
  (force-fn #'future-force--apo)
  (cancel-fn #'ignore)
  callback
  errback)

(defun future-force (f)
  (funcall (future--force-fn f)))

(defun future-force--apo (f)
  (while (eq (future--status f) 'pending)
    (accept-process-output nil 0.05)))

(defun future-cancel (f)
  (when (eq (future--status f) 'pending)
    (setf (future--status f) 'canceled)
    (funcall (future--cancel-fn f))))

(defun future-set (f v)
  ;; FIXME: Probably shouldn't error on 'canceled'.
  (cl-assert (eq (future--status f) 'pending))
  (setf (future--value f) v
        (future--status f) 'success)
  (when (future--callback f)
    (funcall (future--callback f) v)))

(defun future-error (f e)
  ;; FIXME: Probably shouldn't error on 'canceled'.
  (cl-assert (eq (future--status f) 'pending))
  (setf (future--error f) e
        (future--status f) 'error)
  (when (future--errback f)
    (funcall (future--errback f) e)))

;; Or we can make these both lists of functions...
(defun future-set-callback (f c)
  (cl-assert (null (future--callback f)))
  (setf (future--callback f) c)
  (when (eq (future--status f) 'success)
    (funcall c (future--value f))))

;; ...and rename to -add-callback/-add-errback.
(defun future-set-errback (f c)
  (cl-assert (null (future--errback f)))
  (setf (future--errback f) c)
  (when (eq (future--status f) 'error)
    (funcall c (future--error f))))

(provide 'future)
;;; future.el ends here

bug#41531: 28.0.50; proper Eldoc async support

[Andrii Kolomoiets]

Sorry for the late reply.

João Távora <joaotavora@gmail.com> writes:

>> 1. Display only first line of the hover info.  Again :-)
>
> You should be able to do this with either
>
>    (setq eldoc-echo-area-use-multiline-p 1)
>
> or
>
>    (setq eldoc-echo-area-use-multiline-p nil)
>
> Did you try this? If so, what exactly didn't work for you when you
> did?

This way the signature info is truncated.  For the function with many
parameters the info of the last parameters is not visible. 

>> 2. The hover info is sometimes displayed right before the signature info
>> making the echo area to "blink".  I suppose this must be fixed on Eglot
>> side by not requesting both the hover and the signature infos at the
>> same time.
>
> Not something to be fixed in Eglot, definitely, it's not its fault or
> responsibility: it just reports whatever it has.

According to specification, server may send `triggerCharacters` in the
`SignatureHelpOptions`: the characters that trigger signature help
automatically.  Maybe Eglot should not always request the signature
info.
Though I like the current implementation.

>> 3. That IMO useless "...truncated, see *help* buffer" message is moved
>> to Eldoc.  Do we really need to show this message every time?
>
> I see.  Maybe not _every time_ but at least _once_, I'd say.  Once per
> Eldoc session (but what is an Eldoc session)?  Once per x truncated
> messages?  Customization variable? (I hate those, but maybe).
>
> Or maybe never show it?

Yep. Just like no additional message like "Press C-h v for the full
documentation" is shown hovering the variable in the `emacs-lisp-mode`.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

João Távora <joaotavora@gmail.com> writes:

> Hi Stefan, Dmitry, Andrii and maintainers,
>
> Moving the discussion that started in
> https://github.com/joaotavora/eglot/pull/459 to the bug tracker, and
> attaching the two patches that contain what I think is a decent
> short-term solution to the eldoc/async problems.

Hello again,

The work that started with this discussion is now mostly complete.  It
has been sitting in the scratch/eldoc-async branch of the Savannah repo
for a while, but I've been very busy and didn't have time to annouce it.

I've cleaned it up to to a four commit effort which.

    99fd115a8c Make more parts of Emacs use new Eldoc capabilities
    aaf5dfc71e * lisp/emacs-lisp/eldoc.el (Version): Bump to 1.1.0
    62dc1d4824 New M-x eldoc for on-demand and interactive documentation requests
    6c54414d5f Better handle asynchronous Eldoc sources

I had good feedback on and off-list about it.  It works with a version
of Eglot in its scratch/work-with-new-eldoc branch.

There is of course a lot that can still be fixed or added, especially in
the domain of controlling the outlets for documentation (which now are
restricted to the echo area, and poorly formatted buffer).

Anyway I think my efforts are ready to push to master.  I'll do so soon
unless someone raises a serious problem about it.

Dmitry has expressed his intent to make the new eldoc.el work with a new
futures/promises library.  He prototyped one of those libraries.  I have
nothing against that in the future.  However,

1. I don't have the resources to make the eldoc.el prototype work with
   Dmitry's or other libraries;

2. We should revisit the purpose and the details of that and other
   libraries in a separate discussion.  For now it's high time we
   advance the Eldoc libray;.

Thanks,
João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Tue, 30 Jun 2020 12:31:10 +0100
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, andreyk.mad@gmail.com,
>  Dmitry Gutov <dgutov@yandex.ru>
> 
> The work that started with this discussion is now mostly complete.  It
> has been sitting in the scratch/eldoc-async branch of the Savannah repo
> for a while, but I've been very busy and didn't have time to annouce it.

Thanks.

> Anyway I think my efforts are ready to push to master.  I'll do so soon
> unless someone raises a serious problem about it.

Not a serious problem, but some comments:

> -(defun eldoc-message (&optional string)
> +(make-obsolete
> + 'eldoc-message "use `eldoc-documentation-functions' instead." "1.1.0")

Isn't the version part of the obsolete message supposed to tell the
version of Emacs?

The change to the number of arguments of the functions in the
eldoc-documentation-functions hook is backward-incompatible, isn't it?
I see you've changed the relevant functions in our sources, but what
about 3rd-party packages?

Also, the doc string of this hook needs clarification regarding the
arguments: it first says that CALLBACK is the only mandatory argument
to the hook functions, but then, out of the blue, appear additional
arguments DOCSTRING and a list of key-value pairs.  Confusing.

The doc strings have some words in UK English spelling "(e.g.,
"honour"), please fix that.  Also, please make sure comments all start
with a capital letter, end with a period, and comprise full English
sentences.

The doc string of eldoc-documentation-compose doesn't say a word about
the function's argument.

In the doc string of eldoc-documentation-strategy, you use the phrase
"queries the special hook for all functions that produce doc strings"
to mean, AFAIU, that the specified functions in the hook-variable list
are called.  IMO, this wording could be confusing; suggest to use this
instead:

  `eldoc-documentation-compose': calls all the functions in the hook,
  and displays all of the resulting doc strings ...

This doc string doesn't explain the use of the timer, it explains the
reason for its existence.  It should also describe the use:

> +(defvar eldoc--enthusiasm-curbing-timer nil
> +  "Timer used by `eldoc-documentation-enthusiast' to avoid blinking.")

Last, but not least: the "async" part of the branch's name hints on
some advanced and extremely useful functionality that these changes
are supposed to allow, but I see no mention of that in NEWS and in the
manual bits.  What did I miss?

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

Thanks for the feedback, Eli.

I'll get to each point later on. But I'm surprised the NEWS entry I had
written got lost. In the git rebasing, maybe.

João

On Sat, Jul 4, 2020, 08:45 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Date: Tue, 30 Jun 2020 12:31:10 +0100
> > Cc: Stefan Monnier <monnier@iro.umontreal.ca>, andreyk.mad@gmail.com,
> >  Dmitry Gutov <dgutov@yandex.ru>
> >
> > The work that started with this discussion is now mostly complete.  It
> > has been sitting in the scratch/eldoc-async branch of the Savannah repo
> > for a while, but I've been very busy and didn't have time to annouce it.
>
> Thanks.
>
> > Anyway I think my efforts are ready to push to master.  I'll do so soon
> > unless someone raises a serious problem about it.
>
> Not a serious problem, but some comments:
>
> > -(defun eldoc-message (&optional string)
> > +(make-obsolete
> > + 'eldoc-message "use `eldoc-documentation-functions' instead." "1.1.0")
>
> Isn't the version part of the obsolete message supposed to tell the
> version of Emacs?
>
> The change to the number of arguments of the functions in the
> eldoc-documentation-functions hook is backward-incompatible, isn't it?
> I see you've changed the relevant functions in our sources, but what
> about 3rd-party packages?
>
> Also, the doc string of this hook needs clarification regarding the
> arguments: it first says that CALLBACK is the only mandatory argument
> to the hook functions, but then, out of the blue, appear additional
> arguments DOCSTRING and a list of key-value pairs.  Confusing.
>
> The doc strings have some words in UK English spelling "(e.g.,
> "honour"), please fix that.  Also, please make sure comments all start
> with a capital letter, end with a period, and comprise full English
> sentences.
>
> The doc string of eldoc-documentation-compose doesn't say a word about
> the function's argument.
>
> In the doc string of eldoc-documentation-strategy, you use the phrase
> "queries the special hook for all functions that produce doc strings"
> to mean, AFAIU, that the specified functions in the hook-variable list
> are called.  IMO, this wording could be confusing; suggest to use this
> instead:
>
>   `eldoc-documentation-compose': calls all the functions in the hook,
>   and displays all of the resulting doc strings ...
>
> This doc string doesn't explain the use of the timer, it explains the
> reason for its existence.  It should also describe the use:
>
> > +(defvar eldoc--enthusiasm-curbing-timer nil
> > +  "Timer used by `eldoc-documentation-enthusiast' to avoid blinking.")
>
> Last, but not least: the "async" part of the branch's name hints on
> some advanced and extremely useful functionality that these changes
> are supposed to allow, but I see no mention of that in NEWS and in the
> manual bits.  What did I miss?
>

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

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Sat, 4 Jul 2020 10:21:43 +0100
> Cc: 41531@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>, andreyk.mad@gmail.com, 
> 	dgutov@yandex.ru
> 
> I'm surprised the NEWS entry I had written got lost. In the git rebasing,
> maybe.

There is a NEWS entry in the branch, but it only says this:

  -*** 'eldoc-documentation-function' is now a user option.
  -Modes should use the new hook instead of this user option to register
  -their backends.
  +*** New user option 'eldoc-documentation-strategy'
  +The built-in choices available for this user option let users compose
  +the results of 'eldoc-documentation-functions' in various ways.  The
  +user option replaces 'eldoc-documentation-function', which is now
  +obsolete.

I don't see how by reading this anyone could understand that the
change allows significant new functionality.  maybe you wrote more
than that, and it did get lost somehow?

(FTR, I did "git diff ...origin/scratch/eldoc-async" to see the
changes on the branch.)

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

Ah, ok I see what you mean.

I didn't feel it was necessary to enter into details since it is backwards
compatible. But I can add some things to mention that async is now
supported in a "first class" sense.

João

On Sat, Jul 4, 2020, 10:31 Eli Zaretskii <eliz@gnu.org> wrote:

> > From: João Távora <joaotavora@gmail.com>
> > Date: Sat, 4 Jul 2020 10:21:43 +0100
> > Cc: 41531@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
> andreyk.mad@gmail.com,
> >       dgutov@yandex.ru
> >
> > I'm surprised the NEWS entry I had written got lost. In the git rebasing,
> > maybe.
>
> There is a NEWS entry in the branch, but it only says this:
>
>   -*** 'eldoc-documentation-function' is now a user option.
>   -Modes should use the new hook instead of this user option to register
>   -their backends.
>   +*** New user option 'eldoc-documentation-strategy'
>   +The built-in choices available for this user option let users compose
>   +the results of 'eldoc-documentation-functions' in various ways.  The
>   +user option replaces 'eldoc-documentation-function', which is now
>   +obsolete.
>
> I don't see how by reading this anyone could understand that the
> change allows significant new functionality.  maybe you wrote more
> than that, and it did get lost somehow?
>
> (FTR, I did "git diff ...origin/scratch/eldoc-async" to see the
> changes on the branch.)
>

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

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Date: Sat, 4 Jul 2020 10:37:50 +0100
> Cc: 41531@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>, andreyk.mad@gmail.com, 
> 	dgutov@yandex.ru
> 
> I can add some things to mention that async is now supported in a
> "first class" sense.

Please do, I think it's important.  You don't have to tell too many
details in NEWS about that, just mention it, and say that the details
are in the ELisp manual and the doc strings of this-and-that.

TIA

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 30.06.2020 14:31, João Távora wrote:
> Dmitry has expressed his intent to make the new eldoc.el work with a new
> futures/promises library.  He prototyped one of those libraries.  I have
> nothing against that in the future.  However,
> 
> 1. I don't have the resources to make the eldoc.el prototype work with
>     Dmitry's or other libraries;
> 
> 2. We should revisit the purpose and the details of that and other
>     libraries in a separate discussion.  For now it's high time we
>     advance the Eldoc libray;.

Unsurprisingly, I object to this approach. We should have better async 
support in multiple Emacs facilities, and that means standardizing on 
some particular async primitives and functions that can manipulate them. 
There is no need to release support for ad-hoc async values (you didn't 
like mine, so it's only fair play that I object to yours) when we can 
transition to full futures right away.

I'll get into a little more detail in the more full review, tonight or 
tomorrow, but for now my impression is that, in the absence of such 
standard library and async manipulation functions, you decided to 
implement them specially in Eldoc. Even though most of that logic should 
be extracted to general purpose code that manipulates async objects 
(futures/promises or the like). And I wonder how the desire not to have 
such logic in Eglot has influenced your total vision of the API.

Because with futures in standard library, it could look fairly 
different, and could need fewer changes compared to its current shape.

Regarding #1, it should be trivial to reimplement on top of futures. I 
could do this myself, as long as everybody is fine with the proposed 
shape of futures on standardize on.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> Anyway I think my efforts are ready to push to master.  I'll do so soon
>> unless someone raises a serious problem about it.
>
> Not a serious problem, but some comments:
>
>> -(defun eldoc-message (&optional string)
>> +(make-obsolete
>> + 'eldoc-message "use `eldoc-documentation-functions' instead." "1.1.0")
>
> Isn't the version part of the obsolete message supposed to tell the
> version of Emacs?

Yes, but eldoc.el is its own versioned piece of software now, because it
is a GNU Elpa :core package.  So I opted to use that versioning instead.
Maybe I shouldn't have?  Don't know if there's prior art for this
situation, but I'm reasonably confident I personally have taken this
decision before.

> The change to the number of arguments of the functions in the
> eldoc-documentation-functions hook is backward-incompatible, isn't it?
> I see you've changed the relevant functions in our sources, but what
> about 3rd-party packages?

I've checked and there were none.  The eldoc-documentation-functions
variable came to be in Emacs master: it's not in Emacs 27.  I myself
"released" eldoc.el to GNU Elpa almost 2 months ago (see
9ebf51999ce58cccc2a228fd07a18c7b472dd3fd).  I had intended to streamline
that release together with these async-related developments, but reality
(and mostly Dmitry :-) intervened.

So indeed there is the danger that, in the intervening 2 months, some
3rd party package depending on GNU Elpa's Eldoc, started using the
argless eldoc-documentation-functions, and will be surprised by this
backwards-incompatible change.  However, I think the changes of that are
unlikely.  I searched Github and Google for such references and couldn't
find any.

So I think it's safe.  (But even if it weren't, I'd still argue for the
change + fallout ).

> Also, the doc string of this hook needs clarification regarding the
> arguments: it first says that CALLBACK is the only mandatory argument
> to the hook functions, but then, out of the blue, appear additional
> arguments DOCSTRING and a list of key-value pairs.  Confusing.

Understood.

> The doc strings have some words in UK English spelling "(e.g.,
> "honour"), please fix that.  Also, please make sure comments all start
> with a capital letter, end with a period, and comprise full English
> sentences.

OK.

> The doc string of eldoc-documentation-compose doesn't say a word about
> the function's argument.

That function isn't meant to be called by users, and the EAGERLYP is a
code-saving trick.  But of course I should document it.

> In the doc string of eldoc-documentation-strategy, you use the phrase
> "queries the special hook for all functions that produce doc strings"
> to mean, AFAIU, that the specified functions in the hook-variable list
> are called.  IMO, this wording could be confusing; suggest to use this
> instead:
>
>   `eldoc-documentation-compose': calls all the functions in the hook,
>   and displays all of the resulting doc strings ...

This of course, assumes that people know that "functions in the hook"
aren't like "functions in a list".  The symbol `t` may be in "in the
hook".

But it's better for practical purposed, I admit.  Because "result" is
often conflated with "return value", and that's only _one_ of the ways
the functions can produce doc strings, I'd modify that to:

   `eldoc-documentation-compose': calls all the functions in the hook,
   and displays the doc strings that they produce...
>
> This doc string doesn't explain the use of the timer, it explains the
> reason for its existence.  It should also describe the use:
>
>> +(defvar eldoc--enthusiasm-curbing-timer nil
>> +  "Timer used by `eldoc-documentation-enthusiast' to avoid
>blinking.")

OK.

> Last, but not least: the "async" part of the branch's name hints on
> some advanced and extremely useful functionality that these changes
> are supposed to allow, but I see no mention of that in NEWS and in the
> manual bits.  What did I miss?

Not much, we talked about this.  Will change NEWS according to what we
discussed.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 30.06.2020 14:31, João Távora wrote:
>> Dmitry has expressed his intent to make the new eldoc.el work with a new
>> futures/promises library.  He prototyped one of those libraries.  I have
>> nothing against that in the future.  However,
>> 1. I don't have the resources to make the eldoc.el prototype work
>> with
>>     Dmitry's or other libraries;
>> 2. We should revisit the purpose and the details of that and other
>>     libraries in a separate discussion.  For now it's high time we
>>     advance the Eldoc libray;.
>
> Unsurprisingly, I object to this approach. We should have better async
> support in multiple Emacs facilities, and that means standardizing on 
> some particular async primitives and functions that can manipulate
> them. There is no need to release support for ad-hoc async values (you
> didn't like mine, so it's only fair play that I object to yours) when
> we can transition to full futures right away.

I haven't seen a consistent plan "for transitioning to futures".  All
I've seen is (complicated, IMO) ideas on how to avoid the
callback-calling style by hiding the callback in an object.  I've seen
little argument or discussion of what futures are supposed to do or fix
or improve.  And I've seen no feedback from the author of what seems to
be the most promising futures library, aio.el which uses the
generator.el library, to provide, presumably, more elegant
"language-level" futures (i.e. futures that aren't only hiding the
callback in some other object).

Despite that, I've stated here repeatedly, tiringly: In principle, I
don't object to futures at all.  I just think it has to be a
thought-through change.  Propose your library to emacs-devel and let's
iterate it there.

> I'll get into a little more detail in the more full review, tonight or
> tomorrow, but for now my impression is that, in the absence of such 
> standard library and async manipulation functions, you decided to
> implement them specially in Eldoc.

Of course, and that's what engineering is about.  For the most trivial
of changes X there is _always_ a refactoring R that will make
implementing X and a possible Y and Z much simpler, more elegant, more
"meta".  Knowing where to stop is what this game is about.  In this
case, there is only a vision what Y and Z might be, and a foggier vision
of how bad/good design decisions in R might affect them.

So I fixed the real, actual problem in Eldoc using the async paradigm
that is widely used in Emacs and most widely understood by programmers
of all (most?) trades: callbacks.

And, again, for the nth time, there is nothing in my code that prevents
your -- or someone else's -- "futures" library to be the nec plus ultra
of async in Emacs.  But, in the meantime, I won't let you make these
Eldoc changes hostage to your predilection for futures.  It's quite a
legitimate inclination, of course, it musn't be needlessly put it in the
way of other's work.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 04.07.2020 14:00, João Távora wrote:
> I had intended to streamline
> that release together with these async-related developments, but reality
> (and mostly Dmitry:-)  intervened.

To be fair to yours truly, though, AFAIK we are the only two people in 
this discussion who maintain third-party eldoc clients, or work on 
integrated development experience in general.

That's a pretty sad state of affairs, BTW.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 04.07.2020 14:48, João Távora wrote:

>> Unsurprisingly, I object to this approach. We should have better async
>> support in multiple Emacs facilities, and that means standardizing on
>> some particular async primitives and functions that can manipulate
>> them. There is no need to release support for ad-hoc async values (you
>> didn't like mine, so it's only fair play that I object to yours) when
>> we can transition to full futures right away.
> 
> I haven't seen a consistent plan "for transitioning to futures".

Do you want to see the patch based on your code? It's the only 
"transition" I meant here.

If you have other questions, please ask.

> All
> I've seen is (complicated, IMO) ideas on how to avoid the
> callback-calling style by hiding the callback in an object.

Hiding is absolutely not the point.

> I've seen
> little argument or discussion of what futures are supposed to do or fix
> or improve.

I think I have described at length the very simple idea of what it is 
supposed to improve: provide a standard primitive and a library of 
composition/manipulation functions that would make these operations 
simpler. Which can be used in Emacs core, and in clients of the various 
facilities and expose future-based interfaces.

> And I've seen no feedback from the author of what seems to
> be the most promising futures library, aio.el which uses the
> generator.el library, to provide, presumably, more elegant
> "language-level" futures (i.e. futures that aren't only hiding the
> callback in some other object).

I'm very curious to know Christopher's opinion myself. Based on my 
experience, this will result in a much longer, protracted discussion, if 
it will result in one at all.

You seem to be in a hurry to get this in for some reason, but I'm fine 
with waiting a little more, if we get a better result.

> Despite that, I've stated here repeatedly, tiringly: In principle, I
> don't object to futures at all.  I just think it has to be a
> thought-through change.  Propose your library to emacs-devel and let's
> iterate it there.

If you think emacs-devel will bring more feedback, no problem.

>> I'll get into a little more detail in the more full review, tonight or
>> tomorrow, but for now my impression is that, in the absence of such
>> standard library and async manipulation functions, you decided to
>> implement them specially in Eldoc.
> 
> Of course, and that's what engineering is about.  For the most trivial
> of changes X there is _always_ a refactoring R that will make
> implementing X and a possible Y and Z much simpler, more elegant, more
> "meta".  Knowing where to stop is what this game is about.  In this
> case, there is only a vision what Y and Z might be, and a foggier vision
> of how bad/good design decisions in R might affect them.

The thing about refactoring, is it doesn't change observable behavior. 
Refactoring would keep the stable interface (and e.g. reuse future-based 
utility functions under the covers).

Whereas the improvement I would like to see here, would change it. So 
it's not something that could be simply moved to backlog.

And as I intent to explain later, I believe you will need 
future-manipulating functions inside eldoc client code anyway, for best 
user experience. So we might as well bite the bullet and introduce 
futures at the API level.

> So I fixed the real, actual problem in Eldoc using the async paradigm
> that is widely used in Emacs and most widely understood by programmers
> of all (most?) trades: callbacks.

Please recall how you rejected my proposal along the same lines.

And it's fine. We can do better.

> And, again, for the nth time, there is nothing in my code that prevents
> your -- or someone else's -- "futures" library to be the nec plus ultra
> of async in Emacs.  But, in the meantime, I won't let you make these
> Eldoc changes hostage to your predilection for futures.

"won't let you" is not exactly how things work here. We usually try hard 
to reach a consensus.

In any case, I have considered this for some time, and my arguments for 
"let's work on this some more" won't be limited to futures-vs-callbacks.

> It's quite a
> legitimate inclination, of course, it musn't be needlessly put it in the
> way of other's work.

I agree that there are improvements to be made in documentation-related 
code (whether it's in Eldoc, or not), and I also think that you might 
not be going far enough in some respects.

But the problem you seem to be urgent to fix (having eldoc-message 
called from client code) is not so terrible that we can't wait until the 
future-related discussion at least has had a chance to happen.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

Sorry,

On 05.07.2020 00:27, Dmitry Gutov wrote:
>> And I've seen no feedback from the author of what seems to
>> be the most promising futures library, aio.el which uses the
>> generator.el library, to provide, presumably, more elegant
>> "language-level" futures (i.e. futures that aren't only hiding the
>> callback in some other object).
> 
> I'm very curious to know Christopher's opinion myself. Based on my 
                                                          ^ But
> experience, this   will result in a much longer, protracted discussion, if 
                    ^ [moving it to emacs-devel]
> it will result in one at all.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> [that is the] "transition" I meant here.
>
> If you have other questions, please ask.

If it wasn't clear, my suggestion is that you send your earlier
dimtry-futures.el (as I am temporarily dubbing it, for clarity) to
emacs-devel, along with a rationale for why it is needed and where it
can be useful, not only in eldoc.el but in other places in Emacs that
use callbacks like like url.el, flymake.el, jsonrpc.rl, timers,
completion, processes, etc.  You may want to include a short comparison
to Christopher Wellon's aio.el, if you have studied it.

> I think I have described at length the very simple idea of what it is
> supposed to improve: provide a standard primitive and a library of 
> composition/manipulation functions that would make these operations
> simpler. Which can be used in Emacs core, and in clients of the
> various facilities and expose future-based interfaces.

Maybe I missed something, but I don't consider our discussion an
at-length discussion.  It might have been verbose, but it wasn't
in-depth at all.  And certainly it didn't involve anywhere enough people
for such an ambitious feature.  I have some comments to your last
proposed dmitry-futures.el, but this bug report is not the place to make
them.

>> And I've seen no feedback from the author of what seems to
>> be the most promising futures library, aio.el which uses the
>> generator.el library, to provide, presumably, more elegant
>> "language-level" futures (i.e. futures that aren't only hiding the
>> callback in some other object).
>
> I'm very curious to know Christopher's opinion myself. Based on my
> experience, this will result in a much longer, protracted discussion,
> if it will result in one at all.
>
> You seem to be in a hurry to get this in for some reason, but I'm fine
> with waiting a little more, if we get a better result.

I want to fix longstanding problems in Eglot.  I have limited resources,
mainly time.  It is you who seem intent on not letting this go in, as if
you won't be able to prove that "better result" after it does.  This is
absurd: if you do show it to be better, then we should migrate eldoc.el
etc to futures.  ...as I've said a trillion times already at this point.

> Please recall how you rejected my proposal along the same lines.

I don't remember your proposal fixing anything in eldoc.el, you merely
suggested I experimented with a technique.  Which I actually did, and I
didn't see any advantage in it.

> And it's fine. We can do better.

I'm sorry you don't like my work.  I had good feedback about it.  If
_you_ can do better, I'm not stopping you.

>> And, again, for the nth time, there is nothing in my code that prevents
>> your -- or someone else's -- "futures" library to be the nec plus ultra
>> of async in Emacs.  But, in the meantime, I won't let you make these
>> Eldoc changes hostage to your predilection for futures.
>
> "won't let you" is not exactly how things work here. We usually try
> hard to reach a consensus.

In my book, there's nothing legitimate in vetoing a bugfix because of
some yearning for some particular abstraction or programming paradigm.
That's as absurd as demanding Emacs be rewritten in my paradigm/language
of choice before anyone else commits anything.  Especially when, for the
quadrillionth time, there is nothing stopping you from making your case
for futures after the bug is fixed.

> In any case, I have considered this for some time, and my arguments
> for "let's work on this some more" won't be limited to
> futures-vs-callbacks.

In that case, if indeed they are not futures-vs-callbacks, are they
_serious_ objections?  Can they not be fixed afterwards?  I would have
expected some manifestation of them already, but you seem to waste all
your energy on your proclivity for futures.  But very well then, let's
have those non-futures objections in this bug report, the sooner the
better.

>> legitimate inclination, of course, it musn't be needlessly put it in the
>> way of other's work.
>
> I agree that there are improvements to be made in
> documentation-related code (whether it's in Eldoc, or not), and I also
> think that you might not be going far enough in some respects.
>
> But the problem you seem to be urgent to fix (having eldoc-message
> called from client code) is not so terrible that we can't wait until
> the future-related discussion at least has had a chance to happen.

I've explained repeatedly: this is holding up multiple things in Eglot.
I want Eglot to serve diagnostic messages, and various kinds of
automatically gathered information about the "things at point" all
through eldoc.el.  Then move on to better stuff, of which there is a lot
in LSP, as you well know.  Also, this fixes SLY and SLIME too, both
hacking their way through eldoc.el.  Possibly the same for CIDER and
other such extensions.  This also opens up eldoc.el in non-async
situations as well, such as elisp-mode.el.  Just read the patch.

Look, Dmitry, think clearly: I'm going to fix eldoc.el and you can have
your futures.el discussion when you want, a discussion where don't know
if I'll be able to participate, but that shouldn't prevent it from
happening.  After that discussion, whatever conclusion you, possibly me,
and other interested parties arrive at, as long as you/we keep Eglot and
SLY functional, or suggest improvements to them, I'll abide.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04.07.2020 14:00, João Távora wrote:
>> I had intended to streamline
>> that release together with these async-related developments, but reality
>> (and mostly Dmitry:-)  intervened.
>
> To be fair to yours truly, though, AFAIK we are the only two people in
> this discussion who maintain third-party eldoc clients, or work on 
> integrated development experience in general.
>
> That's a pretty sad state of affairs, BTW.

I actually have multiple people working with me in Eglot, potentially
eager to see how things in Emacs core can be solved properly and
effectively, and being utterly disappointed by your opposition in this
eldoc.el case.  That's the sad state of affairs IMO.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Eli Zaretskii <eliz@gnu.org> writes:

>> -(defun eldoc-message (&optional string)
>> +(make-obsolete
>> + 'eldoc-message "use `eldoc-documentation-functions' instead." "1.1.0")
>
> Isn't the version part of the obsolete message supposed to tell the
> version of Emacs?

Stuck with "1.1.0", because of the GNU ELPA criterion.  But maybe the
string could be "1.1.0-elpa" or something like that.

> The change to the number of arguments of the functions in the
> eldoc-documentation-functions hook is backward-incompatible, isn't it?
> I see you've changed the relevant functions in our sources, but what
> about 3rd-party packages?

Answered this already.

> Also, the doc string of this hook needs clarification regarding the
> arguments: it first says that CALLBACK is the only mandatory argument
> to the hook functions, but then, out of the blue, appear additional
> arguments DOCSTRING and a list of key-value pairs.  Confusing.

Changed that paragraph to

   To call the CALLBACK function, the hook function must pass it an
   obligatory argument DOCSTRING, a string containing the
   documentation, followed by an optional list of keyword-value
   pairs of the form (:KEY VALUE :KEY2 VALUE2...).  KEY can be:

The situation is very similar to Flymake's flymake-diagnostic-functions.
If you still find this docstring confusing, maybe I should copy that
docstring's structure.

> The doc strings have some words in UK English spelling "(e.g.,
> "honour"), please fix that.  Also, please make sure comments all start
> with a capital letter, end with a period, and comprise full English
> sentences.

I fixed all I could find, but is this a a hard rule?  I like to break it
once in a while:

    ;; We have to foo bar separately ...
    (foo bar)
    ;; ... because the bazness might be too quuxy.
    (if (quux) (foo baz))

I ended up not doing that this time, though.

> The doc string of eldoc-documentation-compose doesn't say a word about
> the function's argument.

I fixed that by creating a helper eldoc--documentation-compose-1 that
both eldoc-documentation-compose and eldoc-documentation-compose-eagerly
call.  I describe the arg in the helper.

> This doc string doesn't explain the use of the timer, it explains the
> reason for its existence.  It should also describe the use:

See the new version below.  I think it is sufficient.  Be aware this is
an internal variable, we don't even let the user customize the time (we
could though, but I think it's overcomplicating, for now).

    (defvar eldoc--enthusiasm-curbing-timer nil
      "Timer used by the `eldoc-documentation-enthusiast' strategy.
    When a doc string is encountered, it must endure a certain amount
    of time unchallenged until it is displayed to the user.  This
    prevents blinking if a lower priority docstring comes in shortly
    before a higher priority one.")

> Last, but not least: the "async" part of the branch's name hints on
> some advanced and extremely useful functionality that these changes
> are supposed to allow, but I see no mention of that in NEWS and in the
> manual bits.  What did I miss?

I reworded it, caught a bug, and mentioned
eldoc-echo-area-use-multiline-p.  Let me know it if you're missing
anything.

João

PS: Actually a proper section for Eldoc in the Emacs manual is probably
missing, but I don't have time to work on it straight away.  Or at least
I think the fix should go in first.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Eli Zaretskii]

> From: João Távora <joaotavora@gmail.com>
> Cc: 41531@debbugs.gnu.org,  monnier@iro.umontreal.ca,
>   andreyk.mad@gmail.com,  dgutov@yandex.ru
> Date: Sun, 05 Jul 2020 13:03:48 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> -(defun eldoc-message (&optional string)
> >> +(make-obsolete
> >> + 'eldoc-message "use `eldoc-documentation-functions' instead." "1.1.0")
> >
> > Isn't the version part of the obsolete message supposed to tell the
> > version of Emacs?
> 
> Stuck with "1.1.0", because of the GNU ELPA criterion.  But maybe the
> string could be "1.1.0-elpa" or something like that.

AFAIK, the obsolescence message we display says "since N.M.X", and the
intent is always to an Emacs version, right?  How can the user
understand that in this case you are talking about the version of some
other package?

> > The doc strings have some words in UK English spelling "(e.g.,
> > "honour"), please fix that.  Also, please make sure comments all start
> > with a capital letter, end with a period, and comprise full English
> > sentences.
> 
> I fixed all I could find, but is this a a hard rule?  I like to break it
> once in a while:
> 
>     ;; We have to foo bar separately ...
>     (foo bar)
>     ;; ... because the bazness might be too quuxy.
>     (if (quux) (foo baz))

This is fine.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

>>> -(defun eldoc-message (&optional string)
>>> +(make-obsolete
>>> + 'eldoc-message "use `eldoc-documentation-functions' instead." "1.1.0")
>>
>> Isn't the version part of the obsolete message supposed to tell the
>> version of Emacs?
>
> Stuck with "1.1.0", because of the GNU ELPA criterion.  But maybe the
> string could be "1.1.0-elpa" or something like that.

I recommend "eldoc-1.1.0" to clarify which version number we're talking about.


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 05.07.2020 02:12, João Távora wrote:
> I actually have multiple people working with me in Eglot, potentially
> eager to see how things in Emacs core can be solved properly and
> effectively, and being utterly disappointed by your opposition in this
> eldoc.el case.  That's the sad state of affairs IMO.

A part of "solving things properly" is listening to feedback.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 05.07.2020 02:07, João Távora wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> [that is the] "transition" I meant here.
>>
>> If you have other questions, please ask.

I meant questions about futures as pertaining to eldoc, of course. I 
have other uses in mind, but redoing existing features is a less urgent 
endeavor. E.g., Flymake is stable, and I don't have any particular bugs 
in mind that need solving. So I'd be fine with keeping it as is. Unless 
someone wanted to take advantage of extra features that futures provide.

> If it wasn't clear, my suggestion is that you send your earlier
> dimtry-futures.el (as I am temporarily dubbing it, for clarity) to
> emacs-devel, along with a rationale for why it is needed and where it
> can be useful, not only in eldoc.el but in other places in Emacs that
> use callbacks like like url.el, flymake.el, jsonrpc.rl, timers,
> completion, processes, etc.  You may want to include a short comparison
> to Christopher Wellon's aio.el, if you have studied it.

Yes, will do.

But note that when I argue for futures, it's for basically any 
implementation that has a container for such values, and a set of common 
functions for manipulating them. Christopher's version would serve that 
just fine (with copyright assignment signed). Which exact version to 
choose, however, can indeed be discussed separately.

>> I think I have described at length the very simple idea of what it is
>> supposed to improve: provide a standard primitive and a library of
>> composition/manipulation functions that would make these operations
>> simpler. Which can be used in Emacs core, and in clients of the
>> various facilities and expose future-based interfaces.
> 
> Maybe I missed something, but I don't consider our discussion an
> at-length discussion.

It also wasn't a discussion really. Too one-sided.

>> You seem to be in a hurry to get this in for some reason, but I'm fine
>> with waiting a little more, if we get a better result.
> 
> I want to fix longstanding problems in Eglot.  I have limited resources,
> mainly time.  It is you who seem intent on not letting this go in, as if
> you won't be able to prove that "better result" after it does.  This is
> absurd: if you do show it to be better, then we should migrate eldoc.el
> etc to futures.  ...as I've said a trillion times already at this point.

Please don't make it sound as if I'm the only one here having a strong 
opinion about proposed code. You disregarded the simple solution in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41531#8, and then went on 
with your own vision of of "simple" function based async, full on with 
extra features and additional incompatible API changes.

>> Please recall how you rejected my proposal along the same lines.
> 
> I don't remember your proposal fixing anything in eldoc.el, you merely
> suggested I experimented with a technique.  Which I actually did, and I
> didn't see any advantage in it.

It certainly did away with clients having to call eldoc-message. And if 
shorter code and better backward compatibility are not advantages, we 
might disagree hard on the meaning of the word.

>> But the problem you seem to be urgent to fix (having eldoc-message
>> called from client code) is not so terrible that we can't wait until
>> the future-related discussion at least has had a chance to happen.
> 
> I've explained repeatedly: this is holding up multiple things in Eglot.
> I want Eglot to serve diagnostic messages, and various kinds of
> automatically gathered information about the "things at point" all
> through eldoc.el.  Then move on to better stuff, of which there is a lot
> in LSP, as you well know.  Also, this fixes SLY and SLIME too, both
> hacking their way through eldoc.el.  Possibly the same for CIDER and
> other such extensions.  This also opens up eldoc.el in non-async
> situations as well, such as elisp-mode.el.  Just read the patch.

Aside: given that this discussion has user interface in mind, it needs 
more consideration of actual user experiences we'd want to allow. Ones 
not provided by eldoc itself as well. For instance, I think we sooner or 
later should have a callsig floating popup (in the vein of MS's editors) 
as an alternative to showing the signature in the echo area only.

 > But very well then, let's
 > have those non-futures objections in this bug report, the sooner the
 > better.

To be sure, they will also contain the "how we could do this better" 
kind of arguments.

Let's start with the basics:

The new API is incompatible with the previous one, it requires changing 
a lot of functions (though admittedly in a minor way). Which is easy to 
miss, as evidenced by describe-char-eldoc which still doesn't accept any 
arguments. Whereas we could limit ourselves to the change which would 
only make the clients use the different hook (or keep using the old one, 
for compatibility with older Flymake/Emacs versions).

The choice to require a return of a non-nil value if the callback is 
going to be used is also kinda odd (+1 control flow to test). What's the 
problem with using the callback synchronously if the doc is available 
right away?

The decision to double down on having different 
eldoc-documentation-strategy values seems odd to me in several respects.

First of all, if I understand the main motivation behind it, it's to 
delegate the implementation of asynchronous primitives to Eldoc. We 
don't want to bother with that in Eglot, etc. But you mentioned "a type 
of docstring" to be returned in the discussion (and the callbacks have 
the plist argument as a future proofing). If the type is a buffer, its 
contents might as well be created from several async calls, which would 
require the same async primitives (though rather in general purpose 
form) available on the client anyway. Though it doesn't seem to be 
necessary for LSP in common operations, it's unlikely to be the only 
such protocol we'd ever want to support.

The strategies themselves:

- eldoc-documentation-enthusiast: What's the difference compared to the 
'default' one? Sacrificing CPU load for lower latency? It's an odd 
choice to force the user to make. The only people who can make an ideal 
decision regarding this are probably the authors of 
eldoc-documentation-functions, but it wouldn't help if 
eldoc-documentation-functions has functions coming from different 
authors, facilities, etc.

- eldoc-documentation-compose: Okay, this is kinda interesting (though 
not essential), and the implementation is not working as well as I'd 
hoped it would. It makes the echo area blink up and down as I move the 
cursor around. There's no expected truncation of individual docstrings 
when they don't fit in the window's width. And I don't understand what 
you expect it to do if several docstrings are multiline, and as an 
aggregate they don't fit the target height. I think the only reasonably 
predictable behavior would be to truncate each of them to one line, always.

- eldoc-documentation-compose-eagerly: Ooh, now I think I see why 
documentation functions have to return these non-nil, non-string values. 
Is it that this particular strategy wouldn't have to wait too long for 
documentation functions that never intended to call back? Returning 
futures instead (when async is needed) would provide this kind of 
guarantee without the seeming duplication of signals.

On a related note, I believe some facilities might want to use only 
certain "kinds" of documentation functions (as indicated by the plist 
arguments). If the plist is only provided together with the response, 
there is no way to avoid unnecessary computations (e.g. making HTTP 
requests that return some other kinds of values). If the plist was 
returned together with a future value, however, it would be easy to do, 
as long as we document that the futures should start the computation 
until a callback is provided (if possible, at least).

And in a different high-level argument: you stated that you intend to 
have Eglot use a non-default value of eldoc-documentation-strategy. 
Which would basically have you use Eldoc as a library, controlling both 
the list of documentation functions and how they are composed.

Whereas having the eldoc-documentation-strategy defcustom at all makes 
it seem like the user's choice, and that all Eldoc clients must be able 
to perform well under any strategy chosen by the user. We might want to 
think twice before introducing such responsibility.

Next, eldoc-print-current-symbol-info is a mess, very hard to follow. I 
am frankly offended that you argued that both ad-hoc futures I showed, 
or any potential "proper" ones would make backtraces hard to read and 
debug, and then introduced this kind of control flow with callbacks 
calling dynamically generated callbacks, retrieved from a variable 
dynamically set in a caller function up the stack (to be clear: I think 
having eldoc--make-callback variable at all is a bad idea). This should 
very well be possible to untangle in the future, but I'd rather not have 
code like this in Emacs if we can help it.

Further, having all strategies basically delegate to hardcoded options 
inside eldoc-print-current-symbol-info seems to confirm that the set of 
available strategies is a closed one. Which is not what I would expect 
from a variable called eldoc-documentation-strategy.

These are my objections, for now. I'll have to study eldoc--handle-docs 
a bit later, but any problems it has are probably orthogonal to the rest 
of the list.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Please don't make it sound as if I'm the only one here having a strong
> opinion about proposed code. You disregarded the simple solution in 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41531#8, and then went

Are you in any way trying to say that you presented a "simple solution"
that somehow magically solves the problems of Eldoc?  If you do, you're
living in conflict with reality.

There is the problem to be solved.  And then there are techniques for
solving the problem.  These are two entirely separate things.  To be
absolutely clear: you have not, ever, proposed an alternate solution to
the Eldoc async _problem_.

You talked and talked and presented your favourite async handling
techniques, like having functions return functions that return
functions.  You contented that I -- not you -- should work with it to
solve the problem.

Callbacks, futures, promises, ad-hoc or proper, are just techniques.  I
tried a couple techniques, including a basic futures-based one proposed
by Stefan.  I didn't find any as compelling as the simplest and most
widely used in Emacs: callbacks.

For the quintillionth time: I AM NOT AGAINST FUTURES IN PRINCIPLE: I
just don't use these Eldoc fixes to shoehorn something rushedly into
Emacs.  Make a new thread, or join the existing one:

    https://lists.gnu.org/archive/html/emacs-devel/2020-03/msg00935.html

Afterwards, propose a change to the technique, not only in Eldoc but
elsewhere, too.  This idea is so simple that it boggles the mind that
you don't grasp it.

> urgent endeavor. E.g., Flymake is stable, and I don't have any
> particular bugs in mind that need solving.

Great.  I'll just note here that it uses exactly the same technique I'm
proposing in Eldoc to communicate with multiple backends: it's curious
how that doesn't bother you.  I would reasonably expect "futures" or
something else its implementation much simpler too.

> Aside: given that this discussion has user interface in mind, it needs
> more consideration of actual user experiences we'd want to allow. Ones 
> not provided by eldoc itself as well. For instance, I think we sooner
> or later should have a callsig floating popup (in the vein of MS's
> editors) as an alternative to showing the signature in the echo area
> only.

That is addressed in a comment in eldoc-docs

      ;; Finally, output to the echo area.  We handle the
      ;; `truncate-sym-name-if-fit' special case first, by selecting a
      ;; top-section of the `*eldoc' buffer.  I'm pretty sure nicer
      ;; strategies can be used here, probably by splitting this
      ;; function into some `eldoc-display-functions' special hook.

I agree there is ample space for improvement, including a 
"floating popup" (which I wouldn't limit to callsigs though).  Make
another bug report to study this.

> The new API is incompatible with the previous one, it requires
> changing a lot of functions (though admittedly in a minor way).

This is demonstrably false. As I've explained to Eli there is no
incompatibility in practice.  0 (zero) third-party extensions make use
of the protocol being changed from Eldoc 1.0.0 to Eldoc 1.1.0, unless
you're stalling here to secretly work on one.

But if we really, really wanted to, it's easy to get rid of the
arguments, too, with a variation to the callback technique.  I just
don't think it's worth it: a technique is a technique.

> is easy to miss, as evidenced by describe-char-eldoc which still
> doesn't accept any arguments.

Oh, an actual useful comment! Easily solved, thanks.  And it was only
"missed" because it wasn't used anywhere.

> Whereas we could limit ourselves to the change which would only make
> the clients use the different hook (or keep using the old one, for
> compatibility with older Flymake/Emacs versions).

Compatibility to the old `eldoc-documentation-function' protocol is
fully guaranteed.

> The choice to require a return of a non-nil value if the callback is
> going to be used is also kinda odd (+1 control flow to test). What's
> the problem with using the callback synchronously if the doc is
> available right away?

Nothing, you can do it.  As long as you return non-nil, non-string.  But
if you are are going to synchronously call the callback on a string, you
might as well return that string, just seems simpler.

> The decision to double down on having different
> eldoc-documentation-strategy values seems odd to me in several
> respects.
>
> First of all, if I understand the main motivation behind it, it's to
> delegate the implementation of asynchronous primitives to Eldoc.

It's rather to make clients don't worry about the synchronization.  Not
specifically to make Eldoc worry about it.  As soon as you and others
come up with the uber-async library, I think Eldoc and Flymake and many
others will be very pleased to delegate work to it.

> We don't want to bother with that in Eglot, etc. But you mentioned "a
> type of docstring" to be returned in the discussion (and the callbacks
> have the plist argument as a future proofing). If the type is a
> buffer, its contents might as well be created from several async calls

If you want to support "buffers" as a "type of docstring", just do that:
make buffers a type of docstring.  The obvious way is to have multiple
sources produce multiple buffers.

Thing: why would you ever want to put buffer-joining complexity in the
client?  They're not in the business of doing async gymnastics, they're
in the business of serving things coming from servers as efficiently and
effectively as possible.

> , which would require the same async primitives (though rather in
> general purpose form) available on the client anyway. Though it
> doesn't seem to be necessary for LSP in common operations, it's
> unlikely to be the only such protocol we'd ever want to support.

I don't know about that, but if we did, the current mechanism work
nicely for the example you presented.

> The strategies themselves:
>
> - eldoc-documentation-enthusiast: What's the difference compared to
> the 'default' one? Sacrificing CPU load for lower latency? It's an odd
> choice to force the user to make. The only people who can make an
> ideal decision regarding this are probably the authors of
> eldoc-documentation-functions, but it wouldn't help if
> eldoc-documentation-functions has functions coming from different
> authors, facilities, etc.

Has nothing to do with CPU.  This is the way Eglot works now, or at
least tries to: there are two delayed doc-producing backends, and
neither is guaranteed to complete.  One has priority over the other (and
special hooks are a decent, standard Emacs way to manage priority)

Eglot shows the lower-priority one if it shows it can survive for more
than x seconds (currently x = 0.3, uncustomizable).  No more doc
blinking.

> - eldoc-documentation-compose: Okay, this is kinda interesting (though
>   not essential),

> I think the only reasonably predictable behavior would be to truncate
> each of them to one line, always.

That's a display problem, not a composition problem For now it works OK
for one liners and also multiple multi-liners.  Displaying doc is not
the main goal of these patches, there is certainly room for improvement,
as I said above.

> - eldoc-documentation-compose-eagerly: Ooh, now I think I see why
> Returning futures instead (when async is needed) would provide this
> kind of guarantee without the seeming duplication of signals.

Can't let go of that futures drum, can you?  It'd be a pleasure to see
you walk the walk.

> On a related note, I believe some facilities might want to use only
> certain "kinds" of documentation functions (as indicated by the plist 
> arguments). If the plist is only provided together with the response,
> there is no way to avoid unnecessary computations (e.g. making HTTP 
> requests that return some other kinds of values). If the plist was
> returned together with a future value, however, it would be easy to
> do, as long as we document that the futures should start the
> computation until a callback is provided (if possible, at least).

Save it for your future futures implementation.

> And in a different high-level argument: you stated that you intend to
> have Eglot use a non-default value of eldoc-documentation-strategy. 

OK, but this has nothing to do with Eldoc, does it?  Make a bug report
for Eglot, I'll explain why it does this, and maybe I'll change it..

> Next, eldoc-print-current-symbol-info is a mess, very hard to
> follow. I am frankly offended that you argued that both ad-hoc futures

I meant no effense.

> idea). This should very well be possible to untangle in the future,
> but I'd rather not have code like this in Emacs if we can help it.

You're such an ace programmer that you code alternatives that are so
brief that they occupy no code at all!

> Further, having all strategies basically delegate to hardcoded options
> inside eldoc-print-current-symbol-info seems to confirm that the set
> of available strategies is a closed one. Which is not what I would
> expect from a variable called eldoc-documentation-strategy.

There are four strategies to choose from but you can make more.  What
did you have in mind?  

> These are my objections, for now. I'll have to study
> eldoc--handle-docs a bit later, but any problems it has are probably
> orthogonal to the rest of the list.

Thanks.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 05.07.2020 02:12, João Távora wrote:
>> I actually have multiple people working with me in Eglot, potentially
>> eager to see how things in Emacs core can be solved properly and
>> effectively, and being utterly disappointed by your opposition in this
>> eldoc.el case.  That's the sad state of affairs IMO.
>
> A part of "solving things properly" is listening to feedback.

I've told you this before: you mistake attention for obedience.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

João Távora <joaotavora@gmail.com> writes:

> For the quintillionth time: I AM NOT AGAINST FUTURES IN PRINCIPLE:
> just don't use these Eldoc fixes to shoehorn something rushedly into
> Emacs.

Just a further note: not only am I NOT "against" futures, I don't even
disagree with you that a good async library CAN improve Eldoc and other
async code.  The question is, like it always is: how much of an effect
and at what cost.

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

[ Sorry I can't really take part in the discussion, 'cause it's
  a rather busy time here.  ]

[ The discussion is becoming too tense for my taste.  Please take
  a break and think whether all this is important enough to warrant
  criticizing each other this way.  I think you're both great hackers
  whose opinion and expertise I value very much.  ]

I think getting async support for eldoc is more important than *how* we
get it.

I suggest we install the current eldoc-async code into `master` and in
parallel work quickly on a futures/promises/aio/... package for
`master`.

As long as it's done before we cut the `emacs-28` branch, I can't see
any reason why we couldn't change the new (and hence not yet released,
except for the GNU ELPA package) API in a backward-incompatible way
(just like the eldoc-async already does).


        Stefan


João Távora [2020-07-07 11:56:01] wrote:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>> Please don't make it sound as if I'm the only one here having a strong
>> opinion about proposed code. You disregarded the simple solution in 
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41531#8, and then went
>
> Are you in any way trying to say that you presented a "simple solution"
> that somehow magically solves the problems of Eldoc?  If you do, you're
> living in conflict with reality.
>
> There is the problem to be solved.  And then there are techniques for
> solving the problem.  These are two entirely separate things.  To be
> absolutely clear: you have not, ever, proposed an alternate solution to
> the Eldoc async _problem_.
>
> You talked and talked and presented your favourite async handling
> techniques, like having functions return functions that return
> functions.  You contented that I -- not you -- should work with it to
> solve the problem.
>
> Callbacks, futures, promises, ad-hoc or proper, are just techniques.  I
> tried a couple techniques, including a basic futures-based one proposed
> by Stefan.  I didn't find any as compelling as the simplest and most
> widely used in Emacs: callbacks.
>
> For the quintillionth time: I AM NOT AGAINST FUTURES IN PRINCIPLE: I
> just don't use these Eldoc fixes to shoehorn something rushedly into
> Emacs.  Make a new thread, or join the existing one:
>
>     https://lists.gnu.org/archive/html/emacs-devel/2020-03/msg00935.html
>
> Afterwards, propose a change to the technique, not only in Eldoc but
> elsewhere, too.  This idea is so simple that it boggles the mind that
> you don't grasp it.
>
>> urgent endeavor. E.g., Flymake is stable, and I don't have any
>> particular bugs in mind that need solving.
>
> Great.  I'll just note here that it uses exactly the same technique I'm
> proposing in Eldoc to communicate with multiple backends: it's curious
> how that doesn't bother you.  I would reasonably expect "futures" or
> something else its implementation much simpler too.
>
>> Aside: given that this discussion has user interface in mind, it needs
>> more consideration of actual user experiences we'd want to allow. Ones 
>> not provided by eldoc itself as well. For instance, I think we sooner
>> or later should have a callsig floating popup (in the vein of MS's
>> editors) as an alternative to showing the signature in the echo area
>> only.
>
> That is addressed in a comment in eldoc-docs
>
>       ;; Finally, output to the echo area.  We handle the
>       ;; `truncate-sym-name-if-fit' special case first, by selecting a
>       ;; top-section of the `*eldoc' buffer.  I'm pretty sure nicer
>       ;; strategies can be used here, probably by splitting this
>       ;; function into some `eldoc-display-functions' special hook.
>
> I agree there is ample space for improvement, including a 
> "floating popup" (which I wouldn't limit to callsigs though).  Make
> another bug report to study this.
>
>> The new API is incompatible with the previous one, it requires
>> changing a lot of functions (though admittedly in a minor way).
>
> This is demonstrably false. As I've explained to Eli there is no
> incompatibility in practice.  0 (zero) third-party extensions make use
> of the protocol being changed from Eldoc 1.0.0 to Eldoc 1.1.0, unless
> you're stalling here to secretly work on one.
>
> But if we really, really wanted to, it's easy to get rid of the
> arguments, too, with a variation to the callback technique.  I just
> don't think it's worth it: a technique is a technique.
>
>> is easy to miss, as evidenced by describe-char-eldoc which still
>> doesn't accept any arguments.
>
> Oh, an actual useful comment! Easily solved, thanks.  And it was only
> "missed" because it wasn't used anywhere.
>
>> Whereas we could limit ourselves to the change which would only make
>> the clients use the different hook (or keep using the old one, for
>> compatibility with older Flymake/Emacs versions).
>
> Compatibility to the old `eldoc-documentation-function' protocol is
> fully guaranteed.
>
>> The choice to require a return of a non-nil value if the callback is
>> going to be used is also kinda odd (+1 control flow to test). What's
>> the problem with using the callback synchronously if the doc is
>> available right away?
>
> Nothing, you can do it.  As long as you return non-nil, non-string.  But
> if you are are going to synchronously call the callback on a string, you
> might as well return that string, just seems simpler.
>
>> The decision to double down on having different
>> eldoc-documentation-strategy values seems odd to me in several
>> respects.
>>
>> First of all, if I understand the main motivation behind it, it's to
>> delegate the implementation of asynchronous primitives to Eldoc.
>
> It's rather to make clients don't worry about the synchronization.  Not
> specifically to make Eldoc worry about it.  As soon as you and others
> come up with the uber-async library, I think Eldoc and Flymake and many
> others will be very pleased to delegate work to it.
>
>> We don't want to bother with that in Eglot, etc. But you mentioned "a
>> type of docstring" to be returned in the discussion (and the callbacks
>> have the plist argument as a future proofing). If the type is a
>> buffer, its contents might as well be created from several async calls
>
> If you want to support "buffers" as a "type of docstring", just do that:
> make buffers a type of docstring.  The obvious way is to have multiple
> sources produce multiple buffers.
>
> Thing: why would you ever want to put buffer-joining complexity in the
> client?  They're not in the business of doing async gymnastics, they're
> in the business of serving things coming from servers as efficiently and
> effectively as possible.
>
>> , which would require the same async primitives (though rather in
>> general purpose form) available on the client anyway. Though it
>> doesn't seem to be necessary for LSP in common operations, it's
>> unlikely to be the only such protocol we'd ever want to support.
>
> I don't know about that, but if we did, the current mechanism work
> nicely for the example you presented.
>
>> The strategies themselves:
>>
>> - eldoc-documentation-enthusiast: What's the difference compared to
>> the 'default' one? Sacrificing CPU load for lower latency? It's an odd
>> choice to force the user to make. The only people who can make an
>> ideal decision regarding this are probably the authors of
>> eldoc-documentation-functions, but it wouldn't help if
>> eldoc-documentation-functions has functions coming from different
>> authors, facilities, etc.
>
> Has nothing to do with CPU.  This is the way Eglot works now, or at
> least tries to: there are two delayed doc-producing backends, and
> neither is guaranteed to complete.  One has priority over the other (and
> special hooks are a decent, standard Emacs way to manage priority)
>
> Eglot shows the lower-priority one if it shows it can survive for more
> than x seconds (currently x = 0.3, uncustomizable).  No more doc
> blinking.
>
>> - eldoc-documentation-compose: Okay, this is kinda interesting (though
>>   not essential),
>
>> I think the only reasonably predictable behavior would be to truncate
>> each of them to one line, always.
>
> That's a display problem, not a composition problem For now it works OK
> for one liners and also multiple multi-liners.  Displaying doc is not
> the main goal of these patches, there is certainly room for improvement,
> as I said above.
>
>> - eldoc-documentation-compose-eagerly: Ooh, now I think I see why
>> Returning futures instead (when async is needed) would provide this
>> kind of guarantee without the seeming duplication of signals.
>
> Can't let go of that futures drum, can you?  It'd be a pleasure to see
> you walk the walk.
>
>> On a related note, I believe some facilities might want to use only
>> certain "kinds" of documentation functions (as indicated by the plist 
>> arguments). If the plist is only provided together with the response,
>> there is no way to avoid unnecessary computations (e.g. making HTTP 
>> requests that return some other kinds of values). If the plist was
>> returned together with a future value, however, it would be easy to
>> do, as long as we document that the futures should start the
>> computation until a callback is provided (if possible, at least).
>
> Save it for your future futures implementation.
>
>> And in a different high-level argument: you stated that you intend to
>> have Eglot use a non-default value of eldoc-documentation-strategy. 
>
> OK, but this has nothing to do with Eldoc, does it?  Make a bug report
> for Eglot, I'll explain why it does this, and maybe I'll change it..
>
>> Next, eldoc-print-current-symbol-info is a mess, very hard to
>> follow. I am frankly offended that you argued that both ad-hoc futures
>
> I meant no effense.
>
>> idea). This should very well be possible to untangle in the future,
>> but I'd rather not have code like this in Emacs if we can help it.
>
> You're such an ace programmer that you code alternatives that are so
> brief that they occupy no code at all!
>
>> Further, having all strategies basically delegate to hardcoded options
>> inside eldoc-print-current-symbol-info seems to confirm that the set
>> of available strategies is a closed one. Which is not what I would
>> expect from a variable called eldoc-documentation-strategy.
>
> There are four strategies to choose from but you can make more.  What
> did you have in mind?  
>
>> These are my objections, for now. I'll have to study
>> eldoc--handle-docs a bit later, but any problems it has are probably
>> orthogonal to the rest of the list.
>
> Thanks.
>
> João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 07.07.2020 13:58, João Távora wrote:
> I've told you this before: you mistake attention for obedience.

Most of the time I feel you're not even addressing my points.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 07.07.2020 16:38, Stefan Monnier wrote:
> As long as it's done before we cut the `emacs-28` branch, I can't see
> any reason why we couldn't change the new (and hence not yet released,
> except for the GNU ELPA package) API in a backward-incompatible way
> (just like the eldoc-async already does).

It's a good point, and if that was all that we'd need to agree on, I'd 
be fine to proceed.

But if we just wanted to get async into Eglot, that's what my simple 
patch did: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=41531#8, 
proposed months ago. With minimal backward compatibility issues.

The current discussion, and the current eldoc-async branch adds some 
more design decisions, as well as breaking changes.

And those decisions seem to be informed and made necessary by the lack 
of standard functions for combining async computations that eldoc 
clients could use.

As soon as we get futures/promises/aio into the core, that will cease to 
be the case. But I fear we wouldn't be able to roll back the related 
decisions so easily, however.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 07.07.2020 13:58, João Távora wrote:
>> I've told you this before: you mistake attention for obedience.
>
> Most of the time I feel you're not even addressing my points.

I try my best, but I don't have any control over your feelings :-)

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 07.07.2020 13:56, João Távora wrote:
> Are you in any way trying to say that you presented a "simple solution"
> that somehow magically solves the problems of Eldoc?  If you do, you're
> living in conflict with reality.
> 
> There is the problem to be solved.  And then there are techniques for
> solving the problem.  These are two entirely separate things.  To be
> absolutely clear: you have not, ever, proposed an alternate solution to
> the Eldoc async_problem_.

Perhaps there is a misunderstanding here.

What would it take for you to say that "the problem" is indeed "solved" 
to a sufficient degree that your work on Eglot can continue?

If you could mention both the strictly minimum conditions, as well as 
the "bonus round", that would be great.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

> I suggest we install the current eldoc-async code into `master` and in
> parallel work quickly on a futures/promises/aio/... package for
> `master`.

That's the point I've been making all along, indeed! Finally progress!

Thanks,
João

PS: I for one will try to invest some effort in some more
modular/powerful eldoc-display-functions... let's hope that's less
contentious.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

> The current discussion, and the current eldoc-async branch adds some more
> design decisions, as well as breaking changes.

I don't see any breaking changes (other than to things which haven't
yet been released IMO).

> As soon as we get futures/promises/aio into the core, that will cease to be
> the case.

Then let's get moving on that.  No need to wait.

> But I fear we wouldn't be able to roll back the related decisions
> so easily, however.

I don't see any reason to fear that.  The more time we spend discussing
what the ideal should look like, the less time we have to actually
get there.

The current eldoc-async branch doesn't get us further from the ideal,
I believe, unless `emacs-28` gets cut before we get our act together.

But if we don't get our act together before `emacs-28` then the
alternative is to have Emacs-28 without support for async eldoc, which
I think is even worse.

I recommend we try and be pragmatic.  Especially since it will make us
all happier (instead of arguing against each other, we get to work on
improving the code).


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 07.07.2020 13:56, João Távora wrote:

> You talked and talked and presented your favourite async handling
> techniques, like having functions return functions that return
> functions.  You contented that I -- not you -- should work with it to
> solve the problem.

I showed a solution to the part of the problem that we managed to define 
at that time. You added some more features now.

If you like, I can rewrite your branch in terms of that proposal. Or in 
terms of eldoc-future (as suggested by Stefan). Your pick.

(But we still need to discuss the extra changes in this branch).

> For the quintillionth time: I AM NOT AGAINST FUTURES IN PRINCIPLE: I
> just don't use these Eldoc fixes to shoehorn something rushedly into
> Emacs.  Make a new thread, or join the existing one:
> 
>      https://lists.gnu.org/archive/html/emacs-devel/2020-03/msg00935.html

Thanks for the link. I seem to have missed it. Not exactly the 
discussion to join, but the projects mentioned there could be helpful.

> Afterwards, propose a change to the technique, not only in Eldoc but
> elsewhere, too.  This idea is so simple that it boggles the mind that
> you don't grasp it.

Sure. As long as this discussion waits.

>> urgent endeavor. E.g., Flymake is stable, and I don't have any
>> particular bugs in mind that need solving.
> 
> Great.  I'll just note here that it uses exactly the same technique I'm
> proposing in Eldoc to communicate with multiple backends: it's curious
> how that doesn't bother you.  I would reasonably expect "futures" or
> something else its implementation much simpler too.

It doesn't have existing clients that return results synchronously. It 
almost always works with external processes. It doesn't care if a 
checker never calls back. It doesn't compose results from multiple 
checkers. Basically: it's simple, and whatever legacy it did have, you 
cordoned that behind flymake-proc (which everybody promptly migrated off).

And as you can note, the interface you proposed here is not exactly the 
same: due to some of the requirements above, it's slightly different. A 
flymake backend is considered running whenever it didn't signal an 
error. The convention you proposed here involves returning a non-string, 
non-nil value.

I imagine having different interfaces between facilities is better than 
having similar-by-subtly-different ones.

>> Aside: given that this discussion has user interface in mind, it needs
>> more consideration of actual user experiences we'd want to allow. Ones
>> not provided by eldoc itself as well. For instance, I think we sooner
>> or later should have a callsig floating popup (in the vein of MS's
>> editors) as an alternative to showing the signature in the echo area
>> only.
> 
> That is addressed in a comment in eldoc-docs
> 
>        ;; Finally, output to the echo area.  We handle the
>        ;; `truncate-sym-name-if-fit' special case first, by selecting a
>        ;; top-section of the `*eldoc' buffer.  I'm pretty sure nicer
>        ;; strategies can be used here, probably by splitting this
>        ;; function into some `eldoc-display-functions' special hook.

I think the first question to ask here, is whether the eldoc clients are 
not ultimately the best place to decide how the doc should be formatted 
and displayed.

So the alternative to your current proposal (with composing strategies) 
would be for us to provide a set of utility functions to clients: ones 
to manipulate async computations, ones to truncate the doc to desired 
dimensions, ones to combine several docs in one, etc, while honoring the 
value of eldoc-echo-area-use-multiline-p.

The advantage to that approach is that we don't limit the display to 
particular format (e.g. the truncate-sym-name-if-fit requires the symbol 
to be at the beginning of the line, and for it to be specified as the 
:thing property as well). The downside: mainly being unable to compose 
eldoc results from multiple clients. But I wonder how well we can manage 
to handle that anyway.

> I agree there is ample space for improvement, including a
> "floating popup" (which I wouldn't limit to callsigs though).  Make
> another bug report to study this.

Callsigs are a whole other matter, actually. The best experience seems 
impossible to implement through eldoc. The LSP protocol shows an example 
of a good data structure required for it (a list of overloads, the 
number of the current guessed one, and a documentation string for the 
function). Eldoc can be used in the absence of other options, but a 
different hook which requires richer values seems more appropriate.

>> The new API is incompatible with the previous one, it requires
>> changing a lot of functions (though admittedly in a minor way).
> 
> This is demonstrably false. As I've explained to Eli there is no
> incompatibility in practice.  0 (zero) third-party extensions make use
> of the protocol being changed from Eldoc 1.0.0 to Eldoc 1.1.0, unless
> you're stalling here to secretly work on one.

So what happens if we merge this to master now? Either you expect no 
clients, and there is no need to hurry, or we expect some new eldoc 
clients to use this interface. But then the same clients will experience 
breakage as soon as we switch over to "proper" futures.

> But if we really, really wanted to, it's easy to get rid of the
> arguments, too, with a variation to the callback technique.  I just
> don't think it's worth it: a technique is a technique.

The variation that I showed in my little patch a month ago?

>> is easy to miss, as evidenced by describe-char-eldoc which still
>> doesn't accept any arguments.
> 
> Oh, an actual useful comment! Easily solved, thanks.  And it was only
> "missed" because it wasn't used anywhere.

Well, you corrected its docstring in this branch.

>> The choice to require a return of a non-nil value if the callback is
>> going to be used is also kinda odd (+1 control flow to test). What's
>> the problem with using the callback synchronously if the doc is
>> available right away?
> 
> Nothing, you can do it.  As long as you return non-nil, non-string.  But
> if you are are going to synchronously call the callback on a string, you
> might as well return that string, just seems simpler.

Seems like a missed opportunity to simplify (or not).

>> First of all, if I understand the main motivation behind it, it's to
>> delegate the implementation of asynchronous primitives to Eldoc.
> 
> It's rather to make clients don't worry about the synchronization.

Because it's fairly difficult, right? Especially in the absence of 
helpful standard libraries.

>> We don't want to bother with that in Eglot, etc. But you mentioned "a
>> type of docstring" to be returned in the discussion (and the callbacks
>> have the plist argument as a future proofing). If the type is a
>> buffer, its contents might as well be created from several async calls
> 
> If you want to support "buffers" as a "type of docstring", just do that:
> make buffers a type of docstring.  The obvious way is to have multiple
> sources produce multiple buffers.

That doesn't really jive with what I imagined. If I want to produce a 
"pretty" buffer in a client, I will print to it. Possibly making several 
HTTP calls in the process, if that's not hard to do.

I would also probably prefer not to worry about some other eldoc client 
leaving its documentation function in the list before mine. Or having 
them later in the list, leaving some unrelated stuff in the resulting 
buffer. As a client, I would probably know which calls are made, which 
data is returned, and how I want it to be rendered.

> Thing: why would you ever want to put buffer-joining complexity in the
> client?

Because it knows in which order to put them? Or how to render the seams 
best? Or it could mix the HTTP responses in other ways than simply 
joining them with "\n\n". Having generic code do that seems limiting.

Having individual clients do that should encourage more attention to detail.

>> The strategies themselves:
>>
>> - eldoc-documentation-enthusiast: What's the difference compared to
>> the 'default' one? Sacrificing CPU load for lower latency? It's an odd
>> choice to force the user to make. The only people who can make an
>> ideal decision regarding this are probably the authors of
>> eldoc-documentation-functions, but it wouldn't help if
>> eldoc-documentation-functions has functions coming from different
>> authors, facilities, etc.
> 
> Has nothing to do with CPU.  This is the way Eglot works now, or at
> least tries to: there are two delayed doc-producing backends, and
> neither is guaranteed to complete.

Why not? HTTP responses normally arrive reliably.

> One has priority over the other (and
> special hooks are a decent, standard Emacs way to manage priority)
> 
> Eglot shows the lower-priority one if it shows it can survive for more
> than x seconds (currently x = 0.3, uncustomizable).  No more doc
> blinking.

Why not just wait for the first one, if its documentation function 
returned non-nil?

I considered commenting on the 0.3 magic number, but dropped it in the 
first review.

>> - eldoc-documentation-compose: Okay, this is kinda interesting (though
>>    not essential),
> 
>> I think the only reasonably predictable behavior would be to truncate
>> each of them to one line, always.
> 
> That's a display problem, not a composition problem For now it works OK
> for one liners and also multiple multi-liners.  Displaying doc is not
> the main goal of these patches, there is certainly room for improvement,
> as I said above.

Whether we can reliably display these docs in a "composed" way, from any 
sources, or not, should probably factor into the design. Because if not, 
do we really need different strategies?

>> - eldoc-documentation-compose-eagerly: Ooh, now I think I see why
>> Returning futures instead (when async is needed) would provide this
>> kind of guarantee without the seeming duplication of signals.
> 
> Can't let go of that futures drum, can you?  It'd be a pleasure to see
> you walk the walk.

There's not much inherent difficulty in extracting the future-merge code 
from here or elsewhere. The actual problems I mentioned in the email 
with the (tiny) future.el come from elsewhere (e.g. from requirements 
for using them for completion), but they need to be decided on, in order 
to minimize breakage later.

>> On a related note, I believe some facilities might want to use only
>> certain "kinds" of documentation functions (as indicated by the plist
>> arguments). If the plist is only provided together with the response,
>> there is no way to avoid unnecessary computations (e.g. making HTTP
>> requests that return some other kinds of values). If the plist was
>> returned together with a future value, however, it would be easy to
>> do, as long as we document that the futures should start the
>> computation until a callback is provided (if possible, at least).
> 
> Save it for your future futures implementation.

My point was, again, adopting futures here would create a structural 
change. A more incompatible one than if we adopted a more compatible 
API. Or straight away used eldoc-local futures.

>> And in a different high-level argument: you stated that you intend to
>> have Eglot use a non-default value of eldoc-documentation-strategy.
> 
> OK, but this has nothing to do with Eldoc, does it?  Make a bug report
> for Eglot, I'll explain why it does this, and maybe I'll change it..

It does, if the actual requirements here mean that Eglot could just as 
fine perform combining its documentation results itself, in its 
documentation function. Then Eldoc could eventually do away with the 
eldoc-documentation-function/strategy variable altogether.

And the current change to the API would be minimal.

>> idea). This should very well be possible to untangle in the future,
>> but I'd rather not have code like this in Emacs if we can help it.
> 
> You're such an ace programmer that you code alternatives that are so
> brief that they occupy no code at all!

Nice punch.

I hope you haven't missed the implication that it's _hard_ for me to 
make heads or tails of your code there. But I could take a shot.

>> Further, having all strategies basically delegate to hardcoded options
>> inside eldoc-print-current-symbol-info seems to confirm that the set
>> of available strategies is a closed one. Which is not what I would
>> expect from a variable called eldoc-documentation-strategy.
> 
> There are four strategies to choose from but you can make more.  What
> did you have in mind?

Well... if I were thinking further in the direction of strategies, 
perhaps some would first request/wait the documentation from sources 
that return buffers, and then if none of those return any, then query 
the rest of functions. Or order the sources based on their kind before 
doing the calls, using user-supplied algo. Or perhaps skip buffers which 
are already displayed in some window.

My point here was, though, that a strategy sounds like something 
customizable and extensible. So their semantics, docstrings and 
implementations will need to be more accessible to an average Lisp 
developer.

>> These are my objections, for now. I'll have to study
>> eldoc--handle-docs a bit later, but any problems it has are probably
>> orthogonal to the rest of the list.
> 
> Thanks.

Having looked at it, the only problems there I can report on are 
practical, the same as I described when talking about 
eldoc-documentation-compose in the previous email: blinking after every 
user command, missing truncation when composing several docstrings, and 
undefined behavior with multiple multiline docstrings.

Is it at all possible to get rid of blinking? One-line eldoc doesn't blink.

Also, umm... it seems to truncate the contents of a long doc buffer to 
its bottom part. At least that's what I get when trying the related 
branch of Eglot.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> But if we really, really wanted to, it's easy to get rid of the
>> arguments, too, with a variation to the callback technique.  I just
>> don't think it's worth it: a technique is a technique.
>
> The variation that I showed in my little patch a month ago?

No, to be clear that was not a variation to the callback technique.  One
way is simply to passing the callback as a special variable (and there
are more ways.)

The rest of your long email hints that you've misunderstood what this
change to Eldoc is accomplishing.  I'm afraid I've done all I can to
explain it, including docstrings, NEWS entries, commit messages and
going through your previous very long code review.  I understand you
expected a futures library would come with this change, but it does not,
not for now.  I've explained that is only a technique, but in this last
email you conflate every issue with it again.  My position is: if there
really is value in them, futures will soon be in Emacs.  Let's follow
Stefan advice, it's good advice.  I might even work a bit on futures
myself.

Best,
João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 08.07.2020 01:49, João Távora wrote:
> The rest of your long email hints that you've misunderstood what this
> change to Eldoc is accomplishing.

So basically you even refuse to discuss the design choices in the 
branch, or the shortcomings of the implementation, its current bugs, etc?

And just merge the branch wholesale?

Thanks, Stefan!

 > I might even work a bit on futures myself.

Knock yourself out.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 07.07.2020 19:07, Stefan Monnier wrote:

 >> But I fear we wouldn't be able to roll back the related decisions
 >> so easily, however.

> I don't see any reason to fear that.  The more time we spend discussing
> what the ideal should look like, the less time we have to actually
> get there.

Given we can't even agree on the acceptance criteria, how would the 
rollback process look? Another "let's get it merged, folks"?

With nobody particular in charge of Eldoc except maybe for Joao now? Who 
will of course be ecstatic to change the API to one he explicitly 
disliked in the beginning.

> The current eldoc-async branch doesn't get us further from the ideal,
> I believe, unless `emacs-28` gets cut before we get our act together.
> 
> But if we don't get our act together before `emacs-28` then the
> alternative is to have Emacs-28 without support for async eldoc, which
> I think is even worse.

Why you don't consider the alternative with less invasive changes, is 
beyond my understanding.

> I recommend we try and be pragmatic.  Especially since it will make us
> all happier (instead of arguing against each other, we get to work on
> improving the code).

I wouldn't call the definition of eldoc-print-current-symbol-info in 
this branch an improvement over anything.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 08.07.2020 01:49, João Távora wrote:
>> The rest of your long email hints that you've misunderstood what this
>> change to Eldoc is accomplishing.
>
> So basically you even refuse to discuss the design choices in the
> branch, or the shortcomings of the implementation, its current bugs,
> etc?

I read your email, but it's very hard to follow your verbal conjectures
that this here or that there will fail catastrophically.

I suggest you make a bug report for each bug, describe a recipe for
reproducing the bug.  Likewise, if there are shortcomings in the design,
state clearly what you wanted do, but can't.  I promise to attend to
them, time permitting.

It's important to disconnect these reports from your fondness for
futures, which is a technique for which we don't have a library or
consensus for.  That in particular makes your otherwise extremely
valuable feedback very hard to follow.  Let's take Stefan's advice on
the futures front, is my recommendation.

Also, I agree with Stefan that things have become tense and agressive,
and no good progress can be made in these conditions.

Best,
João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 08.07.2020 02:24, João Távora wrote:

> I read your email, but it's very hard to follow your verbal conjectures
> that this here or that there will fail catastrophically.

No such conjectures there.

> I suggest you make a bug report for each bug, describe a recipe for
> reproducing the bug.  Likewise, if there are shortcomings in the design,
> state clearly what you wanted do, but can't.  I promise to attend to
> them, time permitting.

How about you reply to the email I already wrote and ask questions where 
something is hard to understand?

> It's important to disconnect these reports from your fondness for
> futures, which is a technique for which we don't have a library or
> consensus for.

You're ducking away from your previous statements. One message, you 
accuse me of not implementing everything myself. In the next one, after 
I offer to do just that, you cut the discussion short and accuse me of 
fearmongering or something.

> Also, I agree with Stefan that things have become tense and agressive,
> and no good progress can be made in these conditions.

Let's tone it down, then.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

On Wed, Jul 8, 2020 at 12:42 AM Dmitry Gutov <dgutov@yandex.ru> wrote:
> One message, you
> accuse me of not implementing everything myself. In the next one, after
> I offer to do just that, you cut the discussion short

You're confused, but by all means code away, you don't need my
permission for that. Good luck!

João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 08.07.2020 02:46, João Távora wrote:
> You're confused, but by all means code away, you don't need my
> permission for that. Good luck!

So I spend a few hours coding, and you wave the result away just as 
promptly as you did with the code review? Or with my previous patch?

I asked what you actually need here in terms of functionality to go on 
with your work on Eglot. No response came to that either.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

> Why you don't consider the alternative with less invasive changes, is beyond
> my understanding.

[ It's the same old maintainer's conundrum, trying to balance the need
  to stay in control of the code and make sure it's coherent etc.. while
  at the same time delegating and encouraging participation.  ]

To a large extent, I tend to give a lot of weight to existing code.

I don't have the time or energy to work on eldoc myself, and apparently
neither do you, so the one who writes the code gets to take most of
the decisions.

From what I remember the last time I looked at the branch, there were
things I didn't much like, but not more so than in the rest of Emacs ;-)

I can't ask everyone to code what I would write (and it's often for the
better since what I'd write would likely not work because I'm not aware
of the underlying difficulties that would make it fail).


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 08.07.2020 06:58, Stefan Monnier wrote:
> I don't have the time or energy to work on eldoc myself, and apparently
> neither do you, so the one who writes the code gets to take most of
> the decisions.

I have offered to do a rewrite multiple times now.

The only encouragement in that direction I have received was "can't stop 
you".

>  From what I remember the last time I looked at the branch, there were
> things I didn't much like, but not more so than in the rest of Emacs;-)

I thought we were more into detailed reviews and good code.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Stefan Monnier]

>> I don't have the time or energy to work on eldoc myself, and apparently
>> neither do you, so the one who writes the code gets to take most of
>> the decisions.
> I have offered to do a rewrite multiple times now.

Feel free to do so.  But it does mean that one of you two will
be frustrated because your/his code won't be accepted.

Maybe a better option, then is to:

- do a quick version of the code for yourself.
- then compare that with Joao's version.
- then do a mix of:
  - argue to change or remove some parts of his code (those that would
    make it difficult for you to install your code on top of his)
  - keep the other changes to apply them after he installs his

WDYT?


        Stefan

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

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

>>> I don't have the time or energy to work on eldoc myself, and apparently
>>> neither do you, so the one who writes the code gets to take most of
>>> the decisions.
>> I have offered to do a rewrite multiple times now.
>
> Feel free to do so.  But it does mean that one of you two will
> be frustrated because your/his code won't be accepted.
>
> Maybe a better option, then is to:
>
> - do a quick version of the code for yourself.
> - then compare that with Joao's version.
> - then do a mix of:
>   - argue to change or remove some parts of his code (those that would
>     make it difficult for you to install your code on top of his)
>   - keep the other changes to apply them after he installs his
>
> WDYT?

Fine with this plan.  Would even dare to call it "development" ;-)

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 08.07.2020 16:25, Stefan Monnier wrote:
> - do a quick version of the code for yourself.
> - then compare that with Joao's version.
> - then do a mix of:
>    - argue to change or remove some parts of his code (those that would
>      make it difficult for you to install your code on top of his)
>    - keep the other changes to apply them after he installs his

Since he went ahead, ignored the review and pushed the changes, 
apparently I can just do the same now.

That just leaves the buggy new features which went in without proper 
justification. Can I go ahead and remove them, then?

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[João Távora]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 08.07.2020 16:25, Stefan Monnier wrote:
>> - do a quick version of the code for yourself.
>> - then compare that with Joao's version.
>> - then do a mix of:
>>    - argue to change or remove some parts of his code (those that would
>>      make it difficult for you to install your code on top of his)
>>    - keep the other changes to apply them after he installs his
>
> Since he went ahead, ignored the review and pushed the changes,
> apparently I can just do the same now.
>
> That just leaves the buggy new features which went in without proper
> justification. Can I go ahead and remove them, then?

I think you should M-x report-emacs-bug and explain the bug. 

Please stop falsely claiming I "ignored" you review.  This is distateful
and offensive.  I replied to both emails: neither were a code review
like Eli's and Stefan's were, which were much easier to follow.  For the
first one I replied diligently to each point as I could, especially when
direct mentions to code that weren't related to futures and when I could
understand your question.  While the first email was already quite full
of tension and off-hand remarks about my code being offensive and
undemonstrated claims about code being bad in general, the second one
was worse in that respect, it was purely opinative and again conflated
most issues with your penchant for an implementation based on futures,
making it extremely hard to follow your point, as I already told you.

Therefore, I suggest you describe the issues with M-x report-emacs-bug
for bugs and for features that you don't understand the value of.  Make
a separate bug report for each problem.  Again, I promise to reply to
those.  Here are suggestions for subject lines of those bug reports, as
I gathered them from my reading of your emails:

- No purpose in `eldoc-documentation-enthusiast`
- `eldoc-documentation-compose` causes blinking
- `eldoc-documentation-functions` should use futures library
- `eldoc-print-current-symbol-info` is extremely complicated
- This much simpler patch would solve all of Eldoc's asynchronous problems
- Eldoc now eats my homework

Etc, etc.  Hopefully some progress can be made in those discussions.

Thank you very much,
João

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Dmitry Gutov]

On 08.07.2020 18:12, João Távora wrote:
> Please stop falsely claiming I "ignored" you review.  This is distateful
> and offensive.

Not as offensive as having one's writing ignored.

 > I replied to both emails: neither were a code review
like Eli's and Stefan's were, which were much easier to follow.

Both Stefan and Eli were too busy to do an in-depth review. And I now 
know better than to do one for you either.

 > While the first email was already quite full
of tension and off-hand remarks about my code being offensive and
undemonstrated claims about code being bad in general, the second one
was worse in that respect,

There was a lot of different issues discussed in those emails. You 
ignoring most of them and retelling the contents now in this simplistic 
way is offensive (but about on par with your other messages in this 
discussion). I'm not going to describe the same things again and again, 
in many different ways. I have wasted enough time on this already.

And if you didn't want reviews this long, you shouldn't have submitted a 
branch with multiple orthogonal features intermixed. You should have 
picked a proper solution to the issue #1, then filed a bug for the next 
one. Then we could have had much easier, targeted reviews.

bug#41531: 27.0.91; Better handle asynchronous eldoc backends

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 8 Jul 2020 21:32:33 +0300
> Cc: 41531@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
>  andreyk.mad@gmail.com
> 
> Both Stefan and Eli were too busy to do an in-depth review.

Actually, busy or not, I did the best review I could.  I'm sure Stefan
did the same.

And next time you'd like to know whether I did an in-depth review or
was too busy to afford that, you can ask me.