* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive @ 2014-09-03 20:01 Tom Tromey 2014-09-03 20:42 ` Michael Heerdegen 0 siblings, 1 reply; 10+ messages in thread From: Tom Tromey @ 2014-09-03 20:01 UTC (permalink / raw) To: 18399 I wrote this function to advise vc-dir, to make it default to starting at the project root. It's largely copied from vc-dir, then slightly hacked: (defun tromey-vc-dir (dir &optional backend) (interactive (list ;; When you hit C-x v d in a visited VC file, ;; the *vc-dir* buffer visits the directory under its truename; ;; therefore it makes sense to always do that. ;; Otherwise if you do C-x v d -> C-x C-f -> C-c v d ;; you may get a new *vc-dir* buffer, different from the original (let ((def-dir (or (if (fboundp 'vc-root-dir) (vc-root-dir)) default-directory))) (file-truename (read-directory-name "VC status for directory: " def-dir def-dir t nil))) (if current-prefix-arg (intern (completing-read "Use VC backend: " (mapcar (lambda (b) (list (symbol-name b))) vc-handled-backends) nil t nil nil))))) (list dir backend)) (require 'vc-dir) (advice-add #'vc-dir :filter-args #'tromey-vc-dir) If I now use M-x vc-dir, I get an error: vc-responsible-backend: Wrong type argument: stringp, ("/home/tromey/Emacs/trunk/" nil) However, if I invoke M-x tromey-vc-dir directly (and hack in a little bit of code so I can easily see the result), it works properly. Using that hack and then invoking vc-dir shows that the result of tromey-vc-dir in the advised-and-interactive case is: (("/home/tromey/Emacs/trunk/" nil) nil) ... which is wrong. So I think there must be a bug with how nadvice handles interactive specs, either generally or with :filter-args. In GNU Emacs 24.4.50.4 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.9) of 2014-08-29 on bapiya Repository revision: 117768 dmantipov@yandex.ru-20140829122330-7g7qcbxsy64afin0 Windowing system distributor `Fedora Project', version 11.0.11404000 Configured using: `configure --prefix=/home/tromey/Emacs/install' Configured features: XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS NOTIFY LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB Important settings: value of $LANG: en_US.UTF-8 value of $XMODIFIERS: @im=none locale-coding-system: utf-8-unix Major mode: Lisp Interaction Minor modes in effect: diff-auto-refine-mode: t shell-dirtrack-mode: t helm-match-plugin-mode: t helm-occur-match-plugin-mode: t eldoc-mode: t which-function-mode: t global-auto-revert-mode: t erc-services-mode: t erc-list-mode: t erc-menu-mode: t erc-autojoin-mode: t erc-ring-mode: t erc-networks-mode: t erc-pcomplete-mode: t erc-track-mode: t erc-match-mode: t erc-button-mode: t erc-fill-mode: t erc-stamp-mode: t erc-netsplit-mode: t erc-irccontrols-mode: t erc-noncommands-mode: t erc-move-to-prompt-mode: t erc-readonly-mode: t savehist-mode: t tooltip-mode: t electric-indent-mode: t mouse-wheel-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t column-number-mode: t line-number-mode: t auto-fill-function: do-auto-fill Recent input: - f u n c i t o n SPC ' C-u C-b C-b C-t C-e ) C-j C-x o C-g C-p C-e C-j C-x o C-g C-p C-e C-j C-f C-f C-] C-] C-p C-p C-p C-e C-j C-] C-z n C-x 1 C-s a r i t y C-s C-x C-f <M-backspace> <M-backspace> <M-backspace> <M-backspace> <M-backspace> <M-backspace> <M-backspace> <M-backspace> <M-backspace> t r <tab> l i s <tab> e m a c s - l i s <tab> n a d <tab> <return> M-x l g r e p <return> a r i t y <return> <return> <return> C-x o C-x 1 C-u C-u C-n C-u C-n C-u C-n C-c C-c C-x 1 M-< C-x k <return> C-u C-p C-p C-p C-p C-c C-c C-x 1 C-l C-v C-l C-s b y t e - C-w C-w C-r C-r C-r C-a C-u C-u C-n C-l C-z o C-p M-d M-d C-e <backspace> C-j C-u C-u C-p C-M-f C-M-b M-f C-M-f C-M-f C-M-b C-M-f C-M-f C-M-f C-M-f C-M-b C-M-b C-M-b C-M-b C-M-b C-a C-s i n t e r a c t i v e C-g C-g C-g C-s d i r C-a M-v C-z n C-x b . e m <tab> <return> C-x v v C-u C-n C-u C-n M-g d i r <return> d e f - d i r <return> y n n n n n y y q C-u C-p C-u C-p C-p C-p TAB C-n TAB C-n TAB C-n TAB C-x C-s C-x v v r e n a m e SPC v a r i a b l e SPC i n SPC t r o m e y - v - <backspace> c - d i r C-c C-c C-z n M-x r e p o r t - e m <tab> <return> Recent messages: Mark saved where search started Checking out /home/tromey/Emacs/COPY/.emacs...done Mark set Replaced 3 occurrences Saving file /home/tromey/Emacs/COPY/.emacs... Wrote /home/tromey/Emacs/COPY/.emacs Mark set Press C-c C-c when you are done editing. Enter a change comment. Type C-c C-c when done Checking in /home/tromey/Emacs/COPY/.emacs...done Load-path shadows: /home/tromey/.emacs.d/elpa/css-mode-1.0/css-mode hides /home/tromey/Emacs/install/share/emacs/24.4.50/lisp/textmodes/css-mode /home/tromey/.emacs.d/elpa/bubbles-0.5/bubbles hides /home/tromey/Emacs/install/share/emacs/24.4.50/lisp/play/bubbles Features: (emacsbug log-edit pcvs-util smerge-mode bug-reference goto-addr image-file dabbrev bbdb-sc supercite regi gnus-draft xscheme unsafep trace testcover shadow scheme re-builder profiler pcase inf-lisp ielm ert debug elp edebug cl-indent checkdoc cus-edit copyright gnus-fun mailalias mail-hist nnir shr-color color diff-mode easy-mmode idutils derived cc-langs cc-mode cc-fonts cc-guess cc-menus cc-cmds ido helm-mode helm-files rx image-dired tramp tramp-compat tramp-loaddefs trampver shell dired-x dired-aux ffap helm-buffers helm-elscreen helm-tags helm-bookmark helm-adaptive helm-info helm-net helm-plugin bookmark helm-locate helm-help helm-match-plugin helm-grep helm-regexp helm-external helm-utils helm cl-macs eieio-opt speedbar sb-image ezimage dframe strokes help-mode shr flow-fill mm-archive gnus-html browse-url xml url-cache mm-url url url-proxy url-privacy url-expand url-methods url-history url-cookie url-domsuf url-util url-parse url-vars bbdb-gui bbdb-hooks mule-util sort smiley gnus-cite gnus-async gnus-bcklg qp gnus-ml disp-table gnus-topic nndraft nnmh nnfolder utf-7 bbdb-gnus bbdb-snarf mail-extr bbdb-com warnings cl gv gnutls network-stream starttls gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg nntp gnus-cache gnus-registry registry eieio-base gnus-art mm-uu mml2015 epg-config mm-view mml-smime smime dig mailcap gnus-sum gnus-group gnus-undo smtpmail sendmail gnus-start gnus-cloud nnimap nnmail mail-source tls utf7 netrc nnoo parse-time gnus-spec gnus-int gnus-range message dired rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils mailheader gnus-win gnus gnus-ems nnheader mail-utils misearch multi-isearch jka-compr add-log vc-arch vc-mtn vc-hg vc-git vc-bzr vc-sccs vc-svn vc-cvs vc-rcs tcl flyspell ispell eldoc diminish projectile edmacro kmacro pkg-info find-func lisp-mnt epl grep compile dash s appt diary-lib diary-loaddefs cal-menu calendar cal-loaddefs which-func imenu minimap autorevert filenotify cus-start cus-load status erc-services erc-list erc-menu erc-join erc-ring erc-networks erc-pcomplete pcomplete erc-track erc-match erc-button wid-edit cl-loaddefs cl-lib erc-fill erc-stamp erc-netsplit erc-goodies erc erc-backend erc-compat format-spec auth-source eieio byte-opt bytecomp byte-compile cconv eieio-core gnus-util mm-util mail-prsvr password-cache thingatpt pp advice help-fns vc-dir ewoc vc vc-dispatcher cc-styles cc-align cc-engine cc-vars cc-defs bbdb timezone ange-ftp comint ansi-color ring server savehist dwarf-mode-autoloads gdb-shell-autoloads jabber-autoloads lisppaste-autoloads pydoc-info-autoloads info-look info easymenu weblogger-autoloads package bbdb-autoloads time-date tooltip electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode register page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote make-network-process dbusbind gfilenotify dynamic-setting system-font-setting font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs) Memory information: ((conses 16 765705 79812) (symbols 48 98987 0) (miscs 40 29965 3141) (strings 32 306257 26993) (string-bytes 1 6729704) (vectors 16 94726) (vector-slots 8 2050490 133815) (floats 8 591 1493) (intervals 56 28928 314) (buffers 976 110) (heap 1024 200585 13852)) Tom ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-03 20:01 bug#18399: 24.4.50; nadvice :filter-args -vs- interactive Tom Tromey @ 2014-09-03 20:42 ` Michael Heerdegen 2014-09-03 22:27 ` Tom Tromey 0 siblings, 1 reply; 10+ messages in thread From: Michael Heerdegen @ 2014-09-03 20:42 UTC (permalink / raw) To: Tom Tromey; +Cc: 18399 Tom Tromey <tom@tromey.com> writes: > So I think there must be a bug with how nadvice handles interactive > specs, either generally or with :filter-args. According to the doc (of `add-function'), an filter-args advice function has to accept exactly one argument (which is bound to the list of given arguments). So I think what you see is expected. I have stumbled over that behavior several times myself. Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-03 20:42 ` Michael Heerdegen @ 2014-09-03 22:27 ` Tom Tromey 2014-09-04 2:59 ` Stefan Monnier 2014-09-04 15:44 ` Stefan Monnier 0 siblings, 2 replies; 10+ messages in thread From: Tom Tromey @ 2014-09-03 22:27 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 18399, Tom Tromey Michael> According to the doc (of `add-function'), an filter-args advice Michael> function has to accept exactly one argument (which is bound to Michael> the list of given arguments). So I think what you see is Michael> expected. Michael> I have stumbled over that behavior several times myself. I looked at the docs again and I agree. Sorry about the noise. Perhaps a note and/or a small example here would be nice for future users. If we were both fooled by this then perhaps others will be as well. Tom ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-03 22:27 ` Tom Tromey @ 2014-09-04 2:59 ` Stefan Monnier 2014-09-04 13:13 ` Stefan Monnier 2014-09-04 15:44 ` Stefan Monnier 1 sibling, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2014-09-04 2:59 UTC (permalink / raw) To: Tom Tromey; +Cc: Michael Heerdegen, 18399 Michael> According to the doc (of `add-function'), an filter-args advice Michael> function has to accept exactly one argument (which is bound to Michael> the list of given arguments). So I think what you see is Michael> expected. Michael> I have stumbled over that behavior several times myself. > I looked at the docs again and I agree. Sorry about the noise. Perhaps > a note and/or a small example here would be nice for future users. If > we were both fooled by this then perhaps others will be as well. FWIW, the use of a single formal arg receiving the actual arg-list in :filter-args is based on performance reasons (we have the list anyway, so it's more efficient to pass it to `funcall' than to `apply'). Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-04 2:59 ` Stefan Monnier @ 2014-09-04 13:13 ` Stefan Monnier 0 siblings, 0 replies; 10+ messages in thread From: Stefan Monnier @ 2014-09-04 13:13 UTC (permalink / raw) To: Tom Tromey; +Cc: Michael Heerdegen, 18399 > FWIW, the use of a single formal arg receiving the actual arg-list > in :filter-args is based on performance reasons (we have the list > anyway, so it's more efficient to pass it to `funcall' than to `apply'). Oh, and also, this way the function can return the list as-is when the args don't need to be modified. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-03 22:27 ` Tom Tromey 2014-09-04 2:59 ` Stefan Monnier @ 2014-09-04 15:44 ` Stefan Monnier 2014-09-04 23:11 ` Michael Heerdegen 1 sibling, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2014-09-04 15:44 UTC (permalink / raw) To: Tom Tromey; +Cc: Michael Heerdegen, 18399-done > I looked at the docs again and I agree. Sorry about the noise. Perhaps > a note and/or a small example here would be nice for future users. If > we were both fooled by this then perhaps others will be as well. I installed the patch below, which I hope will help clear up such confusion. Stefan === modified file 'doc/lispref/ChangeLog' --- doc/lispref/ChangeLog 2014-08-19 18:56:29 +0000 +++ doc/lispref/ChangeLog 2014-09-04 15:42:28 +0000 @@ -1,3 +1,8 @@ +2014-09-04 Stefan Monnier <monnier@iro.umontreal.ca> + + * functions.texi (Core Advising Primitives): Add a note about the + confusing treatment of `interactive' for :filter-args (bug#18399). + 2014-08-19 Eli Zaretskii <eliz@gnu.org> * display.texi (Bidirectional Display): Update the Emacs's class === modified file 'doc/lispref/functions.texi' --- doc/lispref/functions.texi 2014-05-27 01:09:45 +0000 +++ doc/lispref/functions.texi 2014-09-04 15:40:13 +0000 @@ -1220,15 +1220,6 @@ This macro is the handy way to add the advice @var{function} to the function stored in @var{place} (@pxref{Generalized Variables}). -If @var{function} is not interactive, then the combined function will inherit -the interactive spec, if any, of the original function. Else, the combined -function will be interactive and will use the interactive spec of -@var{function}. One exception: if the interactive spec of @var{function} -is a function (rather than an expression or a string), then the interactive -spec of the combined function will be a call to that function with as sole -argument the interactive spec of the original function. To interpret the spec -received as argument, use @code{advice-eval-interactive-spec}. - @var{where} determines how @var{function} is composed with the existing function, e.g. whether @var{function} should be called before, or after the original function. @xref{Advice combinators}, for the list of @@ -1271,6 +1262,21 @@ @code{:override} advice will override not only the original function but all other advices applied to it as well. @end table + +If @var{function} is not interactive, then the combined function will inherit +the interactive spec, if any, of the original function. Else, the combined +function will be interactive and will use the interactive spec of +@var{function}. One exception: if the interactive spec of @var{function} +is a function (rather than an expression or a string), then the interactive +spec of the combined function will be a call to that function with as sole +argument the interactive spec of the original function. To interpret the spec +received as argument, use @code{advice-eval-interactive-spec}. + +Note: The interactive spec of @var{function} will apply to the combined +function and should hence obey the calling convention of the combined function +rather than that of @var{function}. In many cases, it makes no difference +since they are identical, but it does matter for @code{:around}, +@code{:filter-args}, and @code{filter-return}, where @var{function}. @end defmac @defmac remove-function place function ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-04 15:44 ` Stefan Monnier @ 2014-09-04 23:11 ` Michael Heerdegen 2014-09-05 1:24 ` Stefan Monnier 2014-09-06 5:40 ` Tom Tromey 0 siblings, 2 replies; 10+ messages in thread From: Michael Heerdegen @ 2014-09-04 23:11 UTC (permalink / raw) To: 18399; +Cc: tom Hi Stefan > I installed the patch below, which I hope will help clear up such > confusion. Thanks, a good clarification. But I'm not sure if that covers what Tom meant. I myself was confused by the fact that :filter-args is the only case of all advice types where the advice fun receives the arguments as a list. It's a bit surprising, although the doc is clear and there are good reasons for that "exception". Maybe we could add a sentence to the ‘:filter-args’ paragraph of (info "(elisp) Advice combinators") like "Note that FUNCTION is called with only one argument, the list of arguments, for this advice type". Although it is redundant, it may be good to emphasize it. Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-04 23:11 ` Michael Heerdegen @ 2014-09-05 1:24 ` Stefan Monnier 2014-09-05 16:29 ` Michael Heerdegen 2014-09-06 5:40 ` Tom Tromey 1 sibling, 1 reply; 10+ messages in thread From: Stefan Monnier @ 2014-09-05 1:24 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 18399, tom > But I'm not sure if that covers what Tom meant. The fact that we mention the unusual calling convention there does partly cover it. > Although it is redundant, it may be good to emphasize it. As you say, it's a bit redundant there. Stefan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-05 1:24 ` Stefan Monnier @ 2014-09-05 16:29 ` Michael Heerdegen 0 siblings, 0 replies; 10+ messages in thread From: Michael Heerdegen @ 2014-09-05 16:29 UTC (permalink / raw) To: Stefan Monnier; +Cc: 18399, tom Stefan Monnier <monnier@IRO.UMontreal.CA> writes: > > But I'm not sure if that covers what Tom meant. > > The fact that we mention the unusual calling convention there does > partly cover it. Ok, I agree, let's keep the patch as is, thanks. Michael. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#18399: 24.4.50; nadvice :filter-args -vs- interactive 2014-09-04 23:11 ` Michael Heerdegen 2014-09-05 1:24 ` Stefan Monnier @ 2014-09-06 5:40 ` Tom Tromey 1 sibling, 0 replies; 10+ messages in thread From: Tom Tromey @ 2014-09-06 5:40 UTC (permalink / raw) To: Michael Heerdegen; +Cc: 18399, tom Michael> I myself was confused by the fact that :filter-args is the only case of Michael> all advice types where the advice fun receives the arguments as a list. Michael> It's a bit surprising, although the doc is clear and there are good Michael> reasons for that "exception". Maybe we could add a sentence to the Michael> ‘:filter-args’ paragraph of (info "(elisp) Advice combinators") like Michael> "Note that FUNCTION is called with only one argument, the list of Michael> arguments, for this advice type". Yeah, I had the same thought and had written the appended patch. My reason was simply that I had been (mis-)reading the :filter-args text, not the stuff about the (interactive) spec. Tom === modified file 'doc/lispref/functions.texi' *** doc/lispref/functions.texi 2014-06-02 00:18:22 +0000 --- doc/lispref/functions.texi 2014-09-06 05:40:02 +0000 *************** *** 1480,1485 **** --- 1480,1488 ---- @example (lambda (&rest r) (apply @var{oldfun} (funcall @var{function} r))) @end example + Note carefully that, unlike with other combinators, in the + @code{:filter-args} case, the original arguments are passed as a + single argument to the advising function. @item :filter-return Call the old function first and pass the result to @var{function}. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-09-06 5:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-03 20:01 bug#18399: 24.4.50; nadvice :filter-args -vs- interactive Tom Tromey 2014-09-03 20:42 ` Michael Heerdegen 2014-09-03 22:27 ` Tom Tromey 2014-09-04 2:59 ` Stefan Monnier 2014-09-04 13:13 ` Stefan Monnier 2014-09-04 15:44 ` Stefan Monnier 2014-09-04 23:11 ` Michael Heerdegen 2014-09-05 1:24 ` Stefan Monnier 2014-09-05 16:29 ` Michael Heerdegen 2014-09-06 5:40 ` Tom Tromey
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.