Convert old advise ad-set-arg to new advice

Question

How would I convert the following old style advice to a new style advice?

(setq python--pdb-breakpoint-string "import pdb; pdb.set_trace()")

(defadvice compile (before ad-compile-smart activate)
  "Advises `compile' so it sets the argument COMINT to t
if breakpoints are present in `python-mode' files"
  (when (derived-mode-p major-mode 'python-mode)
    (save-excursion
      (save-match-data
        (goto-char (point-min))
        (if (re-search-forward (concat "^\\s-*" python--pdb-breakpoint-string "$")
                               (point-max) t)
            ;; set COMINT argument to `t'.
            (ad-set-arg 1 t))))))

Snippet from: https://masteringemacs.org/article/compiling-running-scripts-emacs

From Porting old advice, the recommendations are :around or :filter-args.

Using :filter-args, I'm at a loss for what to return for the first argument of compile. The first argument is command. I have no clue what that is, or might be, and don't know how to find it during advising/let it pass through.

(advice-add 'compile :filter-args
            (lambda ()
              (when (derived-mode-p major-mode 'python-mode)
                (save-excursion
                  (save-match-data
                    (goto-char (point-min))
                    (if (re-search-forward (concat "^\\s-*" python--pdb-breakpoint-string "$") (point-max) t)
                        ;; new arglist: 
                        ;; (command t)
                        ))))))

Using :around, I think I have the same issue, but it's hard to tell. I'm getting overwhelmed by the documentation. It seems like I would need to call apply if the if passes and call the original compile otherwise:

(advice-add 'compile :around
            (lambda (oldcompile)
              (when (derived-mode-p major-mode 'python-mode)
                (save-excursion
                  (save-match-data
                    (goto-char (point-min))
                    (if (re-search-forward (concat "^\\s-*" my-python-break-string "$")
                                           (point-max) t)
                        (apply oldcompile command t)
                       oldcompile))))))

Tobias · Accepted Answer · 2020-02-19T17:50:06.280

See the documentation for add-function or the corresponding page in the manual:

If you define an :filter-args advice your function FUNCTION advising OLDFUN is called as follows:

(lambda (&rest r) (apply OLDFUN (funcall FUNCTION r)))

That means you get the arguments as list, may modify that list, and pass it as list of arguments to the old function.

(setq python--pdb-breakpoint-string "import pdb; pdb.set_trace()")

(defun my-compile-advice (args)
  "Advises `compile' so it sets the argument COMINT to t
if breakpoints are present in `python-mode' files"
  (when (derived-mode-p major-mode 'python-mode)
    (save-excursion
      (save-match-data
        (goto-char (point-min))
        (if (re-search-forward (concat "^\\s-*" python--pdb-breakpoint-string "$")
                               (point-max) t)
            ;; set COMINT argument to `t'.
            (let ((command (car args)))
              (setq args (list command t)))))))
  args)

(advice-add 'compile :filter-args #'my-compile-advice)

I recommend always to define a function for advising OLDFUN. That gives you the possibility to jump to the definition of the advice from the help of OLDFUN. Furthermore, it simplifies debugging a lot.

score 1 · Answer 2 · answered Feb 19 '20 at 17:55

You need to know how your function will be invoked, that is, what arguments it will take and what result must return, (info "(elisp) Advice combinators") or C-h add-function says:

:filter-args (lambda (&rest r) (apply OLDFUN (funcall FUNCTION r)))

thus your function (that is FUNCTION in above) should take exactly one argument, which is always a list (suggested by &rest), and return another list (suggested by apply).

For example, we want to swap A and B in foo

(defun foo (a b) (list a b))

(define-advice foo (:filter-args (args) swap)
  "Swap A and B."
  (list (nth 1 args) (nth 0 args)))

(foo 1 2)
;; => (2 1)

:around is the most generic one, it can replace all others

:around (lambda (&rest r) (apply FUNCTION OLDFUN r))

so your function should accept one or more argument, the first is OLDFUN, thus both of the following work

(define-advice foo (:around (old-fun a b) swap-2)
  (funcall old-fun b a))

(define-advice foo (:around (old-fun &rest r) swap-3)
  (apply old-fun (nreverse r)))

Stefan · Answer 3 · 2020-02-19T23:20:23.177

I guess the closest replacement for

(defadvice FOO (before ...)
  ... (ad-set-arg N V) ...)

is

(advice-add FOO :around
  (lambda (orig-fun &rest args)
    ... (setf (nth N args) V) ...
    (apply orig-fun args)))

But as Tobias points out, this can fail if there is no Nth argument. Also using setf/setq makes you lose valuable karma points. So you're better off writing code that's not as similar-looking as the original. E.g.

(advice-add 'compile :around
  (lambda (orig-fun command &optional comint)
    (funcall orig-fun command
             ;; Set COMINT argument to t if there's a Python breakpoint.
             (or (when (derived-mode-p major-mode 'python-mode)
                   (save-excursion
                     (save-match-data
                       (goto-char (point-min))
                       (re-search-forward
                        (concat "^\\s-*" python--pdb-breakpoint-string "$")
                        nil t))))
                 comint))))

Hi Stefan, I also had `(setf (nth N args) V)` in my code before. But, you need to be careful with that. It might be that the argument N is optional. If the user does not specify it `(setf (nth N args) V)` fails. In fact, the `COMINT` argument of `compile` is optional. — Tobias, Feb 19 '20 at 22:32
@Tobias: you're right that reality is more complex than my example suggests. I'll clarify. — Stefan, Feb 19 '20 at 23:12

Convert old advise ad-set-arg to new advice

3 Answers3