[nongnu] scratch/evil 44469e900e 3/4: Replace uses of `defadvice` with `advice-add`

[Stefan Monnier]

branch: scratch/evil
commit 44469e900e58ce6800f066769d2fb7f2ee212e85
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Commit: Stefan Monnier <monnier@iro.umontreal.ca>

    Replace uses of `defadvice` with `advice-add`
    
    This either requires a dependency on the `nadvice` package,
    or bumping the minimum Emacs version to 24.4.  I went with
    the `nadvice` package, but maybe bumping up to 24.4 would be better.
    
    * evil.el: Require `nadvice`.
    
    * evil-core.el (evil--local-advice-add): New function.
    (evil--global-advice-list): New var.
    (evil-mode): Use it instead of `ad-dis/enable`.
    (evil--global-advice-add): New function.
    (set-window-buffer, select-window, toggle-input-method, use-global-map):
    * evil-search.el (isearch-message-prefix, isearch-delete-char)
    (isearch-lazy-highlight-search):
    * evil-integration.el (keyboard-quit, wdired-change-to-dired-mode)
    (show-paren-function, quail-show-key, describe-char, ace-jump-done):
    Use `evail--(local/global)-advice-add` instead of `defadvice`.
    (preceding-sexp, pp-last-sexp): Remove old code for when `advice-add`
    is not available.
    
    * evil-repeat.el (evil--read-key-sequence-advice): Adapt to use in
    `advice-add`.
    (read-key-sequence, read-key-sequence-vector): Use 
`evail--local-advice-add`.
    
    * evil-keybindings.el (elp-results): Use `evil--local-advice-add`
    and move outside of `eval-after-load`.
---
 evil-core.el        | 61 +++++++++++++++++++++++++------------
 evil-integration.el | 88 +++++++++++++++++++++++------------------------------
 evil-keybindings.el |  6 ++--
 evil-repeat.el      |  8 ++---
 evil-search.el      | 18 ++++++-----
 evil.el             |  2 +-
 6 files changed, 96 insertions(+), 87 deletions(-)

diff --git a/evil-core.el b/evil-core.el
index 4966491850..49e4899aac 100644
--- a/evil-core.el
+++ b/evil-core.el
@@ -106,12 +106,20 @@
 
 ;;; Code:
 
