12

Following on from an answer to another question about the new advice system:

In old-style advice.el, it was possible to manipulate individual members of an advised function's argument list, without making any assertions regarding those members not so manipulated. For example, the following advice:

(defadvice ansi-term (around prompt-for-name last)
  (let ((name (read-from-minibuffer "Tag: ")))
    (and (not (string= name ""))
         (ad-set-arg 1 (concat "Term: " name)))
    ad-do-it))

allows the (optional) provision of a buffer-name argument to an ansi-term call, while ansi-term will still obtain its first argument by prompting according to its own interactive form.

(For later reference, ansi-term's signature is (PROGRAM &optional BUFFER-NAME), and its interactive form prompts for PROGRAM with several possible defaults, but does nothing regarding BUFFER-NAME.)

I'm not sure whether or not this is possible in nadvice.el. If it is, I'm not certain how it can be done. I've found a couple of ways to replace an advised function's argument list.

For example, from *info* (elisp) Advice combinators:

`:filter-args'
 Call FUNCTION first and use the result (which should be a list) as
 the new arguments to pass to the old function.  More specifically,
 the composition of the two functions behaves like:
      (lambda (&rest r) (apply OLDFUN (funcall FUNCTION r)))

Other combinators provide similar capabilities, and the common thread among them is that, while a function's argument list may be replaced, truncated, extended, et al, there's no apparent way for function advice to modify the argument at a given position in the list without asserting anything about the rest of it.

In the case under discussion, it appears impossible for the advice author to pass ansi-term only a buffer name, because it's not possible to construct a list which has a value in position 1 but nothing, not even nil, in position 0. In the general case, it appears impossible for the advice author to arbitrarily modify arguments beyond position 0.

This seems unfortunate in that, in order to produce a similar effect, it's necessary to copy-paste code: specifically, either I can copy ansi-term's interactive form and extend it to my taste, or I can copy ansi-term altogether and extend it likewise. In either case, I now must redefine part of the Emacs Lisp distribution in my init file, which strikes me as undesirable in terms of both durability and aesthetics.

My question, then, is: Can this sort of argument list mangling be done with nadvice.el? If so, how?

2
  • 3
    Why don't you define your own interactive command on top of ansi-term? I think that's the preferable solution here.
    – user227
    Jan 3, 2016 at 21:35
  • 1
    Of course there's nothing stopping me from doing that, but it would necessitate replacing the better part of a decade's worth of muscle memory, which I would like to avoid if I can. Jan 3, 2016 at 23:30

3 Answers 3

5

This seems unfortunate in that, in order to produce a similar effect, it's necessary to copy-paste code: [...] I can copy ansi-term's interactive form

On the contrary, I think it would be a good idea to copy-paste the interactive form of the advised function, even though you don't actually have to do so here.

I read you question from top to bottom. When I got to the code-block I guessed that your advise probably is changing the buffer name. But I didn't know until you later provided the signature as a comment.

In the case under discussion, it appears impossible for the advice author to pass ansi-term only a buffer name, because it's not possible to construct a list which has a value in position 1 but nothing, not even nil, in position 0.

Indeed nothing is less nothing than nothing. :-) But that's hardly relevant here.

As you can see in the documentation you quoted, the value returned by the advice is used as the arguments to the advised function. The return value has to be a list of all arguments not just the ones that have changed.

Staying as close as possible to the old advise, this is what you would have to do using nadvice:

(defun ansi-term--tag-buffer (args)
  ;; As npostavs pointed out we also have to make sure the list is
  ;; two elements long.  Which makes this approach even more undesirable.
  (when (= (length args) 1)
    (setq args (nconc args (list nil))))
  (let ((name (read-from-minibuffer "Tag: ")))
    (and (not (string= name ""))
         (setf (nth 1 args) (concat "Term: " name))))
  args)

(advice-add 'ansi-term :filter-args 'ansi-term--tag-buffer)

But I recommend you define the advice like this instead:

(defun ansi-term--tag-buffer (program &optional buffer-name)
  (list program
        (let ((tag (read-from-minibuffer "Tag: ")))
          (if (string= tag "")
              buffer-name
            (concat "Term: " tag)))))

This variant actually is self-explanatory.

6
  • For the 1st variant you need to extend the args list in case of a call like (ansi-term "foo"), or else (setf (nth 1 args)... would raise an error.
    – npostavs
    Jan 3, 2016 at 20:37
  • Yes, you're right. Another reason to use the second variant - the first has a bug ;-) Lets, for demonstration purposes, just assume that buffer-name is mandatory.
    – tarsius
    Jan 3, 2016 at 22:35
  • "On the contrary, I think it would be a good idea to copy-paste the interactive form of the advised function" - why so? Copy-pasting code is a bad idea in pretty much every other case; why not here? Jan 3, 2016 at 23:28
  • Actually I don't think "copy-paste" is the right term in this case, I just used it because you did. But even if it were appropriate to use that term here, then "do not copy-paste" is just a heuristic not an absolute rule. Other heuristics, which I think do apply here, are "give meaningful names to variables and arguments" and "when you have a choice between complicating something or being verbose, go with verbose".
    – tarsius
    Jan 4, 2016 at 11:23
  • 1
    Um, actually, this is still broken, :filter-args advice gets a single argument which is a list of args to the advised function, so the 1st variant should drop &rest and the 2nd variant would have to use some sort of destructuring construct to get nice names.
    – npostavs
    Feb 8, 2016 at 22:26
4

Here's how I'd do it:

(defun my-ansi-term-prompt-for-name (orig-fun program
                                     &optional buffer-name &rest args)
  (apply orig-fun program
         (or buffer-name
             (let ((name (read-string "Tag: ")))
               (and (> (length name) 0)
                    (concat "Term: " name))))
         args))
(advice-add 'ansi-term :around #'my-ansi-term-prompt-for-name)

while I was the one who introduced :filter-args I personally find it rarely convenient.

0

It took me sometime to get the right syntax for :filter-args. I would share here just in case it is useful:

(defun me//make-temp-file-in-tmp (args)
  "Make temporary file in /tmp instead of original file directory."
  (cons
    (replace-regexp-in-string
     "/\\([[:alnum:]]+?/\\)+"
     temporary-file-directory
     (car args))
    (cdr args)))

(advice-add 'make-temp-file :filter-args #'me//make-temp-file-in-tmp)
3
  • @Tobias Yeah, you are right. Aug 22 at 20:53
  • I also like the setf-version: (defun me//make-temp-file-in-tmp (args) (setf (car args) (replace-regexp-in-string "/\\([[:alnum:]]+?/\\)+" temporary-file-directory (car args))) args). This version emphasizes that we only modify a member of args and keep its structure. You may add this version if you want. Furthermore, why not keeping your quoted version as an alternative?
    – Tobias
    Aug 23 at 5:45
  • If you collect some possibilities this answer becomes a nice overview for :filter-args. In that case I would change the introductory sentence to something like "Some possibilities to use the :filter-args-advice". The answer does not gain much from your statement that it took some time for you.
    – Tobias
    Aug 23 at 5:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.