how to modify a function's definition graciously

Question

Assume there is a sample function defined in a library (this question's precondition is all definitions in this library cannot be modified, something like "read only"):

(defun sample ()
  (foo)
  (bar)
  (baz))

I want to use this library, but the function sample cannot match my request, what I want is:

(defun sample ()
  (foo)
  (when condition
    (bar))
  (baz))

Someone told me to use defadvice, but I noticed that defadvice can only insert code before or after the invocations of sample, like:

(before-advice ...)
(sample)
(after-advice ...)

it cannot modify the definition of sample itself. So, how can I achieve this graciously? Should I have to rewrite a sample myself, called my-sample or sample2?

Answer 1

sds's answer works, except that you presumably only want to be advising bar when sample is executing, so you'd need to advise sample as well in order to activate and deactivate the advice for bar. My with-temporary-advice macro facilitates this:

(defmacro with-temporary-advice (function class name &rest body)
  "Enable the specified advice, evaluate BODY, then disable the advice."
  `(unwind-protect
       (progn
         (ad-enable-advice ,function ,class ,name)
         (ad-activate ,function)
         ,@body)
     (ad-disable-advice ,function ,class ,name)
     (ad-activate ,function)))

(defadvice bar (around my-conditional-bar disable)
  ;; This advice disabled by default, and enabled dynamically.
  (when condition
    ad-do-it))

(defadvice sample (around my-sample-advice activate)
  "Make execution of `bar' conditional when running `sample'."
  (with-temporary-advice 'bar 'around 'my-conditional-bar
   ad-do-it))

Note that if bar is also called in other ways while sample is executing, the advice will apply for those calls as well, so you should account for that if it's a possibility.

Alternatively, you may prefer to use flet to redefine bar when required. This is subject to the same caveat as the first solution, of course.

(defadvice sample (around my-sample-advice activate)
  "Make execution of `bar' conditional when running `sample'."
  (if condition
      ad-do-it
    (flet ((bar () nil))
      ad-do-it)))

That's much simpler to read, but for reasons I don't understand flet is, as of Emacs 24.3, no longer in favour. Its docstring suggests using cl-flet instead, but as cl-flet uses lexical binding, that won't actually work. As best I could tell, it sounded like flet isn't actually going away, however the current recommendation seems to be to use advice instead.

Also note that if, inside bar, the unwanted behaviour depended on some variable, then it would be preferable to use a let binding on that variable instead of the flet binding on the function.

Edit:

These approaches do make it harder to see what is happening, of course. Depending upon the exact situation, it may well be preferable to simply redefine the sample function to do what you want (or to write a my-sample function to call in its place, as you suggested).

Answer 2

Others have already provided good answers, but since some complain about flet's disgrace, I'll show what I'd use:

(defvar my-inhibit-bar nil)
(defadvice bar (around my-condition activate)
  (unless my-inhibit-bar ad-do-it))
(defadvice sample (around my-condition activate)
  (let ((my-inhibit-bar (not condition)))
    ad-do-it))

Look ma! No flet and no ugly activate/deactive! And when you C-h f bar it will clearly tell you that there's more than meets the eye. Also I'd actually use the new advice-add instead:

(defvar my-inhibit-bar nil)
(defun my-bar-advice (doit &rest args)
  (unless my-inhibit-bar (apply doit args)))
(advice-add :around 'bar #'my-bar-advice)
(defun my-sample-advice (doit &rest args)
  (let ((my-inhibit-bar (not condition)))
    (apply doit args)))
(advice-add :around 'sample #'my-sample-advice)

Answer 3

You should advise function bar instead, using an around advice:

(defadvice bar (around my-condition)
  (when condition
    ad-do-it))