GNU bug report logs - #17348
24.3.50; Cannot advice-add the same function :before and :after

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17348 in the body.
You can then email your comments to 17348 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50; Cannot advice-add the same function :before and :after
Date: Sat, 26 Apr 2014 13:16:00 +0200

Evalling:
(progn
  (defun yfcheck () (message "Check"))
  (defun yftest () (message "Test"))
  (advice-add 'yftest :before #'yfcheck)
  (advice-add 'yftest :after #'yfcheck)
  (yftest)
  nil)

gives (in the *Messages* buffer)
Test
Check

I'd have expected:
Check
Test
Check

-- 
Nico.

Message #8 received at 17348 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
Cc: 17348 <at> debbugs.gnu.org
Subject: Re: bug#17348: 24.3.50;
 Cannot advice-add the same function :before and :after
Date: Sat, 26 Apr 2014 08:46:13 -0400

>   (advice-add 'yftest :before #'yfcheck)
>   (advice-add 'yftest :after #'yfcheck)

`advice-add' will not add the same function twice, indeed.
Similarly to `add-hook'.  This is done explicitly in advice--add-function:

    (when a
      ;; The advice is already present.  Remove the old one, first.
      (setf (gv-deref ref)
            (advice--remove-function (gv-deref ref) (advice--car a))))

I guess we could change that, but then the notion of "removal" becomes
more complicated.


        Stefan

Message #11 received at 17348 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Nicolas Richard <theonewiththeevillook <at> yahoo.fr>, 17348 <at> debbugs.gnu.org
Subject: Re: bug#17348: 24.3.50;
 Cannot advice-add the same function :before and :after
Date: Sat, 26 Apr 2014 22:22:19 +0200

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

>>   (advice-add 'yftest :before #'yfcheck)
>>   (advice-add 'yftest :after #'yfcheck)
>
> `advice-add' will not add the same function twice, indeed.
> Similarly to `add-hook'.  This is done explicitly in advice--add-function:
>
>     (when a
>       ;; The advice is already present.  Remove the old one, first.
>       (setf (gv-deref ref)
>             (advice--remove-function (gv-deref ref) (advice--car a))))
>
> I guess we could change that, but then the notion of "removal" becomes
> more complicated.

In the actual situation that lead me to adding the same function twice,
it was a tracing function, like (message "time: %s ; value of
foovariable: %s" (current-time) foovariable) and I assumed that I would
be able to remove them both with one (advice-remove ...) -- I guess that
isn't too complicated.

OTOH I also think that the current behaviour is fine, but I'd suggest
emitting a warning when an advice is being overwritten/removed that way.

-- 
Nico.

Message #14 received at 17348 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
Cc: 17348 <at> debbugs.gnu.org
Subject: Re: bug#17348: 24.3.50;
 Cannot advice-add the same function :before and :after
Date: Thu, 01 May 2014 16:47:20 -0400

> In the actual situation that lead me to adding the same function twice,
> it was a tracing function, like (message "time: %s ; value of
> foovariable: %s" (current-time) foovariable) and I assumed that I would
> be able to remove them both with one (advice-remove ...) -- I guess that
> isn't too complicated.

Removing them both would not be desirable in general.

> OTOH I also think that the current behaviour is fine, but I'd suggest
> emitting a warning when an advice is being overwritten/removed that way.

It's not meant to be a "secret" behavior, so people should be able to
rely on this behavior (just like you can rely on add-hook being
idempotent) and use it, in which case emitting a warning is
not appropriate.

This said, maybe we should make it easier to do what you wanted.
E.g. we could probably make it possible to use different `name'
properties to let a user add a function several times (potentially at
different places, but even at the same place).


        Stefan

Message #19 received at 17348-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Nicolas Richard <theonewiththeevillook <at> yahoo.fr>
Cc: 17348-done <at> debbugs.gnu.org
Subject: Re: bug#17348: 24.3.50;
 Cannot advice-add the same function :before and :after
Date: Sat, 10 May 2014 16:09:21 -0400

I installed a patch in `emacs-24' which will let you add the same
function multiple times, as long as you specify different `name'
properties for each one.

>> In the actual situation that lead me to adding the same function twice,
>> it was a tracing function, like (message "time: %s ; value of
>> foovariable: %s" (current-time) foovariable) and I assumed that I would
>> be able to remove them both with one (advice-remove ...) -- I guess that
>> isn't too complicated.
> Removing them both would not be desirable in general.

With the new code, you can remove each one individually by specifying
its `name', or you can remove them all by specifying the function itself.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.