Add helm-projectile-mode global minor mode

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### New features
 
 * Add other-tab variants (`helm-projectile-find-file-other-tab`, `helm-projectile-find-dir-other-tab`, `helm-projectile-switch-project-other-tab`), matching Helm 4.x's tab-bar support alongside the existing other-window and other-frame variants. "Open in other tab" is also available as an action (bound to `C-c t`) in the project and directory sources.
+* Add `helm-projectile-mode`, a proper global minor mode that installs and removes the Projectile command remaps. `helm-projectile-on`, `helm-projectile-off` and `helm-projectile-toggle` still work but are now deprecated in favor of the mode. The advice helm-projectile installs on Helm (`helm-find-file-or-marked`, `helm-grep-ag-1`) and the `C-c p f` binding in `helm-etags-map` now live with the mode too, so they are only active while it is enabled instead of unconditionally at load.
 
 ### Changes
 

README.md

@@ -143,21 +143,22 @@ targets are available as actions (and on `C-c o`, `C-c C-o` and `C-c t`
 in the project and directory sources). The `-other-tab` commands need a
 recent Helm and Emacs' `tab-bar-mode`.
 
-If you want to use these commands, you have to activate it to replace
-the normal Projectile commands:
+If you want to use these commands, you have to activate
+`helm-projectile-mode` to replace the normal Projectile commands:
 
 ```el
 ;; (setq helm-projectile-fuzzy-match nil)
 (require 'helm-projectile)
-(helm-projectile-on)
+(helm-projectile-mode)
 ```
 
-If you already activate helm-projectile key bindings and you don't
-like it, you can turn it off and use the normal Projectile bindings
-with command `helm-projectile-off`. Similarly, if you want to disable
-fuzzy matching in Helm Projectile (it is enabled by default), you must
-set `helm-projectile-fuzzy-match` to nil before loading
-`helm-projectile`.
+`helm-projectile-mode` is a global minor mode; toggle it off (`M-x
+helm-projectile-mode` again) to go back to the normal Projectile
+bindings. (The old `helm-projectile-on` / `helm-projectile-off`
+commands still work but are deprecated in favor of the mode.) If you
+want to disable fuzzy matching in Helm Projectile (it is enabled by
+default), you must set `helm-projectile-fuzzy-match` to nil before
+loading `helm-projectile`.
 
 To fully learn Helm Projectile and see what it is capable of, you
 should refer to this guide:

helm-projectile.el

