4

I was trying to answer another SO question when I hit upon some very odd behavior. Here's my little test case:

(make-variable-buffer-local
 (defvar my-override-mode-on-save nil
   "Can be set to automatically ignore read-only mode of a file when saving."))

(defadvice file-writable-p (around my-overide-file-writeable-p act)
  "override file-writable-p if `my-override-mode-on-save' is set."
  (or
   my-override-mode-on-save
   ad-do-it))

(defun my-override-toggle-read-only ()
  "Toggle buffer's read-only status, keeping `my-override-mode-on-save' in sync."
  (interactive)
  (setq my-override-mode-on-save (not my-override-mode-on-save))
  (toggle-read-only))

(defun tester-fn ()
  (interactive)
  (let ((xxx (file-writable-p "/tmp/foofoo"))
        (yyy (file-writable-p "/tmp/fooxxfoo")))
    (message (concat "XXX: " (if xxx "yes" "no") "   -   YYY: " (if yyy "yes" "no")))))

where:

  • /tmp/foofoo is a read-only file that I've visited and run my-override-toggle-read-only in.
  • /tmp/fooxxfoo does not exist.
  • /tmp is writable by the user I'm logged in as.

If I run tester-fn in a buffer where my-override-mode-on-save is set to t then I get an unexpected result: XXX: no - YYY: no. If I run tester-fn while in some other buffer (e.g. scratch) I get the expected response in the minibuffer: XXX: no - YYY: yes. Tracing the advice through the debugger shows it to be doing exactly what I think it should be doing, executing the parts I expect it to, skipping the parts I expect it to, returning the value I expect it to. However, tracing tester-fn through the debugger shows very different values being returned (nil & t if the variable evaluates as nil, nil & nil if the variable evaluates as non-nil). The nil & nil return is really what I find bizarre.

I have no clue what's happening here. Anyone know why I'm not getting the results I expect?

Community
  • 1
  • 1
Joe Casadonte
  • 15,888
  • 11
  • 45
  • 57

1 Answers1

5

Your code looks good, except the one missing key. You need to set the return value appropriately:

(defadvice file-writable-p (around my-overide-file-writeable-p act)
  "override file-writable-p if `my-override-mode-on-save' is set."
  (setq ad-return-value
        (or
         my-override-mode-on-save
         ad-do-it)))

This is documented in the advice manual.

tripleee
  • 175,061
  • 34
  • 275
  • 318
Trey Jackson
  • 73,529
  • 11
  • 197
  • 229
  • I've always thought that `ad-return-value` was used to override the return value, not set it (and the manual says as much). But this was indeed the missing piece -- thanks! – Joe Casadonte Jun 19 '10 at 21:50
  • the difference (for me) is the assumption that the return value will be normal (i.e. the last evaluated expression) and `ad-return-value` would be used to change that. – Joe Casadonte Jun 20 '10 at 12:01
  • @JoeCasadonte Ah, I always interpreted advice as giving you hooks to change behavior, but that (quoting) "After-advice and around-advice can override the return value by setting ad-return-value." But I can see your POV. – Trey Jackson Jun 20 '10 at 21:06