-(require 'advice)
 (require 'evil-common)
 
 (declare-function evil-emacs-state-p "evil-states")
 (declare-function evil-ex-p "evil-ex")
 
+(defun evil--local-advice-add (&rest args)
+  "Like `advice-add' for Evil's advices active in `evil-local-mode'."
+  ;; FIXME: Ideally, we should wait until the first buffer with
+  ;; `evil-local-mode' and then disable them when we leave the last such
+  ;; buffer, but the advices need to be careful to be harmless
+  ;; in non-evil-local-mode buffers anyway, so it's OK to activate
+  ;; them even when there's no `evil-local-mode' buffer.
+  (apply #'advice-add args))
+
 (define-minor-mode evil-local-mode
   "Minor mode for setting up Evil in a single buffer."
   :init-value nil
@@ -168,11 +176,16 @@ To enable Evil globally, do (evil-mode)."
 
 (defalias 'evil--fundamental-mode #'fundamental-mode)
 
+(defvar evil--global-advice-list nil)
+
 ;;;###autoload (autoload 'evil-mode "evil" nil t)
 (define-globalized-minor-mode evil-mode evil-local-mode evil-initialize
   :group 'evil)
 
-(defadvice evil-mode (after start-evil activate)
+;; `define-globalized-minor-mode' supports a BODY argument but only since
+;; Emacs-27.1, so we resort to using this ugly advice in the mean time.
+(advice-add 'evil-mode :after #'evil--mode-body)
+(defun evil--mode-body (&rest _)
   ;; Hooks used to not run in Fundamental buffers (bug#23827), so
   ;; other measures are necessary to initialize Evil there. When Evil
   ;; is enabled globally, the default value of `major-mode' is set to
@@ -181,16 +194,21 @@ To enable Evil globally, do (evil-mode)."
       (progn
         (and (eval-when-compile (version< emacs-version "26.1"))
              (eq (default-value 'major-mode) 'fundamental-mode)
-             (setq-default major-mode 'evil--fundamental-mode))
-        (ad-enable-regexp "^evil")
-        (ad-activate-regexp "^evil")
+             (setq-default major-mode #'evil--fundamental-mode))
+        (pcase-dolist (advice evil--global-advice-list)
+          (apply #'advice-add advice))
         (with-no-warnings (evil-esc-mode 1)))
-    (when (eq (default-value 'major-mode) 'evil--fundamental-mode)
+    (when (eq (default-value 'major-mode) #'evil--fundamental-mode)
       (setq-default major-mode 'fundamental-mode))
-    (ad-disable-regexp "^evil")
-    (ad-update-regexp "^evil")
+    (pcase-dolist (`(,funname ,_where ,adfun) evil--global-advice-list)
+      (advice-remove funname adfun))
     (with-no-warnings (evil-esc-mode -1))))
 
+(defun evil--global-advice-add (&rest args)
+  "Like `advice-add' for Evil's advice active in `evail-mode'."
+  (when evil-mode (apply #'advice-add args))
+  (add-to-list 'evil--global-advice-list args))
+
 (defun evil-change-state (state &optional message)
   "Change the state to STATE.
 If STATE is nil, disable all states."
@@ -329,17 +347,20 @@ then this function does nothing."
 ;; run. This is appropriate since many buffers are used for throwaway
 ;; purposes. Passing the buffer to `set-window-buffer' indicates
 ;; otherwise, though, so advise this function to initialize Evil.
-(defadvice set-window-buffer (before evil)
+(evil--global-advice-add 'set-window-buffer :before #'evil--swb-initialize)
+(defun evil--swb-initialize (_window buffer &rest _)
   "Initialize Evil in the displayed buffer."
-  (when (and evil-mode (get-buffer (ad-get-arg 1)))
-    (with-current-buffer (ad-get-arg 1)
+  (when (and evil-mode (get-buffer buffer))
+    (with-current-buffer buffer
       (unless evil-local-mode
         (save-match-data (evil-initialize))))))
 
 ;; Refresh cursor color.
 ;; Cursor color can only be set for each frame but not for each buffer.
+;; FIXME: Shouldn't this belong in `evil-(local-)mode'?
 (add-hook 'window-configuration-change-hook #'evil-refresh-cursor)
-(defadvice select-window (after evil activate)
+(evil--local-advice-add 'select-window :after #'evil--sw-refresh-cursor)
+(defun evil--sw-refresh-cursor (&rest _)
   (evil-refresh-cursor))
 
 (defun evil-generate-mode-line-tag (&optional state)
@@ -422,16 +443,17 @@ This allows input methods to be used in normal-state."
      (add-hook 'input-method-activate-hook #'evil-activate-input-method nil t)
      (add-hook 'input-method-deactivate-hook #'evil-deactivate-input-method 
nil t)))
 
-(defadvice toggle-input-method (around evil)
+(evil--global-advice-add 'toggle-input-method :around 
#'evil--refresh-inpput-method)
+(defun evil--refresh-inpput-method (orig-fun &rest args)
   "Refresh `evil-input-method'."
   (cond
    ((not evil-local-mode)
-    ad-do-it)
+    (apply orig-fun args))
    ((evil-state-property evil-state :input-method)
-    ad-do-it)
+    (apply orig-fun args))
    (t
     (let ((current-input-method evil-input-method))
-      ad-do-it))))
+      (apply orig-fun args)))))
 
 ;; Local keymaps are implemented using buffer-local variables.
 ;; However, unless a buffer-local value already exists,
@@ -1105,13 +1127,12 @@ Add additional BINDINGS if specified."
 
 ;; Advise these functions as they may activate an overriding keymap or
 ;; a keymap with state bindings; if so, refresh `evil-mode-map-alist'.
-(defadvice use-global-map (after evil activate)
+(evil--local-advice-add 'use-global-map :after #'evil--ugm-normalize-keymaps)
+(evil--local-advice-add 'use-local-map :after #'evil--ugm-normalize-keymaps)
+(defun evil--ugm-normalize-keymaps (&rest _)
   "Refresh Evil keymaps."
   (evil-normalize-keymaps))
 
-(defadvice use-local-map (after evil activate)
-  "Refresh Evil keymaps."
-  (evil-normalize-keymaps))
 
 (defmacro evil-define-state (state doc &rest body)
   "Define an Evil state STATE.
diff --git a/evil-integration.el b/evil-integration.el
index e7f156a24b..d622f2b748 100644
--- a/evil-integration.el
+++ b/evil-integration.el
@@ -91,23 +91,31 @@
 ;;; key-binding
 
 ;; Calling `keyboard-quit' should cancel repeat
-(defadvice keyboard-quit (before evil activate)
+(evil--local-advice-add 'keyboard-quit :before #'evil--repeat-abort)
+(defun evil--repeat-abort (&rest _)
   (when (fboundp 'evil-repeat-abort)
     (evil-repeat-abort)))
 
-(eval-after-load 'wdired
-  '(progn
-     (add-hook 'wdired-mode-hook #'evil-change-to-initial-state)
-     (defadvice wdired-change-to-dired-mode (after evil activate)
-       (evil-change-to-initial-state nil t))))
+(add-hook 'wdired-mode-hook #'evil-change-to-initial-state)
+(evil--local-advice-add 'wdired-change-to-dired-mode :after
+            #'evil--change-to-initial-state-with-msg)
+(defun evil--change-to-initial-state-with-msg (&rest _)
+  (evil-change-to-initial-state nil t))
 
 ;;; Parentheses
 
-(defadvice show-paren-function (around evil disable)
+;; FIXME: This one was marked `disable' but AFAICT it got enabled
+;; by `evil-mode' so behaved like a "global" advice, even though it
+;; seems to me it should be local!
+(evil--global-advice-add 'show-paren-function :around 
#'evil--match-paren-in-normal-state)
+(defun evil--match-paren-in-normal-state (orig-fun &rest args)
   "Match parentheses in Normal state."
+  ;; FIXME: Why not use `add-function' on `show-paren-data-function' instead?
+  ;; Is it because we want to add globally yet have it take precedence over
+  ;; buffer-local `show-paren-data-function'?
   (if (eq (not (memq 'not evil-highlight-closing-paren-at-point-states))
           (not (memq evil-state evil-highlight-closing-paren-at-point-states)))
-      ad-do-it
+      (apply orig-fun args)
     (let* ((orig-spdf show-paren-data-function)
            (show-paren-data-function
             (lambda ()
@@ -124,7 +132,7 @@
                     (save-restriction
                       (when narrow (narrow-to-region narrow (point-max)))
                       (funcall orig-spdf))))))))
-      ad-do-it)))
+      (apply orig-fun args))))
 
 ;;; Undo tree
 (eval-after-load 'undo-tree
@@ -210,47 +218,27 @@
              company-filter-candidates))))
 
 ;; Eval last sexp
-(cond
- ((version< emacs-version "25")
-  (defadvice preceding-sexp (around evil activate)
-    "In normal-state or motion-state, last sexp ends at point."
-    (if (and (not evil-move-beyond-eol)
-             (or (evil-normal-state-p) (evil-motion-state-p)))
-        (save-excursion
-          (unless (or (eobp) (eolp)) (forward-char))
-          ad-do-it)
-      ad-do-it))
-
-  (defadvice pp-last-sexp (around evil activate)
-    "In normal-state or motion-state, last sexp ends at point."
-    (if (and (not evil-move-beyond-eol)
-             (or (evil-normal-state-p) (evil-motion-state-p)))
-        (save-excursion
-          (unless (or (eobp) (eolp)) (forward-char))
-          ad-do-it)
-      ad-do-it)))
- (t
-  (defun evil--preceding-sexp (command &rest args)
-    "In normal-state or motion-state, last sexp ends at point."
-    (if (and (not evil-move-beyond-eol)
-             (or (evil-normal-state-p) (evil-motion-state-p)))
-        (save-excursion
-          (unless (or (eobp) (eolp)) (forward-char))
-          (apply command args))
-      (apply command args)))
-
-  (advice-add 'elisp--preceding-sexp :around 'evil--preceding-sexp '((name . 
evil)))
-  (advice-add 'pp-last-sexp          :around 'evil--preceding-sexp '((name . 
evil)))
-  (advice-add 'lisp-eval-last-sexp   :around 'evil--preceding-sexp '((name . 
evil)))))
+
+(defun evil--preceding-sexp (command &rest args)
+  "In normal-state or motion-state, last sexp ends at point."
+  (if (and (not evil-move-beyond-eol)
+           (or (evil-normal-state-p) (evil-motion-state-p)))
+      (save-excursion
+        (unless (or (eobp) (eolp)) (forward-char))
+        (apply command args))
+    (apply command args)))
+
+(evil--local-advice-add 'elisp--preceding-sexp :around #'evil--preceding-sexp)
+(evil--local-advice-add 'pp-last-sexp          :around #'evil--preceding-sexp)
+(evil--local-advice-add 'lisp-eval-last-sexp   :around #'evil--preceding-sexp)
 
 ;; Show key
-(defadvice quail-show-key (around evil activate)
+(evil--local-advice-add 'quail-show-key :around #'evil--in-emacs-state)
+(evil--local-advice-add 'describe-char  :around #'evil--in-emacs-state)
+(defun evil--in-emacs-state (orig-fun &rest args)
   "Temporarily go to Emacs state"
-  (evil-with-state emacs ad-do-it))
+  (evil-with-state emacs (apply orig-fun args)))
 
-(defadvice describe-char (around evil activate)
-  "Temporarily go to Emacs state"
-  (evil-with-state emacs ad-do-it))
 
 ;; ace-jump-mode
 (declare-function ace-jump-char-mode "ext:ace-jump-mode")
@@ -294,10 +282,10 @@ the mark and entering `recursive-edit'."
              (set-mark old-mark))
          (push-mark old-mark)))))
 
-(eval-after-load 'ace-jump-mode
-  `(defadvice ace-jump-done (after evil activate)
-     (when evil-ace-jump-active
-       (add-hook 'post-command-hook #'evil-ace-jump-exit-recursive-edit))))
+(evil--local-advice-add 'ace-jump-done :after #'evil--after-ace-jump-done)
+(defun evil--after-ace-jump-done (&rest _)
+  (when evil-ace-jump-active
+    (add-hook 'post-command-hook #'evil-ace-jump-exit-recursive-edit)))
 
 (defun evil-ace-jump-exit-recursive-edit ()
   "Exit a recursive edit caused by an evil jump."
diff --git a/evil-keybindings.el b/evil-keybindings.el
index 8534ec856d..4b5dc38894 100644
--- a/evil-keybindings.el
+++ b/evil-keybindings.el
@@ -115,9 +115,9 @@
 
 ;;; ELP
 
-(eval-after-load 'elp
-  '(defadvice elp-results (after evil activate)
-     (evil-motion-state)))
+(evil--local-advice-add 'elp-results :after #'evil--set-motion-state)
+(defun evil--set-motion-state (&rest _)
+  (evil-motion-state))
 
 (provide 'evil-keybindings)
 
diff --git a/evil-repeat.el b/evil-repeat.el
index ba00213f50..7ea1e97c17 100644
--- a/evil-repeat.el
+++ b/evil-repeat.el
@@ -625,7 +625,7 @@ If COUNT is negative, this is a more recent kill."
                      (not evil-repeat-move-cursor)))
   (evil-repeat-pop (- count) save-point))
 
-(defun evil--read-key-sequence-advice ()
+(defun evil--read-key-sequence-advice (&rest _)
   "Record `this-command-keys' before it is overwritten."
   (when (and (evil-repeat-recording-p)
              evil-recording-current-command)
@@ -633,10 +633,8 @@ If COUNT is negative, this is a more recent kill."
       (when (functionp repeat-type)
         (funcall repeat-type 'pre-read-key-sequence)))))
 
-(defadvice read-key-sequence (before evil activate)
-  (evil--read-key-sequence-advice))
-(defadvice read-key-sequence-vector (before evil activate)
-  (evil--read-key-sequence-advice))
+(evil--local-advice-add 'read-key-sequence :before 
#'evil--read-key-sequence-advice)
+(evil--local-advice-add 'read-key-sequence-vector :before 
#'evil--read-key-sequence-advice)
 
 (provide 'evil-repeat)
 
diff --git a/evil-search.el b/evil-search.el
index e47a84ee83..8967c6349a 100644
--- a/evil-search.el
+++ b/evil-search.el
@@ -348,13 +348,14 @@ nil if nothing is found."
   "Prefix STRING with the search prompt."
   (format "%s%s" (evil-search-prompt forward) string))
 
-(defadvice isearch-message-prefix (around evil activate)
+(evil--local-advice-add 'isearch-message-prefix :around 
#'evil--use-search-prompt)
+(defun evil--use-search-prompt (orig-fun &rest args)
   "Use `evil-search-prompt'."
-  (if evil-search-prompt
-      (setq ad-return-value evil-search-prompt)
-    ad-do-it))
+  (or evil-search-prompt
+      (apply orig-fun args)))
 
-(defadvice isearch-delete-char (around evil activate)
+(evil--local-advice-add 'isearch-delete-char :around 
#'evil--exit-search-when-empty)
+(defun evil--exit-search-when-empty (orig-fun &rest args)
   "Exit search if no search string."
   (cond
    ((and evil-search-prompt (string= isearch-string ""))
@@ -362,12 +363,13 @@ nil if nothing is found."
       (setq isearch-success nil)
       (isearch-exit)))
    (t
-    ad-do-it)))
+    (apply orig-fun args))))
 
-(defadvice isearch-lazy-highlight-search (around evil activate)
+(evil--local-advice-add 'isearch-lazy-highlight-search :around 
#'evil--without-search-wrap)
+(defun evil--without-search-wrap (orig-fun &rest args)
   "Never wrap the search in this context."
   (let (evil-search-wrap)
-    ad-do-it))
+    (apply orig-fun args)))
 
 ;;; Ex search
 
diff --git a/evil.el b/evil.el
index 009b25f9eb..51cc45db42 100644
--- a/evil.el
+++ b/evil.el
@@ -59,7 +59,7 @@
 ;;      mailing list (see below).
 ;; Created: 2011-03-01
 ;; Version: 1.15.0
-;; Package-Requires: ((emacs "24.1") (cl-lib "0.5") (goto-chg "1.6"))
+;; Package-Requires: ((emacs "24.1") (cl-lib "0.5") (goto-chg "1.6") (nadvice 
"0.3"))
 ;; Keywords: emulations
 ;; URL: https://github.com/emacs-evil/evil
 ;;      Repository: https://github.com/emacs-evil/evil.git