bug#66032: [PATCH] Inline advice documentation into advised function's docstring, after all

[Jens Schmidt via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Severity: wishlist
X-Debbugs-CC: drew.adams@oracle.com, monnier@iro.umontreal.ca

I'm aware that I'm late to the party (got unstuck from Emacs 23
only recently) and that there have been some reports on that
already (bug#14734 and merged).

But at least on one occasion Stefan has asked for a patch, and I
haven't seen yet patches that got declined.  So here is one, so
far the plain code patch only, without NEWS, tests, etc.  It was
actually easier to implement than I thought, I hope I haven't
overseen any edge cases.

Here are two examples of how the generated documentation looks
like.  The former shows documentation of a legacy-advised
function, the latter shows documentation of one of Stefan's
test advice to demonstrate that these cases are handled as well.

------------------------- snip -------------------------
ediff-quit is an interactive byte-compiled Lisp function in
‘ediff-util.el’.

(ediff-quit REVERSE-DEFAULT-KEEP-VARIANTS)

Finish an Ediff session and exit Ediff.
[...]
With prefix argument REVERSE-DEFAULT-KEEP-VARIANTS, temporarily
reverse the meaning of this variable.

<font-lock-warning-face>This function is advised.</f-l-w-f>

This function has :around advice: ‘ad-Advice-ediff-quit’
Does not ask any stupid questions.

Pops to buffer A.

[back]
------------------------- snip -------------------------

------------------------- snip -------------------------
sm-test1 is a Lisp macro.

(sm-test1 A)

<font-lock-warning-face>This function is advised.</f-l-w-f>

This macro has :override advice: No documentation

This is an :override advice, which means that ‘sm-test1’ isn’t
run at all, and the documentation below may be irrelevant.

This macro has :around advice: No documentation

[back]
------------------------- snip -------------------------

What do you think?

[-- Attachment #2: 0001-Improve-documentation-of-advised-macros-or-functions.patch --]
[-- Type: text/x-patch, Size: 3155 bytes --]

From 5b7c14242c1efad6d5ab05bf159ae0a5b7f41c32 Mon Sep 17 00:00:00 2001
From: Jens Schmidt <jschmidt4gnu@vodafonemail.de>
Date: Sat, 16 Sep 2023 14:53:18 +0200
Subject: [PATCH] Improve documentation of advised macros or functions

* lisp/emacs-lisp/nadvice.el (advice--make-single-doc): Inline advice
documentation where possible.
(advice--make-docstring): Add a prominent marker that the macro or
function is advised.
---
 lisp/emacs-lisp/nadvice.el | 47 +++++++++++++++++++++++++++++---------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el
index cd80df2c41d..f04a584cf74 100644
--- a/lisp/emacs-lisp/nadvice.el
+++ b/lisp/emacs-lisp/nadvice.el
@@ -93,17 +93,30 @@ advice--make-single-doc
      (format "This %s has %s advice: "
              (if macrop "macro" "function")
              how)
-     (let ((fun (advice--car flist)))
-       (if (symbolp fun) (format-message "`%S'." fun)
-         (let* ((name (cdr (assq 'name (advice--props flist))))
-                (doc (documentation fun t))
-                (usage (help-split-fundoc doc function)))
-           (if usage (setq doc (cdr usage)))
-           (if name
-               (if doc
-                   (format "%s\n%s" name doc)
-                 (format "%s" name))
-             (or doc "No documentation")))))
+     (let* ((fun (advice--car flist))
+            (doc (documentation fun t))
+            (usage (help-split-fundoc doc function))
+            name)
+       (if usage (setq doc (cdr usage)))
+       (if (symbolp fun)
+           (if doc
+               (concat
+                (format-message "`%S'\n" fun)
+                ;; Remove first line possibly added by
+                ;; `ad-make-single-advice-docstring' for
+                ;; legacy advices.
+                (if (string-match
+                     "^\\(?:Before\\|Around\\|After\\)-advice `.*?':\n"
+                     doc)
+                    (substring doc (match-end 0))
+                  doc))
+             (format-message "`%S'." fun))
+         (setq name (cdr (assq 'name (advice--props flist))))
+         (if name
+             (if doc
+                 (format "%s\n%s" name doc)
+               (format "%s" name))
+           (or doc "No documentation"))))
      "\n"
      (and
       (eq how :override)
@@ -155,10 +168,22 @@ advice--make-docstring
       (help-add-fundoc-usage
        (with-temp-buffer
          (when before
+           (insert
+            (propertize
+             (concat "This " (if macrop "macro" "function") " is advised.")
+             'face 'font-lock-warning-face))
+           (ensure-empty-lines 1)
            (insert before)
            (ensure-empty-lines 1))
          (when origdoc
            (insert origdoc))
+         (when (not before)
+           (ensure-empty-lines 1)
+           (insert
+            (propertize
+             (concat "This " (if macrop "macro" "function") " is advised.")
+             'face 'font-lock-warning-face))
+           (ensure-empty-lines 1))
          (when after
            (ensure-empty-lines 1)
            (insert after))
-- 
2.30.2