From: Stefan Monnier Date: Tue, 15 Jan 2013 06:05:22 +0000 (-0500) Subject: * lisp/emacs-lisp/advice.el (ad-preactivate-advice): Adjust the cleanup to X-Git-Tag: emacs-24.3.90~173^2~7^2~293 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=cb9c0a53bc4a6d67f10d4674472b2884a71852c8;p=emacs.git * lisp/emacs-lisp/advice.el (ad-preactivate-advice): Adjust the cleanup to the use of nadvice.el. * lisp/emacs-lisp/nadvice.el (advice--tweak): Make it possible for `tweak' to return an explicit nil. (advice--remove-function): Change accordingly. * test/automated/advice-tests.el: Split up. Add advice-test-preactivate. --- diff --git a/lisp/ChangeLog b/lisp/ChangeLog index 56770098b43..f324ebbad51 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,5 +1,12 @@ 2013-01-15 Stefan Monnier + * emacs-lisp/nadvice.el (advice--tweak): Make it possible for `tweak' + to return an explicit nil. + (advice--remove-function): Change accordingly. + + * emacs-lisp/advice.el (ad-preactivate-advice): Adjust the cleanup to + the use of nadvice.el. + * progmodes/which-func.el (which-function): Silence imenu errors (bug#13433). diff --git a/lisp/emacs-lisp/advice.el b/lisp/emacs-lisp/advice.el index 07340f06a13..3d03e894534 100644 --- a/lisp/emacs-lisp/advice.el +++ b/lisp/emacs-lisp/advice.el @@ -2866,10 +2866,8 @@ advised definition from scratch." (defun ad-preactivate-advice (function advice class position) "Preactivate FUNCTION and returns the constructed cache." - (let* ((function-defined-p (fboundp function)) - (old-definition - (if function-defined-p - (symbol-function function))) + (let* ((advicefunname (ad-get-advice-info-field function 'advicefunname)) + (old-advice (symbol-function advicefunname)) (old-advice-info (ad-copy-advice-info function)) (ad-advised-functions ad-advised-functions)) (unwind-protect @@ -2883,10 +2881,9 @@ advised definition from scratch." (list (ad-get-cache-definition function) (ad-get-cache-id function)))) (ad-set-advice-info function old-advice-info) - ;; Don't `fset' function to nil if it was previously unbound: - (if function-defined-p - (fset function old-definition) - (fmakunbound function))))) + (advice-remove function advicefunname) + (fset advicefunname old-advice) + (if old-advice (advice-add function :around advicefunname))))) ;; @@ Activation and definition handling: diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el index 1715763d482..b0711fed26c 100644 --- a/lisp/emacs-lisp/nadvice.el +++ b/lisp/emacs-lisp/nadvice.el @@ -173,20 +173,21 @@ WHERE is a symbol to select an entry in `advice--where-alist'." (let ((first (advice--car flist)) (rest (advice--cdr flist)) (props (advice--props flist))) - (or (funcall tweaker first rest props) + (let ((val (funcall tweaker first rest props))) + (if val (car val) (let ((nrest (advice--tweak rest tweaker))) (if (eq rest nrest) flist (advice--make-1 (aref flist 1) (aref flist 3) - first nrest props))))))) + first nrest props)))))))) ;;;###autoload (defun advice--remove-function (flist function) (advice--tweak flist (lambda (first rest props) - (if (or (not first) - (equal function first) + (cond ((not first) rest) + ((or (equal function first) (equal function (cdr (assq 'name props)))) - rest)))) + (list rest)))))) (defvar advice--buffer-local-function-sample nil) diff --git a/test/ChangeLog b/test/ChangeLog index 472a6073884..7857000ba2f 100644 --- a/test/ChangeLog +++ b/test/ChangeLog @@ -1,3 +1,7 @@ +2013-01-15 Stefan Monnier + + * automated/advice-tests.el: Split up. Add advice-test-preactivate. + 2013-01-14 Glenn Morris * automated/compile-tests.el (compile-tests--test-regexps-data): diff --git a/test/automated/advice-tests.el b/test/automated/advice-tests.el index 238561bef84..8beaea64cd9 100644 --- a/test/automated/advice-tests.el +++ b/test/automated/advice-tests.el @@ -21,99 +21,112 @@ ;;; Code: -(ert-deftest advice-tests () +(ert-deftest advice-tests-nadvice () + "Test nadvice code." + (defun sm-test1 (x) (+ x 4)) + (should (equal (sm-test1 6) 10)) + (advice-add 'sm-test1 :around (lambda (f y) (* (funcall f y) 5))) + (should (equal (sm-test1 6) 50)) + (defun sm-test1 (x) (+ x 14)) + (should (equal (sm-test1 6) 100)) + (should (equal (null (get 'sm-test1 'defalias-fset-function)) nil)) + (advice-remove 'sm-test1 (lambda (f y) (* (funcall f y) 5))) + (should (equal (sm-test1 6) 20)) + (should (equal (get 'sm-test1 'defalias-fset-function) nil)) + + (advice-add 'sm-test3 :around + (lambda (f &rest args) `(toto ,(apply f args))) + '((name . wrap-with-toto))) + (defmacro sm-test3 (x) `(call-test3 ,x)) + (should (equal (macroexpand '(sm-test3 56)) '(toto (call-test3 56))))) + +(ert-deftest advice-tests-advice () "Test advice code." - (with-temp-buffer - (defun sm-test1 (x) (+ x 4)) - (should (equal (sm-test1 6) 10)) - (advice-add 'sm-test1 :around (lambda (f y) (* (funcall f y) 5))) - (should (equal (sm-test1 6) 50)) - (defun sm-test1 (x) (+ x 14)) - (should (equal (sm-test1 6) 100)) - (should (equal (null (get 'sm-test1 'defalias-fset-function)) nil)) - (advice-remove 'sm-test1 (lambda (f y) (* (funcall f y) 5))) - (should (equal (sm-test1 6) 20)) - (should (equal (null (get 'sm-test1 'defalias-fset-function)) t)) - - (defun sm-test2 (x) (+ x 4)) - (should (equal (sm-test2 6) 10)) - (defadvice sm-test2 (around sm-test activate) - ad-do-it (setq ad-return-value (* ad-return-value 5))) - (should (equal (sm-test2 6) 50)) - (ad-deactivate 'sm-test2) - (should (equal (sm-test2 6) 10)) - (ad-activate 'sm-test2) - (should (equal (sm-test2 6) 50)) - (defun sm-test2 (x) (+ x 14)) - (should (equal (sm-test2 6) 100)) - (should (equal (null (get 'sm-test2 'defalias-fset-function)) nil)) - (ad-remove-advice 'sm-test2 'around 'sm-test) - (should (equal (sm-test2 6) 100)) - (ad-activate 'sm-test2) - (should (equal (sm-test2 6) 20)) - (should (equal (null (get 'sm-test2 'defalias-fset-function)) t)) - - (advice-add 'sm-test3 :around - (lambda (f &rest args) `(toto ,(apply f args))) - '((name . wrap-with-toto))) - (defmacro sm-test3 (x) `(call-test3 ,x)) - (should (equal (macroexpand '(sm-test3 56)) '(toto (call-test3 56)))) - - (defadvice sm-test4 (around wrap-with-toto activate) - ad-do-it (setq ad-return-value `(toto ,ad-return-value))) - (defmacro sm-test4 (x) `(call-test4 ,x)) - (should (equal (macroexpand '(sm-test4 56)) '(toto (call-test4 56)))) - (defmacro sm-test4 (x) `(call-testq ,x)) - (should (equal (macroexpand '(sm-test4 56)) '(toto (call-testq 56)))) - - ;; Combining old style and new style advices. - (defun sm-test5 (x) (+ x 4)) - (should (equal (sm-test5 6) 10)) - (advice-add 'sm-test5 :around (lambda (f y) (* (funcall f y) 5))) - (should (equal (sm-test5 6) 50)) - (defadvice sm-test5 (around test activate) - ad-do-it (setq ad-return-value (+ ad-return-value 0.1))) - (should (equal (sm-test5 5) 45.1)) - (ad-deactivate 'sm-test5) - (should (equal (sm-test5 6) 50)) - (ad-activate 'sm-test5) - (should (equal (sm-test5 6) 50.1)) - (defun sm-test5 (x) (+ x 14)) - (should (equal (sm-test5 6) 100.1)) - (advice-remove 'sm-test5 (lambda (f y) (* (funcall f y) 5))) - (should (equal (sm-test5 6) 20.1)) - - ;; This used to signal an error (bug#12858). - (autoload 'sm-test6 "foo") - (defadvice sm-test6 (around test activate) - ad-do-it) - - ;; Check interaction between advice and called-interactively-p. - (defun sm-test7 (&optional x) (interactive) (+ (or x 7) 4)) - (advice-add 'sm-test7 :around - (lambda (f &rest args) - (list (cons 1 (called-interactively-p)) (apply f args)))) - (should (equal (sm-test7) '((1 . nil) 11))) - (should (equal (call-interactively 'sm-test7) '((1 . t) 11))) - (let ((smi 7)) - (advice-add 'sm-test7 :before - (lambda (&rest args) - (setq smi (called-interactively-p)))) - (should (equal (list (sm-test7) smi) - '(((1 . nil) 11) nil))) - (should (equal (list (call-interactively 'sm-test7) smi) - '(((1 . t) 11) t)))) - (advice-add 'sm-test7 :around - (lambda (f &rest args) - (cons (cons 2 (called-interactively-p)) (apply f args)))) - (should (equal (call-interactively 'sm-test7) '((2 . t) (1 . t) 11))) - - ;; Check handling of interactive spec. - (defun sm-test8 (a) (interactive "p") a) - (defadvice sm-test8 (before adv1 activate) nil) - (defadvice sm-test8 (before adv2 activate) (interactive "P") nil) - (should (equal (interactive-form 'sm-test8) '(interactive "P"))) - )) + (defun sm-test2 (x) (+ x 4)) + (should (equal (sm-test2 6) 10)) + (defadvice sm-test2 (around sm-test activate) + ad-do-it (setq ad-return-value (* ad-return-value 5))) + (should (equal (sm-test2 6) 50)) + (ad-deactivate 'sm-test2) + (should (equal (sm-test2 6) 10)) + (ad-activate 'sm-test2) + (should (equal (sm-test2 6) 50)) + (defun sm-test2 (x) (+ x 14)) + (should (equal (sm-test2 6) 100)) + (should (equal (null (get 'sm-test2 'defalias-fset-function)) nil)) + (ad-remove-advice 'sm-test2 'around 'sm-test) + (should (equal (sm-test2 6) 100)) + (ad-activate 'sm-test2) + (should (equal (sm-test2 6) 20)) + (should (equal (null (get 'sm-test2 'defalias-fset-function)) t)) + + (defadvice sm-test4 (around wrap-with-toto activate) + ad-do-it (setq ad-return-value `(toto ,ad-return-value))) + (defmacro sm-test4 (x) `(call-test4 ,x)) + (should (equal (macroexpand '(sm-test4 56)) '(toto (call-test4 56)))) + (defmacro sm-test4 (x) `(call-testq ,x)) + (should (equal (macroexpand '(sm-test4 56)) '(toto (call-testq 56)))) + + ;; This used to signal an error (bug#12858). + (autoload 'sm-test6 "foo") + (defadvice sm-test6 (around test activate) + ad-do-it)) + +(ert-deftest advice-tests-combination () + "Combining old style and new style advices." + (defun sm-test5 (x) (+ x 4)) + (should (equal (sm-test5 6) 10)) + (advice-add 'sm-test5 :around (lambda (f y) (* (funcall f y) 5))) + (should (equal (sm-test5 6) 50)) + (defadvice sm-test5 (around test activate) + ad-do-it (setq ad-return-value (+ ad-return-value 0.1))) + (should (equal (sm-test5 5) 45.1)) + (ad-deactivate 'sm-test5) + (should (equal (sm-test5 6) 50)) + (ad-activate 'sm-test5) + (should (equal (sm-test5 6) 50.1)) + (defun sm-test5 (x) (+ x 14)) + (should (equal (sm-test5 6) 100.1)) + (advice-remove 'sm-test5 (lambda (f y) (* (funcall f y) 5))) + (should (equal (sm-test5 6) 20.1))) + +(ert-deftest advice-test-called-interactively-p () + "Check interaction between advice and called-interactively-p." + (defun sm-test7 (&optional x) (interactive) (+ (or x 7) 4)) + (advice-add 'sm-test7 :around + (lambda (f &rest args) + (list (cons 1 (called-interactively-p)) (apply f args)))) + (should (equal (sm-test7) '((1 . nil) 11))) + (should (equal (call-interactively 'sm-test7) '((1 . t) 11))) + (let ((smi 7)) + (advice-add 'sm-test7 :before + (lambda (&rest args) + (setq smi (called-interactively-p)))) + (should (equal (list (sm-test7) smi) + '(((1 . nil) 11) nil))) + (should (equal (list (call-interactively 'sm-test7) smi) + '(((1 . t) 11) t)))) + (advice-add 'sm-test7 :around + (lambda (f &rest args) + (cons (cons 2 (called-interactively-p)) (apply f args)))) + (should (equal (call-interactively 'sm-test7) '((2 . t) (1 . t) 11)))) + +(ert-deftest advice-test-interactive () + "Check handling of interactive spec." + (defun sm-test8 (a) (interactive "p") a) + (defadvice sm-test8 (before adv1 activate) nil) + (defadvice sm-test8 (before adv2 activate) (interactive "P") nil) + (should (equal (interactive-form 'sm-test8) '(interactive "P")))) + +(ert-deftest advice-test-preactivate () + (should (equal (null (get 'sm-test9 'defalias-fset-function)) t)) + (defun sm-test9 (a) (interactive "p") a) + (should (equal (null (get 'sm-test9 'defalias-fset-function)) t)) + (defadvice sm-test9 (before adv1 pre act protect compile) nil) + (should (equal (null (get 'sm-test9 'defalias-fset-function)) nil)) + (defadvice sm-test9 (before adv2 pre act protect compile) + (interactive "P") nil) + (should (equal (interactive-form 'sm-test9) '(interactive "P")))) ;; Local Variables: ;; no-byte-compile: t