@@ -353,11 +353,6 @@ to be specific to `helm-projectile-projects-source'."
          helm-source-projectile-projects-actions
          '(helm-projectile-switch-project-by-name-other-tab . :make-first))))
 
-(define-key helm-etags-map (kbd "C-c p f")
-  (lambda ()
-    (interactive)
-    (helm-run-after-exit 'helm-projectile-find-file nil)))
-
 (defun helm-projectile-file-persistent-action (candidate)
   "Previews the contents of a file CANDIDATE in a temporary buffer.
 This is a persistent action for file-related functionality."
@@ -536,13 +531,11 @@ CANDIDATE is the selected file.  Used when no file is explicitly marked."
           (rename-buffer dired-buffer-name))))))
 
 (defun helm-projectile-run-projectile-hooks-after-find-file (_orig-fun &rest _args)
-  "Run `projectile-find-file-hook' if using projectile."
+  "Run `projectile-find-file-hook' if using projectile.
+Installed as advice on `helm-find-file-or-marked' by `helm-projectile-mode'."
   (when (and projectile-mode (projectile-project-p))
     (run-hooks 'projectile-find-file-hook)))
 
-(advice-add 'helm-find-file-or-marked
-            :after #'helm-projectile-run-projectile-hooks-after-find-file)
-
 (defvar helm-projectile-find-file-map
   (let ((map (copy-keymap helm-find-files-map)))
     (helm-projectile-define-key map
@@ -1548,17 +1541,19 @@ types (for \"ack\") to include in search."
 
 ;;;###autoload
 (defun helm-projectile-on ()
-  "Turn on `helm-projectile' key bindings."
+  "Turn on `helm-projectile-mode'.
+This function is obsolete; use `helm-projectile-mode' instead."
+  (declare (obsolete helm-projectile-mode "1.7.0"))
   (interactive)
-  (message "Turn on helm-projectile key bindings")
-  (helm-projectile-toggle 1))
+  (helm-projectile-mode 1))
 
 ;;;###autoload
 (defun helm-projectile-off ()
-  "Turn off `helm-projectile' key bindings."
+  "Turn off `helm-projectile-mode'.
+This function is obsolete; use `helm-projectile-mode' instead."
+  (declare (obsolete helm-projectile-mode "1.7.0"))
   (interactive)
-  (message "Turn off helm-projectile key bindings")
-  (helm-projectile-toggle -1))
+  (helm-projectile-mode -1))
 
 ;;;###autoload
 (defun helm-projectile-grep (&optional dir files)
@@ -1650,16 +1645,16 @@ use directly."
           type
           (or input helm-projectile--ag-input))))
 
-;; When calling `helm', the function `helm-grep-ag' uses symbol at point as an
-;; argument `:default-input' (via `helm-sources-using-default-as-input').  This
-;; however sets argument `:input' to an empty string.  As a result the shell
-;; command `ag' (or `rg', or `pt') is being run with the active region or a
-;; symbol at point as a search pattern, but typing in minibuffer starts search
-;; from scratch.  This advice will use symbol active region or a symbol point
-;; as an `input' argument to `helm-grep-ag-1', which will ensure both `helm'
-;; arguments `:default-input' and `:input' are populated.
-(advice-add #'helm-grep-ag-1
-            :filter-args #'helm-projectile--ag-automatic-input)
+;; `helm-projectile-mode' installs `helm-projectile--ag-automatic-input' as
+;; `:filter-args' advice on `helm-grep-ag-1'.  When calling `helm', the
+;; function `helm-grep-ag' uses symbol at point as an argument
+;; `:default-input' (via `helm-sources-using-default-as-input').  This however
+;; sets argument `:input' to an empty string.  As a result the shell command
+;; `ag' (or `rg', or `pt') is being run with the active region or a symbol at
+;; point as a search pattern, but typing in minibuffer starts search from
+;; scratch.  The advice uses the active region or symbol at point as an `input'
+;; argument to `helm-grep-ag-1', which ensures both `helm' arguments
+;; `:default-input' and `:input' are populated.
 
 (defun helm-projectile--ag--region-selection ()
   "Return a default input for `helm-grep-ag'."
@@ -1857,40 +1852,75 @@ package `helm-rg'."
 commands are handled separately because they need a compatibility
 fallback.")
 
+(defun helm-projectile--setup (enable)
+  "Install Helm-Projectile's bindings and advice when ENABLE, else remove them.
+This backs `helm-projectile-mode': it (un)installs the command remaps on
+`projectile-mode-map', swaps `projectile-switch-project-action', and the
+advice and key binding Helm-Projectile relies on.  The advice lives here,
+rather than at load time, so it only affects plain Helm while the mode is
+enabled."
+  (if enable
+      (when (eq projectile-switch-project-action #'projectile-find-file)
+        (setq projectile-switch-project-action #'helm-projectile-find-file))
+    (when (eq projectile-switch-project-action #'helm-projectile-find-file)
+      (setq projectile-switch-project-action #'projectile-find-file)))
+  (pcase-dolist (`(,command . ,helm-command) helm-projectile--command-remaps)
+    (define-key projectile-mode-map (vector 'remap command)
+                (and enable helm-command)))
+  (if enable
+      (progn
+        ;; At the time of writing projectile didn't have neither
+        ;; `projectile-switch-to-project-other-window' nor
+        ;; `projectile-switch-to-project-other-frame' (hopefully these will be
+        ;; names should they be added).  Adding `helm-projectile' bindings in a
+        ;; - hopefully - backward compatible way, by setting up keys in
+        ;; `projectile-command-map'.
+        (if (where-is-internal 'projectile-switch-project-other-window projectile-mode-map nil t t)
+            (define-key projectile-mode-map [remap projectile-switch-project-other-window] #'helm-projectile-switch-project-other-window)
+          (define-key projectile-command-map (kbd "4 p") #'helm-projectile-switch-project-other-window))
+        (if (where-is-internal 'projectile-switch-project-other-frame projectile-mode-map nil t t)
+            (define-key projectile-mode-map [remap projectile-switch-project-other-frame] #'helm-projectile-switch-project-other-frame)
+          (define-key projectile-command-map (kbd "5 p") #'helm-projectile-switch-project-other-frame))
+        (define-key helm-etags-map (kbd "C-c p f")
+                    (lambda ()
+                      (interactive)
+                      (helm-run-after-exit 'helm-projectile-find-file nil)))
+        (advice-add 'helm-find-file-or-marked
+                    :after #'helm-projectile-run-projectile-hooks-after-find-file)
+        (advice-add 'helm-grep-ag-1
+                    :filter-args #'helm-projectile--ag-automatic-input))
+    (if (where-is-internal 'helm-projectile-switch-project-other-window projectile-command-map nil t t)
+        (define-key projectile-mode-map (kbd "4 p") nil)
+      (define-key projectile-mode-map [remap projectile-switch-project-other-window] nil))
+    (if (where-is-internal 'helm-projectile-switch-project-other-frame projectile-command-map nil t t)
+        (define-key projectile-mode-map (kbd "5 p") nil)
+      (define-key projectile-mode-map [remap projectile-switch-project-other-frame] nil))
+    (define-key helm-etags-map (kbd "C-c p f") nil)
+    (advice-remove 'helm-find-file-or-marked
+                   #'helm-projectile-run-projectile-hooks-after-find-file)
+    (advice-remove 'helm-grep-ag-1 #'helm-projectile--ag-automatic-input)))
+
+;;;###autoload
+(define-minor-mode helm-projectile-mode
+  "Toggle the Helm version of Projectile commands.
+
+When enabled, remap Projectile's file, directory, buffer and search
+commands to their Helm-Projectile equivalents on `projectile-mode-map',
+and install the advice and key binding Helm-Projectile relies on.
+Disabling removes all of them."
+  :group 'helm-projectile
+  :require 'helm-projectile
+  :global t
+  (helm-projectile--setup helm-projectile-mode))
+
 (defun helm-projectile-toggle (toggle)
-  "Toggle Helm version of Projectile commands.
-When TOGGLE is greater than 0 turn Helm version of Projectile commands
-on.  When TOGGLE is is less or equal to 0 turn Helm version of commands
-off."
-  (let ((enable (> toggle 0)))
-    (if enable
-        (when (eq projectile-switch-project-action #'projectile-find-file)
-          (setq projectile-switch-project-action #'helm-projectile-find-file))
-      (when (eq projectile-switch-project-action #'helm-projectile-find-file)
-        (setq projectile-switch-project-action #'projectile-find-file)))
-    (pcase-dolist (`(,command . ,helm-command) helm-projectile--command-remaps)
-      (define-key projectile-mode-map (vector 'remap command)
-                  (and enable helm-command)))
-    (if enable
-        (progn
-          ;; At the time of writing projectile didn't have neither
-          ;; `projectile-switch-to-project-other-window' nor
-          ;; `projectile-switch-to-project-other-frame' (hopefully these will be
-          ;; names should they be added).  Adding `helm-projectile' bindings in a
-          ;; - hopefully - backward compatible way, by setting up keys in
-          ;; `projectile-command-map'.
-          (if (where-is-internal 'projectile-switch-project-other-window projectile-mode-map nil t t)
-              (define-key projectile-mode-map [remap projectile-switch-project-other-window] #'helm-projectile-switch-project-other-window)
-            (define-key projectile-command-map (kbd "4 p") #'helm-projectile-switch-project-other-window))
-          (if (where-is-internal 'projectile-switch-project-other-frame projectile-mode-map nil t t)
-              (define-key projectile-mode-map [remap projectile-switch-project-other-frame] #'helm-projectile-switch-project-other-frame)
-            (define-key projectile-command-map (kbd "5 p") #'helm-projectile-switch-project-other-frame)))
-      (if (where-is-internal 'helm-projectile-switch-project-other-window projectile-command-map nil t t)
-          (define-key projectile-mode-map (kbd "4 p") nil)
-        (define-key projectile-mode-map [remap projectile-switch-project-other-window] nil))
-      (if (where-is-internal 'helm-projectile-switch-project-other-frame projectile-command-map nil t t)
-          (define-key projectile-mode-map (kbd "5 p") nil)
-        (define-key projectile-mode-map [remap projectile-switch-project-other-frame] nil)))))
+  "Toggle the Helm version of Projectile commands.
+Turn them on when TOGGLE is greater than 0, off otherwise.
+
+This function is obsolete; use the `helm-projectile-mode' minor mode
+instead."
+  (declare (obsolete helm-projectile-mode "1.7.0"))
+  (helm-projectile-mode (if (> toggle 0) 1 -1)))
 
 ;;;###autoload
 (defun helm-projectile (&optional arg)

test/helm-projectile-test.el

@@ -47,17 +47,18 @@
     (expect (commandp 'helm-projectile-switch-project) :to-be-truthy)
     (expect (commandp 'helm-projectile-switch-to-buffer) :to-be-truthy)))
 
-(describe "helm-projectile-toggle"
+(describe "helm-projectile-mode"
   ;; This is the regression guard for helm-projectile referencing Projectile
   ;; internals that no longer exist (the commander macro/bindings were the
   ;; breakage that prompted this suite).  Toggling must not error and must
-  ;; (un)install the command remaps on `projectile-mode-map'.
+  ;; (un)install the command remaps on `projectile-mode-map', plus the advice
+  ;; the mode owns.
   (after-each
     ;; Always leave a clean slate regardless of what the example did.
-    (helm-projectile-toggle 0))
+    (helm-projectile-mode -1))
 
   (it "enables without error and remaps core commands to their Helm versions"
-    (expect (helm-projectile-toggle 1) :not :to-throw)
+    (expect (helm-projectile-mode 1) :not :to-throw)
     (expect (lookup-key projectile-mode-map [remap projectile-find-file])
             :to-be 'helm-projectile-find-file)
     (expect (lookup-key projectile-mode-map [remap projectile-switch-to-buffer])
@@ -70,17 +71,41 @@
             :to-be 'helm-projectile-rg))
 
   (it "installs a remap for every command in the table"
-    (helm-projectile-toggle 1)
+    (helm-projectile-mode 1)
     (dolist (entry helm-projectile--command-remaps)
       (expect (lookup-key projectile-mode-map (vector 'remap (car entry)))
               :to-be (cdr entry))))
 
-  (it "disables without error and clears the remaps"
-    (helm-projectile-toggle 1)
-    (expect (helm-projectile-toggle 0) :not :to-throw)
+  (it "installs its advice only while enabled"
+    (require 'nadvice)
+    (expect (advice-member-p #'helm-projectile-run-projectile-hooks-after-find-file
+                             'helm-find-file-or-marked)
+            :not :to-be-truthy)
+    (helm-projectile-mode 1)
+    (expect (advice-member-p #'helm-projectile-run-projectile-hooks-after-find-file
+                             'helm-find-file-or-marked)
+            :to-be-truthy)
+    (expect (advice-member-p #'helm-projectile--ag-automatic-input 'helm-grep-ag-1)
+            :to-be-truthy))
+
+  (it "disables without error, clears the remaps and removes the advice"
+    (helm-projectile-mode 1)
+    (expect (helm-projectile-mode -1) :not :to-throw)
     (dolist (entry helm-projectile--command-remaps)
       (expect (lookup-key projectile-mode-map (vector 'remap (car entry)))
-              :to-be nil))))
+              :to-be nil))
+    (expect (advice-member-p #'helm-projectile-run-projectile-hooks-after-find-file
+                             'helm-find-file-or-marked)
+            :not :to-be-truthy))
+
+  (it "keeps helm-projectile-on/off/toggle as obsolete aliases"
+    (expect (get 'helm-projectile-on 'byte-obsolete-info) :to-be-truthy)
+    (expect (get 'helm-projectile-off 'byte-obsolete-info) :to-be-truthy)
+    (expect (get 'helm-projectile-toggle 'byte-obsolete-info) :to-be-truthy)
+    (helm-projectile-on)
+    (expect helm-projectile-mode :to-be-truthy)
+    (helm-projectile-off)
+    (expect helm-projectile-mode :not :to-be-truthy)))
 
 (describe "helm-projectile--files-display-real"
   (it "maps each file to its absolute path under the project root"