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?

  • 3
    Why don't you define your own interactive command on top of ansi-term? I think that's the preferable solution here. – lunaryorn Jan 3 '16 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. – Aaron Miller Jan 3 '16 at 23:30
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.

  • 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 '16 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 '16 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? – Aaron Miller Jan 3 '16 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 '16 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 '16 at 22:26
3

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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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