Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Adam Sjøgren]

  Hi.


I just installed a new emacs-snapshot package¹, and got an error on
startup, from these four lines in my .emacs.d/init.el:

  (defadvice gnus (before manual-fetch-before activate)
    (setq asjo-fetch-was-manual t))
  (defadvice gnus (after manual-fetch-after activate)
    (setq asjo-fetch-was-manual nil))

(which is part of a crude homebrewed mechanism to do different things
according to whether I manually fetch email/news in Gnus, or it is done
automatically by a gnus-demon.)

So I checked out the emacs repository and git bisect'ed the problem to:

  daa84a03e1a35c9d86865a39f7e2a768a1f43873 is the first bad commit
  commit daa84a03e1a35c9d86865a39f7e2a768a1f43873
  Author: Stefan Monnier <monnier@iro.umontreal.ca>
  Date:   Thu Nov 8 23:10:16 2012 -0500

      New property dynamic-docstring-function for docstrings.
      * src/doc.c (Fdocumentation): Handle new property
      dynamic-docstring-function to replace the old ad-advice-info.
      * lisp/emacs-lisp/advice.el: Use new dynamic docstrings.
      (ad-make-advised-definition-docstring, ad-advised-definition-p):
      Use dynamic-docstring-function instead of ad-advice-info.
      (ad--make-advised-docstring): New function extracted from
      ad-make-advised-docstring.
      (ad-make-advised-docstring): Use it.
      * lisp/progmodes/sql.el (sql--make-help-docstring): New function, extracted
      from sql-help.
      (sql-help): Use it with dynamic-docstring-function.

