bug#62029: 29.0.60; Allow users to customize eldoc buffer separator

[Dmitry Gutov <dmitry@gutov.dev>]

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

Hi Joao,

On 15/04/2023 12:41, João Távora wrote:
>> Perhaps the former would reuse the existing set of
>> combinator/strategies, [...]
>> This separation could still [...]
>> I'm not going to bother with a patch because backward compatibility,
>> and blah,
> Yes, I understand this, but the consequence downside is that your case
> is weakened because other brains can't see the advantages as clearly as
> your brain does.  And all brains make mistakes.  Code doesn't.
> 
>> and the current approach can obviously function too, though
>> in a more complicated way.
> IMO, that can only be asserted when comparing the two things side by
> side, with actual examples.  Frequently some things are easy to do in
> one approach and harder in the competing approach, and vice versa.  In
> my brain it is your solution which_sounds_  way more complicated.  The
> key word is "sound" which I have to use because I'm hearing words inside
> my brain as I read your writing.  I'm not seeing your solution function
> in front of me, and described in non-ambiguous Elisp language with
> working examples.

This has been on my TODO for a while (sorry about the wait), so here's a 
rough patch for illustration. It incorporates an updated version of your 
earlier proposal for elisp-mode.el (from 24/03/2023) for comparison.

Some benefits that should come with it are:

1. The elements in eldoc-documentation-functions can do less work than 
the ones in eldoc-buffer-functions. I understand that this is not a 
priority for Eglot which just uses the same source of information, but 
for others it can be different. E.g. require an extra network call to 
fetch the full docstring (this is not a hypothetical).
2. Different eldoc-documentation-strategy can be used for the buffer's 
contents. E.g. using the current one (first non-nil) for the echo and 
the "compose" for the buffer seems logical to me, and the patch 
illustrates it.
3. 'M-x eldoc-doc-buffer' is made synchronous and only shows info for 
the point, not a result of some earlier lookup, which might (or might 
not) get refreshed asynchronously.

There can be other solutions for 1 (e.g. passing some extra info *into* 
each eldoc-documentation-functions, so they can choose what to show, 
rather than have them provide all variations in the returned value), but 
it seems harder to find one for 2.

Drawbacks:
1. Seemingly more duplication in the code (but other doc providers might 
choose to use entirely different looks; no duplication in that case).
2. When both functions are based off the same info, reusing it gets 
harder. But it's nothing a extra cache cannot fix.

