unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Ryan <rct@thompsonclan.org>
Cc: 3984@debbugs.gnu.org
Subject: bug#3984:
Date: Fri, 20 Sep 2013 15:59:47 -0400	[thread overview]
Message-ID: <jwv1u4je0hd.fsf-monnier+emacsbugs@gnu.org> (raw)
In-Reply-To: <523C7CDA.6040308@thompsonclan.org> (Ryan's message of "Fri, 20 Sep 2013 09:50:34 -0700")

> Oh, that's actually not necessary. I put it in so I could re-run the test
> without restarting emacs or manually resetting the advice, and I forgot to
> remove it.

OK, removed.

>> Since it's only used once, I don't think it's worth defining a macro for it.
> Probably true. I wasn't sure how much I would end up using it, and it seemed
> like a potentially useful macro to have lying around when testing advice
> that might break core functions of emacs.

Makes sense as well, but let's postpone putting it in until we do end
up using it repeatedly.

I installed some tests based on your patch, as well as my quick fix in
subr.el.

> That would certainly work, assuming that subr.el is always loaded before
> nadvice.el (so call-interactively could not possibly be advised yet), which
> seems likely to be true since subr.el contains so many core functions.

Since subr.el is both preloaded and loaded very early, that should be
safe, yes.

> Since this bug report is just about advising call-interactively, which we
> now have a fix for, should I open a separate bug about the around advice?

Sure.  Note also that in the tests I installed, I added one that
demonstrates a problem with :filter-args as well (because :filter-args
uses `funcall' rather than `apply', so it doesn't match the pattern
expected by advice--called-interactively-skip).

By the way, I don't see your name in our copyright assignment database.
I installed your tests under "tiny change", but if you intend to
contribute more, we'd need you to sign some paperwork.  Would you be
willing to do that?


        Stefan





  reply	other threads:[~2013-09-20 19:59 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-30 22:37 bug#3984: 23.0.96; defadvice of call-interactively defeats interactive-p Drew Adams
2009-07-31  1:58 ` Stefan Monnier
2009-07-31 14:19   ` Drew Adams
2009-07-31 19:31     ` Stefan Monnier
2009-07-31 20:04       ` Drew Adams
2011-10-10  6:00 ` Kai Tetzlaff
2011-10-11 14:26   ` Drew Adams
2011-10-11 15:46     ` Stefan Monnier
2011-10-11 16:05       ` Drew Adams
2013-09-10 20:29 ` Christopher Wellons
2013-09-11  0:29   ` Stefan Monnier
2013-09-13  8:56 ` bug#3984: Fix for #3984 Ryan
2013-09-13 13:18   ` Stefan Monnier
2013-09-13 18:30     ` Ryan
2013-09-13 19:27       ` Ryan
2013-09-13 21:02         ` Stefan Monnier
2013-09-17  3:18           ` Ryan
2013-09-17 13:10             ` Stefan Monnier
2013-09-17 17:22               ` bug#3984: Ryan
2013-09-18  1:46                 ` bug#3984: Stefan Monnier
2013-09-18 23:30                   ` bug#3984: Ryan
2013-09-19  0:47                     ` bug#3984: Ryan
2013-09-19  3:38                       ` bug#3984: Stefan Monnier
2013-09-19  8:06                         ` bug#3984: Ryan
2013-09-19 19:23                           ` bug#3984: Ryan
2013-09-19 20:59                             ` bug#3984: Stefan Monnier
2013-09-19 21:59                             ` bug#3984: Ryan
2013-09-20  4:23                               ` bug#3984: Ryan
2013-09-20  4:58                                 ` bug#3984: Fix case where call-interactively is advised Ryan
2013-09-20  5:03                                   ` bug#3984: Ryan
2013-09-20 14:35                                 ` bug#3984: Stefan Monnier
2013-09-20 16:54                                   ` bug#3984: Ryan
2013-09-20 16:56                                     ` bug#3984: Ryan
2013-09-20 14:54                               ` bug#3984: Stefan Monnier
2013-09-20 16:50                                 ` bug#3984: Ryan
2013-09-20 19:59                                   ` Stefan Monnier [this message]
2013-09-13 10:24 ` bug#3984: bug#123: Potential fix Ryan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=jwv1u4je0hd.fsf-monnier+emacsbugs@gnu.org \
    --to=monnier@iro.umontreal.ca \
    --cc=3984@debbugs.gnu.org \
    --cc=rct@thompsonclan.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).