bug#14202: 24.3.50; add-function doesn't check properly for existing advices with same name prop

bug#14202: 24.3.50; add-function doesn't check properly for existing advices with same name prop

[Michael Heerdegen]

Hi,

recipe from emacs -Q:

Save the following code into a file:

--8<---------------cut here---------------start------------->8---
(require 'nadvice)

(require 'shell)

(eval-when-compile (require 'cl))

(advice-add 'shell-command-sentinel :around
            (lambda (_f process signal)
              "Handle output the way I want."
              (when (memq (process-status process) '(exit signal))
                (let ((messg (format "%s: %s"
                                     (caddr (process-command process))
                                     (substring signal 0 -1)))
                      (buf (process-buffer process)))
                  (when buf
                    (with-current-buffer buf
                      (if (> (point-max) (point-min))
                          (if (<= (+ 3 (count-lines (point-min) (point-max)))
                                  (if resize-mini-windows
                                      (cond ((floatp max-mini-window-height)
                                             (* (frame-height)
                                                max-mini-window-height))
                                            ((integerp max-mini-window-height)
                                             max-mini-window-height)
                                            (t
                                             1))
                                    1))
                              (message "%s" (concat messg ".\n" (propertize "Output" 'face 'underline)
                                                    " was:\n"
                                                    (with-current-buffer buf (buffer-string))) )
                            (message "%s"
                                     (concat messg "\n"
                                             (substitute-command-keys
                                              "Type \\[my-call-temp-command] to show output"))))
                        (message "%s" (concat messg " with no output"))))))))
            '((name . handle-output-more-comfortable)))
--8<---------------cut here---------------end--------------->8---

(The added function itself presumably doesn't make a difference here).
Note the specified `name' property.

Now,

1.  byte-compile the file
2.  load the elc
3.  Also load the uncompiled source

At the end, the advice was added twice:

C-h f shell-command-sentinel RET

==>

,----------------------------------------------------------------------
| shell-command-sentinel is a compiled Lisp function in `simple.el'.
| 
| (shell-command-sentinel PROCESS SIGNAL)
| 
| :around advice: handle-output-more-comfortable
| Handle output the way I want.
| :around advice: handle-output-more-comfortable
| Handle output the way I want.
`----------------------------------------------------------------------

Looks like `advice--member-p' is not DTRT:

When adding the advice the second time, this test

  (equal function (cdr (assq 'name (advice--props definition))))

seems to be "responsible".  But it returns nil, because `function' is
bound to a byte code function, while

  (cdr (assq 'name (advice--props definition)))

evals to the advice name (a symbol).


Thanks,

Michael.




In GNU Emacs 24.3.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2013-04-10 on dex, modified by Debian
 (emacs-snapshot package, version 2:20130410-1)
Windowing system distributor `The X.Org Foundation', version 11.0.11204000
System Description:	Debian GNU/Linux 7.0 (wheezy)

Configured using:
 `configure --build x86_64-linux-gnu --host x86_64-linux-gnu
 --prefix=/usr --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var --infodir=/usr/share/info --mandir=/usr/share/man
 --with-pop=yes
 --enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/24.3.50/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/24.3.50/site-lisp:/usr/share/emacs/site-lisp
 --without-compress-info --with-crt-dir=/usr/lib/x86_64-linux-gnu/
 --with-x=yes --with-x-toolkit=gtk3 --with-imagemagick=yes
 CFLAGS='-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2'
 CPPFLAGS='-D_FORTIFY_SOURCE=2' LDFLAGS='-g -Wl,--as-needed
 -znocombreloc''

bug#14202: 24.3.50; add-function doesn't check properly for existing advices with same name prop

[Stefan Monnier]

> Looks like `advice--member-p' is not DTRT:
> When adding the advice the second time, this test
>   (equal function (cdr (assq 'name (advice--props definition))))
> seems to be "responsible".  But it returns nil, because `function' is
> bound to a byte code function, while

Duh!  Indeed that code was confused.  Should be fixed now.
Thank you,


        Stefan