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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: michael_heerdegen <at> web.de
Subject: bug#14202: closed (Re: bug#14202: 24.3.50; add-function doesn't
 check properly for existing advices with same name prop)
Date: Mon, 15 Apr 2013 15:12:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

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

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 14202 <at> debbugs.gnu.org.

-- 
14202: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14202
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

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

[Message part 3 (message/rfc822, inline)]

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

Previous Next

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