bug#68114: [PATCH] Make 'advice-remove' interactive

[Steven Allen <steven@stebalien.com>]

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


> You should include the default in the prompt using `format-prompt`.
>
>> +     (advice-mapc (lambda (f _) (push (cons (prin1-to-string f) f) advice)) symbol)

Fixed. I also:

- Broke the large `completing-read` call into multiple let bindings.
- Made sure that the default value was actually advised before
  suggesting it.

> The var name `advice` suggests it holds a single piece of advice.
> I'd call it `advices` instead.

Fixed

> Also, some advice's functions are lambda expressions (i.e. closures)
> which can be rather ugly/unwieldy when printed.  When code expects to
> remove them, we usually provide a `name` property for them, so I suggest
> that you use something like

Fixed, although I noticed a few oddities (commented in the code to avoid
confusing future readers).

Basically

1. 'name' can be either a string or a symbol, but they're not considered
the same name.
2. The same advice can be applied multiple times with different names,
so I changed the code to select the advice's 'name' instead of the
advice itself, if named.

> I suspect you want to `require-match` in the `completing-read` call.

Fixed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3740 bytes --]

From 130adbe7542304b2fa5dd23c23d66c275cd5bcb1 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Fri, 29 Dec 2023 09:53:05 -0800
Subject: [PATCH] Make 'advice-remove' interactive

`ad-advice-remove' is already interactive, but it doesn't work with
new-style advice.

* lisp/emacs-lisp/nadvice.el (advice-remove): Make it interactive.
* doc/lispref/functions.texi (Advising Named Functions): Document that
'advice-remove' is now an interactive command.
---
 doc/lispref/functions.texi |  8 +++++---
 etc/NEWS                   |  4 ++++
 lisp/emacs-lisp/nadvice.el | 24 ++++++++++++++++++++++++
 3 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index d0c8f3e90e8..6f5c1a997e2 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -2077,10 +2077,12 @@ Advising Named Functions
 (@pxref{Core Advising Primitives}).
 @end defun
 
-@defun advice-remove symbol function
+@deffn Command advice-remove symbol function
 Remove the advice @var{function} from the named function @var{symbol}.
-@var{function} can also be the @code{name} of a piece of advice.
-@end defun
+@var{function} can also be the @code{name} of a piece of advice.  When
+called interactively, prompt for both an advised @var{function} and
+the advice to remove.
+@end deffn
 
 @defun advice-member-p function symbol
 Return non-@code{nil} if the advice @var{function} is already in the named
diff --git a/etc/NEWS b/etc/NEWS
index c002ec33d45..553365fc7a4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -83,6 +83,10 @@ see the variable 'url-request-extra-headers'.
 \f
 * Changes in Emacs 30.1
 
+** 'advice-remove' is now an interactive command.
+When called interactively, 'advice-remove' now prompts for an advised
+function to the advice to remove.
+
 ** Emacs now supports Unicode Standard version 15.1.
 
 ** Network Security Manager
diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el
index 9f2b42f5765..f2fa5f34ef8 100644
--- a/lisp/emacs-lisp/nadvice.el
+++ b/lisp/emacs-lisp/nadvice.el
@@ -539,6 +539,30 @@ advice-remove
 or an autoload and it preserves `fboundp'.
 Instead of the actual function to remove, FUNCTION can also be the `name'
 of the piece of advice."
+  (interactive
+   (let* ((pred (lambda (sym) (advice--p (advice--symbol-function sym))))
+          (default (when-let* ((f (function-called-at-point))
+                               ((funcall pred f)))
+                     (symbol-name f)))
+          (prompt (format-prompt "Advised Function" default))
+          (symbol (intern (completing-read prompt obarray pred t nil nil default)))
+          advices)
+     (advice-mapc (lambda (f p)
+                    (let ((k (or (alist-get 'name p) f)))
+                      (push (cons
+                             ;; "name" (string) and 'name (symbol) are
+                             ;; considered different names so we use
+                             ;; `prin1-to-string' even if the name is
+                             ;; a string to distinguish between these
+                             ;; two cases.
+                             (prin1-to-string k)
+                             ;; We use `k' here instead of `f' because
+                             ;; the same advice can have multiple
+                             ;; names.
+                             k)
+                            advices)))
+                  symbol)
+     (list symbol (cdr (assoc-string (completing-read "Advice: " advices nil t) advices)))))
   (let ((f (symbol-function symbol)))
     (remove-function (cond ;This is `advice--symbol-function' but as a "place".
                       ((get symbol 'advice--pending)
-- 
2.43.0