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

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 14202 in the body.
You can then email your comments to 14202 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: Michael Heerdegen <michael_heerdegen <at> web.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50;
	add-function doesn't check properly for existing advices with same
	name prop
Date: Sun, 14 Apr 2013 15:18:13 +0200

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''

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Michael Heerdegen <michael_heerdegen <at> web.de>
Cc: 14202-done <at> debbugs.gnu.org
Subject: Re: bug#14202: 24.3.50;
	add-function doesn't check properly for existing advices with same
	name prop
Date: Mon, 15 Apr 2013 11:07:26 -0400

> 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

Previous Next

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