* Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) @ 2012-11-09 23:47 Adam Sjøgren 2012-11-10 1:55 ` Stefan Monnier 2012-11-12 10:36 ` Ivan Kanis 0 siblings, 2 replies; 24+ messages in thread From: Adam Sjøgren @ 2012-11-09 23:47 UTC (permalink / raw) To: emacs-devel 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" ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-09 23:47 Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) Adam Sjøgren @ 2012-11-10 1:55 ` Stefan Monnier 2012-11-11 21:24 ` Tim Cross 2012-11-12 10:36 ` Ivan Kanis 1 sibling, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-10 1:55 UTC (permalink / raw) To: Adam Sjøgren; +Cc: emacs-devel > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-10 1:55 ` Stefan Monnier @ 2012-11-11 21:24 ` Tim Cross 0 siblings, 0 replies; 24+ messages in thread From: Tim Cross @ 2012-11-11 21:24 UTC (permalink / raw) To: emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-09 23:47 Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) Adam Sjøgren 2012-11-10 1:55 ` Stefan Monnier @ 2012-11-12 10:36 ` Ivan Kanis 2012-11-13 11:09 ` Katsumi Yamaoka 1 sibling, 1 reply; 24+ messages in thread From: Ivan Kanis @ 2012-11-12 10:36 UTC (permalink / raw) To: emacs-devel; +Cc: Adam Sjøgren 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". ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-12 10:36 ` Ivan Kanis @ 2012-11-13 11:09 ` Katsumi Yamaoka 2012-11-13 14:34 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Katsumi Yamaoka @ 2012-11-13 11:09 UTC (permalink / raw) To: ivan.kanis; +Cc: asjo, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-13 11:09 ` Katsumi Yamaoka @ 2012-11-13 14:34 ` Stefan Monnier 2012-11-13 23:18 ` Katsumi Yamaoka 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-13 14:34 UTC (permalink / raw) To: Katsumi Yamaoka; +Cc: asjo, ivan.kanis, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-13 14:34 ` Stefan Monnier @ 2012-11-13 23:18 ` Katsumi Yamaoka 2012-11-14 16:19 ` Juanma Barranquero 0 siblings, 1 reply; 24+ messages in thread From: Katsumi Yamaoka @ 2012-11-13 23:18 UTC (permalink / raw) To: monnier; +Cc: asjo, ivan.kanis, emacs-devel 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)))) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-13 23:18 ` Katsumi Yamaoka @ 2012-11-14 16:19 ` Juanma Barranquero 2012-11-15 2:08 ` Katsumi Yamaoka 0 siblings, 1 reply; 24+ messages in thread From: Juanma Barranquero @ 2012-11-14 16:19 UTC (permalink / raw) To: Katsumi Yamaoka; +Cc: asjo, ivan.kanis, monnier, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-14 16:19 ` Juanma Barranquero @ 2012-11-15 2:08 ` Katsumi Yamaoka 2012-11-15 3:30 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Katsumi Yamaoka @ 2012-11-15 2:08 UTC (permalink / raw) To: lekktu; +Cc: asjo, ivan.kanis, monnier, emacs-devel [-- 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))) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-15 2:08 ` Katsumi Yamaoka @ 2012-11-15 3:30 ` Stefan Monnier 2012-11-15 9:00 ` Ivan Kanis 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-15 3:30 UTC (permalink / raw) To: Katsumi Yamaoka; +Cc: lekktu, asjo, ivan.kanis, emacs-devel > With these changes, so far I can play with Emacs as before. Thanks, installed, Stefan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-15 3:30 ` Stefan Monnier @ 2012-11-15 9:00 ` Ivan Kanis 2012-11-15 10:55 ` Katsumi Yamaoka 0 siblings, 1 reply; 24+ messages in thread From: Ivan Kanis @ 2012-11-15 9:00 UTC (permalink / raw) To: Stefan Monnier; +Cc: lekktu, Katsumi Yamaoka, asjo, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-15 9:00 ` Ivan Kanis @ 2012-11-15 10:55 ` Katsumi Yamaoka 2012-11-15 14:39 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Katsumi Yamaoka @ 2012-11-15 10:55 UTC (permalink / raw) To: ivan.kanis; +Cc: lekktu, asjo, monnier, emacs-devel >>> 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. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-15 10:55 ` Katsumi Yamaoka @ 2012-11-15 14:39 ` Stefan Monnier 2012-11-15 23:01 ` Katsumi Yamaoka 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-15 14:39 UTC (permalink / raw) To: Katsumi Yamaoka; +Cc: lekktu, asjo, ivan.kanis, emacs-devel >>>> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-15 14:39 ` Stefan Monnier @ 2012-11-15 23:01 ` Katsumi Yamaoka 2012-11-16 14:16 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Katsumi Yamaoka @ 2012-11-15 23:01 UTC (permalink / raw) To: monnier; +Cc: lekktu, asjo, ivan.kanis, emacs-devel >> 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)) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-15 23:01 ` Katsumi Yamaoka @ 2012-11-16 14:16 ` Stefan Monnier 2012-11-16 16:33 ` Juanma Barranquero 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-16 14:16 UTC (permalink / raw) To: Katsumi Yamaoka; +Cc: lekktu, asjo, ivan.kanis, emacs-devel >>> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 14:16 ` Stefan Monnier @ 2012-11-16 16:33 ` Juanma Barranquero 2012-11-16 17:25 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Juanma Barranquero @ 2012-11-16 16:33 UTC (permalink / raw) To: Stefan Monnier; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 16:33 ` Juanma Barranquero @ 2012-11-16 17:25 ` Stefan Monnier 2012-11-16 17:36 ` Juanma Barranquero 2012-11-16 18:11 ` Stefan Monnier 0 siblings, 2 replies; 24+ messages in thread From: Stefan Monnier @ 2012-11-16 17:25 UTC (permalink / raw) To: Juanma Barranquero; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 17:25 ` Stefan Monnier @ 2012-11-16 17:36 ` Juanma Barranquero 2012-11-16 18:11 ` Stefan Monnier 1 sibling, 0 replies; 24+ messages in thread From: Juanma Barranquero @ 2012-11-16 17:36 UTC (permalink / raw) To: Stefan Monnier; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 17:25 ` Stefan Monnier 2012-11-16 17:36 ` Juanma Barranquero @ 2012-11-16 18:11 ` Stefan Monnier 2012-11-16 18:17 ` Juanma Barranquero 1 sibling, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-16 18:11 UTC (permalink / raw) To: Juanma Barranquero; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel > 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 18:11 ` Stefan Monnier @ 2012-11-16 18:17 ` Juanma Barranquero 2012-11-16 20:11 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Juanma Barranquero @ 2012-11-16 18:17 UTC (permalink / raw) To: Stefan Monnier; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 18:17 ` Juanma Barranquero @ 2012-11-16 20:11 ` Stefan Monnier 2012-11-16 23:52 ` Juanma Barranquero 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-16 20:11 UTC (permalink / raw) To: Juanma Barranquero; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel >> 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))))) ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 20:11 ` Stefan Monnier @ 2012-11-16 23:52 ` Juanma Barranquero 2012-11-19 1:29 ` Stefan Monnier 0 siblings, 1 reply; 24+ messages in thread From: Juanma Barranquero @ 2012-11-16 23:52 UTC (permalink / raw) To: Stefan Monnier; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-16 23:52 ` Juanma Barranquero @ 2012-11-19 1:29 ` Stefan Monnier 2012-11-19 2:36 ` Juanma Barranquero 0 siblings, 1 reply; 24+ messages in thread From: Stefan Monnier @ 2012-11-19 1:29 UTC (permalink / raw) To: Juanma Barranquero; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel >> 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) 2012-11-19 1:29 ` Stefan Monnier @ 2012-11-19 2:36 ` Juanma Barranquero 0 siblings, 0 replies; 24+ messages in thread From: Juanma Barranquero @ 2012-11-19 2:36 UTC (permalink / raw) To: Stefan Monnier; +Cc: Katsumi Yamaoka, ivan.kanis, asjo, emacs-devel 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 ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2012-11-19 2:36 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-09 23:47 Simple defadvice's stopped working (commit daa84a03, Thu Nov 8 23:10:16 2012 -0500) Adam Sjøgren 2012-11-10 1:55 ` Stefan Monnier 2012-11-11 21:24 ` Tim Cross 2012-11-12 10:36 ` Ivan Kanis 2012-11-13 11:09 ` Katsumi Yamaoka 2012-11-13 14:34 ` Stefan Monnier 2012-11-13 23:18 ` Katsumi Yamaoka 2012-11-14 16:19 ` Juanma Barranquero 2012-11-15 2:08 ` Katsumi Yamaoka 2012-11-15 3:30 ` Stefan Monnier 2012-11-15 9:00 ` Ivan Kanis 2012-11-15 10:55 ` Katsumi Yamaoka 2012-11-15 14:39 ` Stefan Monnier 2012-11-15 23:01 ` Katsumi Yamaoka 2012-11-16 14:16 ` Stefan Monnier 2012-11-16 16:33 ` Juanma Barranquero 2012-11-16 17:25 ` Stefan Monnier 2012-11-16 17:36 ` Juanma Barranquero 2012-11-16 18:11 ` Stefan Monnier 2012-11-16 18:17 ` Juanma Barranquero 2012-11-16 20:11 ` Stefan Monnier 2012-11-16 23:52 ` Juanma Barranquero 2012-11-19 1:29 ` Stefan Monnier 2012-11-19 2:36 ` Juanma Barranquero
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.