[PATCH] Add `advice-remove-all`

[PATCH] Add `advice-remove-all`

[Tianxiang Xiong]

[-- Attachment #1.1: Type: text/plain, Size: 451 bytes --]

I recently had the need to remove all advice from a function I was playing
around with, and was surprised to find that `advice-remove-all` did not
exist.

Others have had need of it
<https://emacs.stackexchange.com/questions/24657/unadvice-a-function/24658#24658>
as well, so it seems like a good candidate for inclusion in `nadvice.el`.

The attached patch adds `advice-remove-all` to `nadvice.el`, using the
implementation suggested by xuchungyang.

[-- Attachment #1.2: Type: text/html, Size: 533 bytes --]

[-- Attachment #2: 0001-Add-advice-remove-all.patch --]
[-- Type: text/x-patch, Size: 3429 bytes --]

From cd0378a2493e391ba05889d9261317d3cef0db50 Mon Sep 17 00:00:00 2001
From: Tianxiang Xiong <tianxiang.xiong@gmail.com>
Date: Wed, 26 Apr 2017 20:13:05 -0700
Subject: [PATCH] Add `advice-remove-all`

Removes all advice from a function.
---
 doc/lispref/functions.texi |  4 ++++
 etc/NEWS                   |  4 ++++
 lisp/emacs-lisp/nadvice.el | 18 ++++++++++++------
 3 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index 283f74f..3f3bee3 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -1734,6 +1734,10 @@ 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
 
+@defun advice-remove-all symbol
+Remove all advice from named function @var{symbol}.
+@end defun
+
 @defun advice-member-p function symbol
 Return non-@code{nil} if the advice @var{function} is already in the named
 function @var{symbol}.  @var{function} can also be the @code{name} of
diff --git a/etc/NEWS b/etc/NEWS
index 9d4c72d..729c379 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -24,6 +24,10 @@ When you add a new item, use the appropriate mark if you are sure it applies,
 \f
 * Installation Changes in Emacs 26.1
 
++++
+** The function 'advice-remove-all' is added to 'nadvice.el'
+This allows all advice to be removed from a function.
+
 ** By default libgnutls is now required when building Emacs.
 Use 'configure --with-gnutls=no' to build even when GnuTLS is missing.
 
diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el
index fd1cd2c..957fd85 100644
--- a/lisp/emacs-lisp/nadvice.el
+++ b/lisp/emacs-lisp/nadvice.el
@@ -335,6 +335,12 @@ properties alist that was specified when it was added."
     (funcall f (advice--car function-def) (advice--props function-def))
     (setq function-def (advice--cdr function-def))))
 
+(defun advice-mapc (fun symbol)
+  "Apply FUN to every advice function in SYMBOL.
+FUN is called with a two arguments: the function that was added, and the
+properties alist that was specified when it was added."
+  (advice-function-mapc fun (advice--symbol-function symbol)))
+
 (defun advice-function-member-p (advice function-def)
   "Return non-nil if ADVICE is already in FUNCTION-DEF.
 Instead of ADVICE being the actual function, it can also be the `name'
@@ -453,6 +459,12 @@ of the piece of advice."
   nil)
 
 ;;;###autoload
+(defun advice-remove-all (symbol)
+  "Remove all advice from function named SYMBOL."
+  (advice-mapc (lambda (advice _props) (advice-remove symbol advice))
+               symbol))
+
+;;;###autoload
 (defmacro define-advice (symbol args &rest body)
   "Define an advice and add it to function named SYMBOL.
 See `advice-add' and `add-function' for explanation on the
@@ -476,12 +488,6 @@ otherwise it is named `SYMBOL@NAME'.
     `(prog1 ,@(and (symbolp advice) `((defun ,advice ,lambda-list ,@body)))
        (advice-add ',symbol ,where #',advice ,@(and props `(',props))))))
 
-(defun advice-mapc (fun symbol)
-  "Apply FUN to every advice function in SYMBOL.
-FUN is called with a two arguments: the function that was added, and the
-properties alist that was specified when it was added."
-  (advice-function-mapc fun (advice--symbol-function symbol)))
-
 ;;;###autoload
 (defun advice-member-p (advice symbol)
   "Return non-nil if ADVICE has been added to SYMBOL.
-- 
2.7.4

Re: [PATCH] Add `advice-remove-all`

[Stefan Monnier]

> I recently had the need to remove all advice from a function I was playing
> around with, and was surprised to find that `advice-remove-all` did not
> exist.

Do you have a use-case?

In the case of https://emacs.stackexchange.com/questions/24657, my
impression that all that was needed is an interactive `advice-remove`
which provides completion (so that to remove all pieces of advice, you
may have to use this command a few times, maybe, but it's rather rare).


        Stefan

Re: [PATCH] Add `advice-remove-all`

[Marcin Borkowski]

On 2017-04-27, at 14:52, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> I recently had the need to remove all advice from a function I was playing
>> around with, and was surprised to find that `advice-remove-all` did not
>> exist.
>
> Do you have a use-case?
>
> In the case of https://emacs.stackexchange.com/questions/24657, my
> impression that all that was needed is an interactive `advice-remove`
> which provides completion (so that to remove all pieces of advice, you
> may have to use this command a few times, maybe, but it's rather rare).

Would that work when I (quite foolishly, I admit) advised something with
an anonymous function?

Best,

-- 
Marcin Borkowski

Re: [PATCH] Add `advice-remove-all`

[Stefan Monnier]

>>> I recently had the need to remove all advice from a function I was playing
>>> around with, and was surprised to find that `advice-remove-all` did not
>>> exist.
>> Do you have a use-case?
>> In the case of https://emacs.stackexchange.com/questions/24657, my
>> impression that all that was needed is an interactive `advice-remove`
>> which provides completion (so that to remove all pieces of advice, you
>> may have to use this command a few times, maybe, but it's rather rare).
> Would that work when I (quite foolishly, I admit) advised something with
> an anonymous function?

In the absence of actual code, I can only say that it could work.
Clearly, it would be better to write the code so that it does.


        Stefan


PS: I don't think it's foolish, BTW.

Re: [PATCH] Add `advice-remove-all`

[Marcin Borkowski]

On 2017-05-01, at 19:21, Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>>>> I recently had the need to remove all advice from a function I was playing
>>>> around with, and was surprised to find that `advice-remove-all` did not
>>>> exist.
>>> Do you have a use-case?
>>> In the case of https://emacs.stackexchange.com/questions/24657, my
>>> impression that all that was needed is an interactive `advice-remove`
>>> which provides completion (so that to remove all pieces of advice, you
>>> may have to use this command a few times, maybe, but it's rather rare).
>> Would that work when I (quite foolishly, I admit) advised something with
>> an anonymous function?
>
> In the absence of actual code, I can only say that it could work.
> Clearly, it would be better to write the code so that it does.

Of course.  Maybe I'll try to do that...

>         Stefan
>
> PS: I don't think it's foolish, BTW.

Well, it seems to me it is: showing the value of such a hook is
less-than-optimal and removing the lambda from it _might_ be tricky.

Best,

--
Marcin Borkowski

Re: [PATCH] Add `advice-remove-all`

[Stefan Monnier]

> Well, it seems to me it is: showing the value of such a hook is
> less-than-optimal and removing the lambda from it _might_ be tricky.

Yes, it has downsides, but they don't always matter: I also recommend using
a function symbol rather than a closure, but that doesn't mean using
a closure is necessarily foolish.


        Stefan

Re: [PATCH] Add `advice-remove-all`

[Matthew Newton]

My use case is when I’m developing something in my config and I mess up something up. I might iterate on an advice function and end up with multiple advice elements. This is a quick, convenient way to clean it up. Is there a different way that would make more sense?

Re: [PATCH] Add `advice-remove-all`

[Stefan Monnier]

> My use case is when I’m developing something in my config and I mess up
> something up. I might iterate on an advice function and end up with multiple
> advice elements. This is a quick, convenient way to clean it up. Is there
> a different way that would make more sense?

Maybe the advice info displayed in *Help* by the likes of `C-h o` could
let you do that interactively?

Also, I recently came across the idea to have an `advice-list` command
that shows all the advised functions, in a buffer (a bit like
`list-processes`).  I'm not completely sure it's worth the trouble, but
it could occasionally be a good way to have an overview of "how bad
things got".

It wouldn't belong in the preloaded nadvice.el file, of course.


        Stefan

Re: [PATCH] Add `advice-remove-all`

[T.V Raman]

Using the original advice.el, you could assign an advice a "name";
redefining that named advice then got you the new behavior.

In that environment, and perhaps until emacs 25, doing describe-function
told you a function was advised, more importantly, you  also got a
button that you could click in the help buffer that showed the doc
string for that advice 
--