bug#14317: 24.3.50; nadvice.el: named advices not upgradable

bug#14317: 24.3.50; nadvice.el: named advices not upgradable

[Michael Heerdegen]

Hi Stefan,

if you have some code adding advices that can be identified (i.e.,
adding named advices or fbound symbols, not anonymous functions),
and you change this code defining the advice and re-evaluate, the change
doesn't take effect (in contrast to the behavior of advice.el).

We currently use this:

--8<---------------cut here---------------start------------->8---
(defun advice--add-function (where ref function props)
  (unless (advice--member-p function (cdr (assq 'name props))
                            (gv-deref ref))
    (setf (gv-deref ref)
          (advice--make where function (gv-deref ref) props))))
--8<---------------cut here---------------end--------------->8---

but as a user, I would suspect something like

--8<---------------cut here---------------start------------->8---
(defun advice--add-function (where ref function props)
  (when (advice--member-p function (cdr (assq 'name props))
                          (gv-deref ref))
    (advice--remove-function .........))
  (setf (gv-deref ref)
        (advice--make where function (gv-deref ref) props)))
--8<---------------cut here---------------end--------------->8---

i.e., replacing the old code, instead of doing nothing.

Is the current behavior intended?  If not, can we change it?


Regards,

Michael.

bug#14317: 24.3.50; nadvice.el: named advices not upgradable

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> but as a user, I would suspect something like
>
> (defun advice--add-function (where ref function props)
>   (when (advice--member-p function (cdr (assq 'name props))
>                           (gv-deref ref))
>     (advice--remove-function .........))
>   (setf (gv-deref ref)
>         (advice--make where function (gv-deref ref) props)))
>
> i.e., replacing the old code, instead of doing nothing.

We should probably distinguish between named functions and
named advice.  I mean, it's probably stupid to remove and add
back an fboundp symbol - but you get the idea...


Regards,

Michael.

bug#14317: 24.3.50; nadvice.el: named advices not upgradable

[Stefan Monnier]

> if you have some code adding advices that can be identified (i.e.,
> adding named advices or fbound symbols, not anonymous functions),
> and you change this code defining the advice and re-evaluate, the change
> doesn't take effect (in contrast to the behavior of advice.el).

I think what you ask makes sense, yes.


        Stefan "no time to do it now, tho"

bug#14317: 24.3.50; nadvice.el: named advices not upgradable

[Stefan Monnier]

> Is the current behavior intended?  If not, can we change it?

Changed, thanks,


        Stefan