[-- Attachment #2: eldoc-doc-buffer.diff --]
[-- Type: text/x-patch, Size: 6728 bytes --]

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 22144ed7c18..4646b4e98c2 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -409,7 +409,7 @@ eldoc-display-message-no-interference-p
 
 \f
 (defvar eldoc-documentation-functions nil
-  "Hook of functions that produce doc strings.
+  "Hook of functions that produce short doc strings.
 
 A doc string is typically relevant if point is on a function-like
 name, inside its arg list, or on any object with some associated
@@ -470,8 +470,11 @@ eldoc-documentation-functions
 taken into account if the major mode specific function does not
 return any documentation.")
 
+(defvar eldoc-buffer-functions nil
+  "Hook of functions that produce doc strings for `eldoc-doc-buffer'.")
+
 (defvar eldoc-display-functions
-  '(eldoc-display-in-echo-area eldoc-display-in-buffer)
+  '(eldoc-display-in-echo-area)
   "Hook of functions tasked with displaying ElDoc results.
 Each function is passed two arguments: DOCS and INTERACTIVE.  DOCS
 is a list (DOC ...) where DOC looks like (STRING :KEY VALUE :KEY2
@@ -486,22 +489,21 @@ eldoc-display-functions
 
 (defvar eldoc--doc-buffer nil "Buffer displaying latest ElDoc-produced docs.")
 
-(defun eldoc-doc-buffer (&optional interactive)
-  "Get or display ElDoc documentation buffer.
-
-The buffer holds the results of the last documentation request.
-If INTERACTIVE, display it.  Else, return said buffer."
-  (interactive (list t))
-  (unless (buffer-live-p eldoc--doc-buffer)
-    (user-error (format
-                 "ElDoc buffer doesn't exist, maybe `%s' to produce one."
-                 (substitute-command-keys "\\[eldoc]"))))
+(defun eldoc-doc-buffer ()
+  "Display the ElDoc documentation buffer.
+
+Request and show the documentation relevant to the thing at point."
+  (interactive)
+  (if (buffer-live-p eldoc--doc-buffer) (kill-buffer eldoc--doc-buffer))
+  (let ((eldoc-documentation-functions eldoc-buffer-functions)
+        (eldoc-display-functions '(eldoc-display-in-buffer))
+        (eldoc-documentation-strategy eldoc-buffer-strategy))
+    (eldoc-print-current-symbol-info))
+  (cl-assert (buffer-live-p eldoc--doc-buffer))
   (with-current-buffer eldoc--doc-buffer
-    (cond (interactive
-           (rename-buffer (replace-regexp-in-string "^ *" ""
-                                                    (buffer-name)))
-           (display-buffer (current-buffer)))
-          (t (current-buffer)))))
+    (rename-buffer (replace-regexp-in-string "^ *" ""
+                                             (buffer-name)))
+    (pop-to-buffer (current-buffer))))
 
 (defvar eldoc-doc-buffer-separator
   (concat "\n" (propertize "\n" 'face '(:inherit separator-line :extend t)) "\n")
@@ -676,7 +678,7 @@ eldoc-display-in-buffer
   "Display DOCS in a dedicated buffer.
 If INTERACTIVE is t, also display the buffer."
   (eldoc--format-doc-buffer docs)
-  (when interactive (eldoc-doc-buffer t)))
+  (when interactive (eldoc-doc-buffer)))
 
 (defun eldoc-documentation-default ()
   "Show the first non-nil documentation string for item at point.
@@ -793,6 +795,11 @@ eldoc-documentation-strategy
                 (function :tag "Other function"))
   :version "28.1")
 
+(defcustom eldoc-buffer-strategy #'eldoc-documentation-compose
+  "Doc string doc string."
+  :type 'function
+  :version "30.1")
+
 (defun eldoc--supported-p ()
   "Non-nil if an ElDoc function is set for this buffer."
   (and (not (memq eldoc-documentation-strategy '(nil ignore)))
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index ff90a744ea3..a13904ddb26 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -355,6 +355,10 @@ emacs-lisp-mode
             #'elisp-eldoc-funcall nil t)
   (add-hook 'eldoc-documentation-functions
             #'elisp-eldoc-var-docstring nil t)
+  (add-hook 'eldoc-buffer-functions
+            #'elisp-eldoc-funcall-buf nil t)
+  (add-hook 'eldoc-buffer-functions
+            #'elisp-eldoc-var-docstring-buf nil t)
   (add-hook 'xref-backend-functions #'elisp--xref-backend nil t)
   (setq-local project-vc-external-roots-function #'elisp-load-path-roots)
   (setq-local syntax-propertize-function #'elisp-mode-syntax-propertize)
@@ -1822,6 +1826,22 @@ elisp-eldoc-funcall
                          'font-lock-function-name-face
                        'font-lock-keyword-face)))))
 
+(defun elisp-eldoc-funcall-buf (callback &rest _ignored)
+  (let* ((fn-sym (car (elisp--fnsym-in-current-sexp)))
+         (fulldoc
+          (and (and fn-sym (symbolp fn-sym) (fboundp fn-sym))
+               (with-temp-buffer
+                 (let ((standard-output (current-buffer)))
+                   (describe-function-1 fn-sym)
+                   (help-make-xrefs)
+                   (buffer-string))))))
+    (when fn-sym
+      (funcall callback fulldoc
+               :thing fn-sym
+               :face (if (functionp fn-sym)
+                         'font-lock-function-name-face
+                       'font-lock-keyword-face)))))
+
 (defun elisp-eldoc-var-docstring (callback &rest _ignored)
   "Document variable at point by calling CALLBACK.
 Intended for `eldoc-documentation-functions' (which see).
@@ -1833,6 +1853,21 @@ elisp-eldoc-var-docstring
                :thing sym
                :face 'font-lock-variable-name-face))))
 
+(defun elisp-eldoc-var-docstring-buf (callback &rest _ignored)
+  (let* ((sym (elisp--current-symbol))
+         (rawdoc (and (boundp sym)
+                      (documentation-property
+                       sym 'variable-documentation t)))
+         (fulldoc (and rawdoc
+                       (with-temp-buffer
+                         (insert rawdoc)
+                         (help-make-xrefs)
+                         (buffer-string)))))
+    (when fulldoc
+      (funcall callback fulldoc
+               :thing sym
+               :face 'font-lock-variable-name-face))))
+
 (defun elisp-eldoc-var-docstring-with-value (callback &rest _)
   "Document variable at point by calling CALLBACK.
 Intended for `eldoc-documentation-functions' (which see).
diff --git a/lisp/progmodes/flymake.el b/lisp/progmodes/flymake.el
index b27e6527f81..a56ec6cf8b0 100644
--- a/lisp/progmodes/flymake.el
+++ b/lisp/progmodes/flymake.el
@@ -1284,6 +1284,7 @@ flymake-mode
     (add-hook 'after-save-hook 'flymake-after-save-hook nil t)
     (add-hook 'kill-buffer-hook 'flymake-kill-buffer-hook nil t)
     (add-hook 'eldoc-documentation-functions 'flymake-eldoc-function t t)
+    (add-hook 'eldoc-buffer-functions 'flymake-eldoc-function t t)
 
     ;; If Flymake happened to be already ON, we must cleanup
     ;; existing diagnostic overlays, lest we forget them by blindly