whether and how to avoid eval in defadvice loop

Question

Should one avoid eval in the following code? If so, how? Or is this one of exceptional cases where using eval is better?

(dolist (command '(....))
  (eval
   `(defadvice ,command (around blah activate)
      ...)))

For a real life example of the idiom above:

(dolist (command '(paredit-comment-dwim comment-dwim))
  (eval
   `(defadvice ,command (around my-check-parens-and-warn-for-comment activate)
      (if (and (called-interactively-p 'any)
               (use-region-p))
          (progn
            (my-check-parens-and-warn-if-mismatch "You commented out a region and introduced a mismatched paren")
            ad-do-it
            (my-check-parens-and-warn-if-mismatch "You uncommented out a region and introduced a mismatched paren"))
        ad-do-it))))

I would avoid `defadvice` at all, and instead just re-bind the commands. `defadvice` is hard to debug, and some extension developers may refuse to help you with issues should they discover your adviced their commands. — , Sep 21 '13 at 14:17
Regarding the example from your question, I wonder what you need `eval` for? — , Sep 21 '13 at 14:18
I agree with @lunaryon on both counts. Especially since this is only for interactive use, just write your own commands and remap the keys for the vanilla commands to yours. Should be no need for advice here, AFAICT. — Drew, Sep 21 '13 at 16:37

score 3 · Accepted Answer · answered Sep 21 '13 at 16:17

Two solutions:

Use ad-add-advice instead of defadvice.
If you're using Emacs trunk, you can use the new advice-add.

Using advice-add would look like the following code:

(defun my-check-commented-parens (orig-fun &rest args)
  (if (not (and (called-interactively-p 'any)
                (use-region-p)))
      (apply orig-fun args)
    (my-check-parens-and-warn-if-mismatch "You commented out a region and introduced a mismatched paren")
    (apply orig-fun args)
    (my-check-parens-and-warn-if-mismatch "You uncommented out a region and introduced a mismatched paren")))

(dolist (command '(paredit-comment-dwim comment-dwim))
  (advice-add command :around #'my-check-commented-parens))

I'm curious why someone would give me a -1 on this one. – Stefan Sep 22 '13 at 00:52 — Stefan, Sep 22 '13 at 00:52

whether and how to avoid eval in defadvice loop

1 Answers1