The error I get on startup when I run ./src/emacs built with that commit is:

  Warning (initialization): An error occurred while loading `/home/asjo/.emacs.d/init.el':

  Wrong type argument: symbolp, #[(&optional arg dont-connect slave) \305.\306.\307
  ^K^L#\211.)\207 [ad-return-value asjo-fetch-was-manual arg dont-connect slave nil t ad-Orig-gnus] 5 Advice doc string P]

If I run ./src/emacs --debug-init, I get this backtrace:

  Debugger entered--Lisp error: (wrong-type-argument symbolp #[(&optional arg dont-connect slave) "\305.\306.\307
  ^K^L#\211.)\207" [ad-return-value asjo-fetch-was-manual arg dont-connect slave nil t ad-Orig-gnus] 5 #("Advice doc string" 0 17 (dynamic-docstring-function ad--make-advised-docstring)) "P"])
    ad-real-orig-definition(#[(&optional arg dont-connect slave) "\305.\306.\307\n^K\f#\211.)\207" [ad-return-value asjo-fetch-was-manual arg dont-connect slave nil t ad-Orig-gnus] 5 #("Advice doc string" 0 17 (dynamic-docstring-function ad--make-advised-docstring)) "P"])
    ad--make-advised-docstring(#("Advice doc string" 0 17 (dynamic-docstring-function ad--make-advised-docstring)) #[(&optional arg dont-connect slave) "\305.\306.\307\n^K\f#\211.)\207" [ad-return-value asjo-fetch-was-manual arg dont-connect slave nil t ad-Orig-gnus] 5 #("Advice doc string" 0 17 (dynamic-docstring-function ad--make-advised-docstring)) "P"])
    ad-real-documentation(#[(&optional arg dont-connect slave) "\305.\306.\307\n^K\f#\211.)\207" [ad-return-value asjo-fetch-was-manual arg dont-connect slave nil t ad-Orig-gnus] 5 #("Advice doc string" 0 17 (dynamic-docstring-function ad--make-advised-docstring)) "P"] t)
    ad-docstring(#[(&optional arg dont-connect slave) "\305.\306.\307\n^K\f#\211.)\207" [ad-return-value asjo-fetch-was-manual arg dont-connect slave nil t ad-Orig-gnus] 5 #("Advice doc string" 0 17 (dynamic-docstring-function ad--make-advised-docstring)) "P"])
    ad-advised-definition-p(#[(&optional arg dont-connect slave) "\305.\306.\307\n^K\f#\211.)\207" [ad-return-value asjo-fetch-was-manual arg dont-connect slave nil t ad-Orig-gnus] 5 #("Advice doc string" 0 17 (dynamic-docstring-function ad--make-advised-docstring)) "P"])
    ad-handle-definition(gnus)
    ad-activate(gnus nil)
    (progn (ad-add-advice (quote gnus) (quote (manual-fetch-after nil t (advice lambda nil (setq asjo-fetch-was-manual nil)))) (quote after) (quote nil)) (ad-activate (quote gnus) nil) (quote gnus))
    eval-buffer(#<buffer  *load*> nil "/home/asjo/.emacs.d/init.el" nil t)  ; Reading at buffer position 2546
    load-with-code-conversion("/home/asjo/.emacs.d/init.el" "/home/asjo/.emacs.d/init.el" t t)
    load("/home/asjo/.emacs.d/init" t t)
    #[0 "^H\205\262.	\306=\203.^@\307^H\310Q\202;.	\311=\204.^@\307^H\312Q\202;.\313\307\314\315#\203*.\316\202;.\313\307\314\317#\203:.\320\nB.\321\202;.\316\322.\323.\322\211#\210^K\322=\203a.\324\325\326\307^H\327Q!\"\323.\322\211#\210^K\322=\203`.^A.\210^K\203\243.\330^K!\331\232\203\243.\332^K!\211\333P\334.!\203}.\211\202\210.\334.!\203\207.^A\202\210.\314\262.^A\203\241.\335.^K\"\203\237.\336\337.^K#\210\340\341!\210.^S\266.\f?\205\260.\314.\323\342\322\211#)\262.\207" [init-file-user system-type delayed-warnings-list user-init-file inhibit-default-init inhibit-startup-screen ms-dos "~" "/_emacs" windows-nt "/.emacs" directory-files nil "^\\.emacs\\(\\.elc?\\)?$" "~/.emacs" "^_emacs\\(\\.elc?\\)?$" (initialization "`_emacs' init file is deprecated, please use `.emacs'") "~/_emacs" t load expand-file-name "init" file-name-as-directory "/.emacs.d" file-name-extension "elc" file-name-sans-extension ".el" file-exists-p file-newer-than-file-p message "Warning: %s is newer than %s" sit-for 1 "default"] 7 "\n\n(fn)"]()
    command-line()
    normal-top-level()

It's probably something I'm doing wrong with the advice, as the scheme I
got going was added by trial-and-error, but I thought I'd report it
nonetheless.


  Best regards,

    Adam


¹ Graciously built by Julien Danjou at http://emacs.naquadah.org/

-- 
 "I threw myself out a 1000 times                             Adam Sjøgren
  To seperate body from mind                             asjo@koldfront.dk
  But you know I didn't give a damn
  I'm gonna be just who I am"

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

> I just installed a new emacs-snapshot package¹, and got an error on
> startup, from these four lines in my .emacs.d/init.el:

Better make it a bug report rather than send it to the discussion list.
And also I recommend you use the emacs-24 branch rather than the trunk,
so as to help debug the pretest of 24.3.


        Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Tim Cross]

On 10/11/12 12:55, Stefan Monnier wrote:
>> I just installed a new emacs-snapshot package¹, and got an error on
>> startup, from these four lines in my .emacs.d/init.el:
>
> Better make it a bug report rather than send it to the discussion list.
> And also I recommend you use the emacs-24 branch rather than the trunk,
> so as to help debug the pretest of 24.3.
>
>
>          Stefan
>

Just FYI, I encountered the same error with some of the emacspeak 
defadvice. Switching to the Emacs-24 branch and no error - all compiled 
fine.

Tim

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Ivan Kanis]

asjo@koldfront.dk (Adam Sjøgren) wrote:

> I just installed a new emacs-snapshot package¹, and got an error on
> startup, from these four lines in my .emacs.d/init.el:

I am getting the exact same error, same backtrace. I use the same hook
in gnus as Adam. 

Attempting to fix the issue I bootstrapped emacs. That usually fix
things, but no luck this time. I then recompiled all my personal files
with the new emacs since defadvice is a macro but I still get the error.

I have tried to reproduce with emacs -Q. I then byte compile a mock file
and load. Unfortunately I can't reproduce the bug :(

At this stage I am completely puzzled. I would like to give a recipe to
reproduce but can't think of anything. Any ideas on how I could do that?
-- 
Ivan Kanis

What we obtain too cheap, we esteem too lightly. It is dearness only
that gives everything its value.
    -- Thomas Paine

I am listening to "The Temptations - Mother Nature".

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Katsumi Yamaoka]

Ivan Kanis wrote:
> I am getting the exact same error, same backtrace. I use the same hook
> in gnus as Adam.

Sorry, this is a noise.

The immediate cause of the error we all got is that the function
`documentation' passes a function definition, not a function symbol,
to `ad--make-advised-docstring'.  I worked around it, it makes Emacs
barely work, though it's far from the fix:

ftp://ftp.jpl.org/pub/tmp/advice_el.diff

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

You might try your advice code with the new code in trunk.
There are significant changes in there, so I suspect the bug you suffered
from is gone, but I wouldn't be surprised if it is replaced by others.


        Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Katsumi Yamaoka]

Stefan Monnier wrote:
> You might try your advice code with the new code in trunk.
> There are significant changes in there, so I suspect the bug you suffered
> from is gone, but I wouldn't be surprised if it is replaced by others.

I built Emacs from scratch using the very fresh bzr source.
It looks to have been pretty improved except for at least one
problem: an elc module seems to need to be loaded before advising
a function that provides, i.e.:

This works:
(require 'gnus-art)
(defadvice gnus-article-bla-bla...)

This doesn't work:
(defadvice gnus-article-bla-bla...)

In the latter case I got the following error:

Debugger entered--Lisp error: (wrong-type-argument listp t)
  ad-parse-arglist(t)
  ad-map-arglists(t t)
  ad-make-advised-definition(gnus-article-prepare-display)
  ad-activate-advised-definition(gnus-article-prepare-display nil)
  ad-activate(gnus-article-prepare-display nil)
  (progn (ad-add-advice (quote gnus-article-prepare-display)...

It's easy for you to fix this, isn't it?  :)

The advice that causes this is:
(defadvice gnus-article-prepare-display (after delete-last-empty-lines
					       activate)
  "Delete last empty lines."
  (let ((limit (previous-char-property-change (goto-char (point-max)))))
    (while (progn
	     (forward-line -1)
	     (and (< limit (point)) (looking-at "[\t ]*$")))))
  (forward-line 1)
  (let ((inhibit-read-only t))
    (delete-region (point) (point-max))))

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Juanma Barranquero]

On Wed, Nov 14, 2012 at 12:18 AM, Katsumi Yamaoka <yamaoka@jpl.org> wrote:

> In the latter case I got the following error:
>
> Debugger entered--Lisp error: (wrong-type-argument listp t)
>   ad-parse-arglist(t)
>   ad-map-arglists(t t)
>   ad-make-advised-definition(gnus-article-prepare-display)
>   ad-activate-advised-definition(gnus-article-prepare-display nil)
>   ad-activate(gnus-article-prepare-display nil)
>   (progn (ad-add-advice (quote gnus-article-prepare-display)...

I'm getting this one in advices for functions that aren't yet loaded,
when the advice uses "activate". Easy to see here:

emacs -Q
(defadvice ada-mode (before my-ada activate) t)  <C-x C-e>

Which is perhaps a mistake, but it was previously silently ignored.

    Juanma

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Katsumi Yamaoka]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

Juanma Barranquero wrote:
> On Wed, Nov 14, 2012 at 12:18 AM, Katsumi Yamaoka <yamaoka@jpl.org> wrote:

>> In the latter case I got the following error:
>>
>> Debugger entered--Lisp error: (wrong-type-argument listp t)
>>   ad-parse-arglist(t)
>>   ad-map-arglists(t t)
>>   ad-make-advised-definition(gnus-article-prepare-display)
>>   ad-activate-advised-definition(gnus-article-prepare-display nil)
>>   ad-activate(gnus-article-prepare-display nil)
>>   (progn (ad-add-advice (quote gnus-article-prepare-display)...

> I'm getting this one in advices for functions that aren't yet loaded,
> when the advice uses "activate". Easy to see here:

> emacs -Q
> (defadvice ada-mode (before my-ada activate) t)  <C-x C-e>

> Which is perhaps a mistake, but it was previously silently ignored.

I'm not quite sure it is in a good manner but the patch for
advice.el in the attached diff makes it work in such cases.
The other, that is for nadvice.el, is for an advice of which
the interactive spec is a string, like this example described in
advice.el:

(defadvice find-file (before existing-files-only activate)
  "Find existing files only"
  (interactive "fFind file: "))

With these changes, so far I can play with Emacs as before.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 692 bytes --]

--- advice.el~	2012-11-13 22:00:19.237409000 +0000
+++ advice.el	2012-11-15 01:42:13.391473100 +0000
@@ -2599,3 +2599,3 @@
 	     ;; Construct the individual pieces that we need for assembly:
-	     (orig-arglist (ad-arglist origdef))
+	     (orig-arglist (and origdef (ad-arglist origdef)))
 	     (advised-arglist (or (ad-advised-arglist function)
--- nadvice.el~	2012-11-14 21:52:40.096113000 +0000
+++ nadvice.el	2012-11-15 01:42:13.391473100 +0000
@@ -131,3 +131,3 @@
   (let ((fspec (cadr (interactive-form function))))
-    (when (eq 'function (car fspec)) ;; Macroexpanded lambda?
+    (when (eq 'function (car-safe fspec)) ;; Macroexpanded lambda?
       (setq fspec (nth 1 fspec)))

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

> With these changes, so far I can play with Emacs as before.

Thanks, installed,


        Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Ivan Kanis]

Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> With these changes, so far I can play with Emacs as before.
>
> Thanks, installed,

I tried it this morning and it's fixed. Thank you Stefan.
-- 
Ivan Kanis
http://ivan.kanis.fr

The power of accurate observation is commonly called cynicism by those
who have not got it.
    -- George Bernard Shaw

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Katsumi Yamaoka]

>>> With these changes, so far I can play with Emacs as before.
>> Thanks, installed,
> I tried it this morning and it's fixed. Thank you Stefan.

Unfortunately called-interactively-p never returns non-nil while
performing an advised command interactively.

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

>>>> With these changes, so far I can play with Emacs as before.
>>> Thanks, installed,
>> I tried it this morning and it's fixed. Thank you Stefan.
> Unfortunately called-interactively-p never returns non-nil while
> performing an advised command interactively.

Yes, called-interactively-p (and interactivep, of course) are pretty
sensitive beasts.  The problem you mention was already present before,
tho it probably got a bit worse (it used to "only" affect uses of
called-interactively-p in the advised command, whereas it now also
affects uses in the advice themselves).


        Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Katsumi Yamaoka]

>> Unfortunately called-interactively-p never returns non-nil while
>> performing an advised command interactively.

> Yes, called-interactively-p (and interactivep, of course) are pretty
> sensitive beasts.  The problem you mention was already present before,
> tho it probably got a bit worse (it used to "only" affect uses of
> called-interactively-p in the advised command, whereas it now also
> affects uses in the advice themselves).

Ok.  It seems hard to me to fix, so I will live with this way
(for a while?).

(defvar my-called-interactively-p nil)

(defadvice called-interactively-p (around work-for-advised-commands activate)
  "Work for advised commands."
  (or my-called-interactively-p ad-do-it))

(defadvice interactive-p (around work-for-advised-commands activate)
  "Work for advised commands."
  (or my-called-interactively-p ad-do-it))

(defadvice command (around work-interactively (arg &optional arg EXTRA-ARG)
			   activate)
  "Work interactively."
  (interactive (list bla bla t))
  (let ((my-called-interactively-p EXTRA-ARG))
    ad-do-it))

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

>>> Unfortunately called-interactively-p never returns non-nil while
>>> performing an advised command interactively.
>> Yes, called-interactively-p (and interactivep, of course) are pretty
>> sensitive beasts.  The problem you mention was already present before,
>> tho it probably got a bit worse (it used to "only" affect uses of
>> called-interactively-p in the advised command, whereas it now also
>> affects uses in the advice themselves).
> Ok.  It seems hard to me to fix, so I will live with this way
> (for a while?).

I have some preliminary code to try and fix it, but it only works in
some cases.  `called-interactively-p' is also broken in some
lexical-binding cases (e.g. using called-interactively-p within
a `condition-case' or a `catch').

It is a functionality that is fundamentally problematic.  E.g. (to
revisit a recent claim that funcall+lambda should be identical to let)
if called-interactively-p returns t in
(let ((foo bar)) (called-interactively-p)), should it also return t in
(funcall (lambda (foo) (called-interactively-p)) bar) ?

But if you can point me to concrete examples that need to work (and/or
that used to work), it would be very helpful.


        Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Juanma Barranquero]

On Fri, Nov 16, 2012 at 3:16 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> But if you can point me to concrete examples that need to work (and/or
> that used to work), it would be very helpful.

As an example of an advice from my .emacs that used to work:

(defadvice delete-indentation (before my-delete-indentation activate compile)
  (when (called-interactively-p 'any)
    (ad-set-arg 0 (not (ad-get-arg 0)))))

    Juanma

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

>> But if you can point me to concrete examples that need to work (and/or
>> that used to work), it would be very helpful.
> As an example of an advice from my .emacs that used to work:
> (defadvice delete-indentation (before my-delete-indentation activate compile)
>   (when (called-interactively-p 'any)
>     (ad-set-arg 0 (not (ad-get-arg 0)))))

OK, that's a good start.

My current code can handle uses of called-interactively-p in
after/before advice but not in around advice.  Problem is: all advices
defined with `defadvice' are implemented as (advice-add foo :around ...)
and indeed your "before" advice cannot be implemented as a (advice-add
foo :before ...) because it modifies arguments.

Hmm... maybe advice-add's before and after advices should be different.
Instead of before being like
   (lambda (&rest r) (apply FUNCTION r) (apply OLDFUN r))
it should maybe be like
   (lambda (&rest r) (apply OLDFUN (apply FUNCTION r)))
Or maybe this should be a new WHERE, which we could call `:filter-args',
and we could have a corresponding `:filter-return'.

In that case my code would handle:

   (advice-add 'delete-indentation :filter-args
               (lambda (arg &rest args)
                 (cons (or (called-interactively-p 'any) arg) args)))

-- Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Juanma Barranquero]

On Fri, Nov 16, 2012 at 6:25 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> Hmm... maybe advice-add's before and after advices should be different.
> Instead of before being like
>    (lambda (&rest r) (apply FUNCTION r) (apply OLDFUN r))
> it should maybe be like
>    (lambda (&rest r) (apply OLDFUN (apply FUNCTION r)))
> Or maybe this should be a new WHERE, which we could call `:filter-args',
> and we could have a corresponding `:filter-return'.

Well, having new WHEREs seems more flexible, assuming that
:filter-args is indeed the right thing to do for all "before"
defadvices.

    Juanma

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

> My current code can handle uses of called-interactively-p in
> after/before advice but not in around advice.  Problem is: all advices
> defined with `defadvice' are implemented as (advice-add foo :around ...)
> and indeed your "before" advice cannot be implemented as a (advice-add
> foo :before ...) because it modifies arguments.

Sorry, I was confused.  My code can handle your example just fine.
The problem comes when an around advice is added after your advice, at
which point your advice won't work any more (not because your is an
around advice, but because it's "hidden" by another around advice).
So a :filter-args thingy wouldn't make any difference in this respect.


        Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Juanma Barranquero]

On Fri, Nov 16, 2012 at 7:11 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> My code can handle your example just fine.
> The problem comes when an around advice is added after your advice, at
> which point your advice won't work any more (not because your is an
> around advice, but because it's "hidden" by another around advice).

Which around advice is being added after my advice?

    Juanma

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

>> My code can handle your example just fine.
>> The problem comes when an around advice is added after your advice, at
>> which point your advice won't work any more (not because your is an
>> around advice, but because it's "hidden" by another around advice).
> Which around advice is being added after my advice?

I don't know that there is any, but there could be.  In any case, you're
not using my code yet.  If you want to try it out, see below.
I think I got it to handle around advice now as well.


        Stefan


(eval-when-compile                      ;Poor man's macrolet.
  (defmacro interactive--get-next-frame ()
    `(progn
       (setq frame nextframe)
       (setq nextframe (backtrace-frame (setq i (1+ i))))
       ;; (message "Frame %d = %S" i nextframe)
       )))
(defun called-interactively-p (&optional kind)
  "Return t if the containing function was called by `call-interactively'.
If KIND is `interactive', then only return t if the call was made
interactively by the user, i.e. not in `noninteractive' mode nor
when `executing-kbd-macro'.
If KIND is `any', on the other hand, it will return t for any kind of
interactive call, including being called as the binding of a key or
from a keyboard macro, even in `noninteractive' mode.

This function is very brittle, it may fail to return the intended result when 
the code is debugged, advised, or instrumented in some form.  Some macros and
special forms (such as `condition-case') may also sometimes wrap their bodies
in a `lambda', so any call to `called-interactively-p' from those bodies will 
indicate whether that lambda (rather than the surrounding function) was called
interactively.

Instead of using this function, it is cleaner and more reliable to give your
function an extra optional argument whose `interactive' spec specifies
non-nil unconditionally (\"p\" is a good way to do this), or via
\(not (or executing-kbd-macro noninteractive)).

The only known proper use of `interactive' for KIND is in deciding
whether to display a helpful message, or how to display it.  If you're
thinking of using it for any other purpose, it is quite likely that
you're making a mistake.  Think: what do you want to do when the
command is called from a keyboard macro?"
  (declare (advertised-calling-convention (kind) "23.1"))
  (when (not (and (eq kind 'interactive)
                  (or executing-kbd-macro noninteractive)))
    (let ((i 0)
          frame nextframe)
      (interactive--get-next-frame) ;; Get the first frame.
      (while
          ;; FIXME: The edebug and advice handling should be made modular and
          ;; provided directly by edebug.el and nadvice.el.
          (progn
            (interactive--get-next-frame)
            ;; `pcase' would be a fairly good fit here, but it sometimes moves
            ;; branches within local functions, which then messes up the
            ;; `backtrace-frame' data we get,
            (or
             ;; First comes the binding for to sm-called-interactively-p and
             ;; maybe also for interactive-p.
             (memq (nth 1 frame) '(interactive-p called-interactively-p))
             ;; Then may come special forms (for non-compiled code).
             (null (car frame))
             ;; In byte-compiled code, subexpressions of things like
             ;; condition-case are wrapped in a separate bytecode chunk.
             ;; FIXME: For lexical-binding code, this is much worse, because
             ;; the frames look like "byte-code -> funcall -> #[...]", which
             ;; is not a reliable signature.
             (eq (nth 1 frame) 'byte-code)
             ;; When edebugging a function, some of the sub-expressions are
             ;; wrapped in (lambda () ..).
             (when (and (eq (car-safe (nth 1 frame)) 'lambda)
                        (eq (nth 1 (nth 1 frame)) '())
                        (eq (nth 1 nextframe) 'edebug-enter))
               (interactive--get-next-frame)
               ;; `edebug-enter' calls itself on its first invocation.
               (when (eq (nth 1 nextframe) 'edebug-enter)
                 (interactive--get-next-frame))
               t)
             ;; When the code is advised, we also have a problem.
             ;; FIXME: The Major Ugly Hack below will not handle calls to
             ;; called-interactively-p done from the advised function if the
             ;; deepest advice is an around advice!
             ;; In other cases (calls from an advice or calls from the advised
             ;; function when the deepest advice is not an around advice), it
             ;; should hopefully get it right.
             (when (and (eq (nth 1 nextframe) 'apply) (fboundp 'advice--p)
                        (advice--p (indirect-function
                                    (nth 1 (backtrace-frame (1+ i))))))
               (interactive--get-next-frame)
               (interactive--get-next-frame)
               ;; If we now have the symbol, this was the head advice and
               ;; we're done.
               (while (advice--p (nth 1 frame))
                 ;; This was an inner advice called from some earlier advice.
                 ;; The stack frames look different depending on the particular
                 ;; kind of the earlier advice.
                 (if (and (eq (nth 1 nextframe) 'apply)
                          (advice--p (indirect-function
                                      (nth 1 (backtrace-frame (1+ i))))))
                     ;; The earlier advice was something like a before/after
                     ;; advice where the "next" code is called directly by the
                     ;; advice--p object.
                     (progn
                       (interactive--get-next-frame)
                       (interactive--get-next-frame))
                   ;; It's apparently an around advice, where the "next" is
                   ;; called by the body of the advice in any way it sees fit,
                   ;; so we need to skip the frames of that body.
                   (let ((inneradvice (nth 1 frame))
                         (j i) framej)
                     (while
                         (progn
                           (setq framej (backtrace-frame (setq j (1+ j))))
                           (not (and (eq (nth 1 framej) 'apply)
                                     (eq (nth 3 framej) inneradvice)))))
                     (setq i j)
                     (interactive--get-next-frame)
                     (interactive--get-next-frame)))))
             )))
      ;; Now `frame' should be "the function from which we were called".
      (pcase (cons frame nextframe)
        ;; No subr calls `interactive-p', so we can rule that out.
        (`((,_ ,(pred (lambda (f) (subrp (indirect-function f)))) . ,_) . ,_) nil)
        ;; Somehow, I sometimes got `command-execute' rather than
        ;; `call-interactively' on my stacktrace !?
        ;;(`(,_ . (t command-execute . ,_)) t)
        (`(,_ . (t call-interactively . ,_)) t)))))

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Juanma Barranquero]

On Fri, Nov 16, 2012 at 9:11 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> In any case, you're
> not using my code yet.  If you want to try it out, see below.

It works if compiled, still fails when interpreted. Is that expected?

    Juanma

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Stefan Monnier]

>> In any case, you're not using my code yet.  If you want to try it
>> out, see below.
> It works if compiled, still fails when interpreted. Is that expected?

If you mean that called-interactively-p needs to be compiled, then yes,
it's expected.  If you mean that the caller of called-interactively-p
needs to be compiled, then no, I didn't know.


        Stefan

Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500)

[Juanma Barranquero]

On Mon, Nov 19, 2012 at 2:29 AM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:

> If you mean that called-interactively-p needs to be compiled, then yes,
> it's expected.

Yes.

> If you mean that the caller of called-interactively-p
> needs to be compiled

No, it works from my .emacs, which is currently not compiled.

    Juanma