spacemacs-whitespace-cleanup.el: Rewrite and fix toggling

Fixes #8221.

The previous implementation had a non-standard structure that made it difficult
to correctly implement toggling the minor mode locally and globally. Hence this
commit rewrites the enabling/disabling mechanisms of the minor mode. Both local
and global toggling with `SPC t W` and `SPC t C-S-w` should now work correctly.

As `ws-butler-global-mode` is not used anymore, users that customized
`ws-butler-global-exempt-modes` need to customize
`global-spacemacs-whitespace-cleanup-modes` instead. The default values are
equivalent: `markdown-mode` is excluded.

core/core-dotspacemacs.el

@@ -648,7 +648,11 @@ Possible values are:
 `all' to aggressively delete empty lines and long sequences of whitespace,
 `trailing' to delete only the whitespace at end of lines,
 `changed' to delete only whitespace for changed lines or
-`nil' to disable cleanup."
+`nil' to disable cleanup.
+
+The variable `global-spacemacs-whitespace-cleanup-modes' controls
+which major modes have whitespace cleanup enabled or disabled
+by default."
   '(choice (const nil) (const all) (const trailing) (const changed))
   'spacemacs-dotspacemacs-init)
 

core/templates/.spacemacs.template

@@ -518,6 +518,9 @@ It should only modify the values of Spacemacs settings."
    ;; to aggressively delete empty line and long sequences of whitespace,
    ;; `trailing' to delete only the whitespace at end of lines, `changed' to
    ;; delete only whitespace for changed lines or `nil' to disable cleanup.
+   ;; The variable `global-spacemacs-whitespace-cleanup-modes' controls
+   ;; which major modes have whitespace cleanup enabled or disabled
+   ;; by default.
    ;; (default nil)
    dotspacemacs-whitespace-cleanup nil
 

layers/+spacemacs/spacemacs-editing/local/spacemacs-whitespace-cleanup/spacemacs-whitespace-cleanup.el

@@ -2,7 +2,7 @@
 
 ;; Author: Sylvain Benner <[email protected]>
 ;; Keywords: editing, whitespace, spacemacs
-;; Version: 0.1
+;; Version: 0.2
 
 ;; This file is free software; you can redistribute it and/or modify
 ;; it under the terms of the GNU General Public License as published by
@@ -26,9 +26,6 @@
 
 ;;; Code:
 
-(defvar spacemacs-whitespace-cleanup-globally nil
-  "If non nil then `spacemacs-whitespace-cleanup-mode' is applied globally.")
-
 ;;;###autoload
 (define-minor-mode spacemacs-whitespace-cleanup-mode
   "Minor mode to clean whitespace.
@@ -39,18 +36,47 @@ of the cleanup."
   :lighter " CleanW"
   :group 'spacemacs
   (if spacemacs-whitespace-cleanup-mode
-      (spacemacs-whitespace-cleanup//turn-on
-       spacemacs-whitespace-cleanup-globally)
-    (spacemacs-whitespace-cleanup//turn-off
-     spacemacs-whitespace-cleanup-globally)))
+      (if (eq dotspacemacs-whitespace-cleanup 'changed)
+          (ws-butler-mode)
+        (add-hook 'before-save-hook 'spacemacs-whitespace-cleanup/clean-up nil t))
+    ;; Always disable everything because `dotspacemacs-whitespace-cleanup' could
+    ;; have changed and the configuration reloaded.
+    (when (fboundp 'ws-butler-mode)
+        (ws-butler-mode -1))
+    (remove-hook 'before-save-hook 'spacemacs-whitespace-cleanup/clean-up t)))
 
 (define-global-minor-mode global-spacemacs-whitespace-cleanup-mode
   spacemacs-whitespace-cleanup-mode
-  (lambda ()
-    (let ((spacemacs-whitespace-cleanup-globally t))
-      (spacemacs-whitespace-cleanup-mode)))
+  spacemacs-whitespace-cleanup-mode
   :group 'spacemacs
-  :require 'spacemacs-whitespace-cleanup-mode)
+  :require 'spacemacs-whitespace-cleanup-mode
+  :predicate '(not markdown-mode))
+
+(defun spacemacs-whitespace-cleanup/clean-up (&optional called-interactively)
+  "When `dotspacemacs-whitespace-cleanup' is set
+to `all' or `trailing', this function is automatically called as
+part of the `before-save-hook'. It can also be called manually.
+Note that it has no effect if `dotspacemacs-whitespace-cleanup'
+is `changed'."
+  (interactive "p")
+  (pcase dotspacemacs-whitespace-cleanup
+    ('all
+     (whitespace-cleanup))
+    ('trailing
+     (delete-trailing-whitespace))
+    ('changed
+     (if called-interactively
+         (user-error "`spacemacs-whitespace-cleanup/clean-up' has no effect because
+`dotspacemacs-whitespace-cleanup' is `changed'. Hence whitespace
+cleanup is exclusively handled by `ws-butler-mode'. Consider
+changing this dotfile variable, or directly using
+`whitespace-cleanup' or `delete-trailing-whitespace'."))
+     (error "Error: `spacemacs-whitespace-cleanup/clean-up' was called even
+though the value of `dotspacemacs-whitespace-cleanup' is
+`changed'."))
+    (x (user-error
+        "%s is not a valid option for 'dotspacemacs-whitespace-cleanup'"
+        x))))
 
 (defun spacemacs-whitespace-cleanup/on-message (&optional global)
   "Return a string to display when the mode is activated."
@@ -64,27 +90,5 @@ of the cleanup."
                 "%s is not a valid option for 'dotspacemacs-whitespace-cleanup'"
                 x)))))
 
-(defun spacemacs-whitespace-cleanup//turn-on (&optional global)
-  "Turn on `spacemacs-whitespace-cleanup-mode'."
-  (pcase dotspacemacs-whitespace-cleanup
-    ('all
-     (add-hook 'before-save-hook 'whitespace-cleanup nil (not global)))
-    ('trailing
-     (add-hook 'before-save-hook 'delete-trailing-whitespace nil (not global)))
-    ('changed
-     (when (fboundp 'ws-butler-mode)
-       (if global (ws-butler-global-mode) (ws-butler-mode))))))
-
-(defun spacemacs-whitespace-cleanup//turn-off (&optional global)
-  "Turn off `spacemacs-whitespace-cleanup-mode'."
-  (pcase dotspacemacs-whitespace-cleanup
-    ('all
-     (remove-hook 'before-save-hook 'whitespace-cleanup (not global)))
-    ('trailing
-     (remove-hook 'before-save-hook 'delete-trailing-whitespace (not global)))
-    ('changed
-     (when (fboundp 'ws-butler-mode)
-       (if global (ws-butler-global-mode -1) (ws-butler-mode -1))))))
-
 (provide 'spacemacs-whitespace-cleanup)
 ;;; spacemacs-whitespace-cleanup.el ends here.

layers/+spacemacs/spacemacs-editing/packages.el

@@ -470,14 +470,12 @@
     (spacemacs|add-toggle global-whitespace-cleanup
       :mode global-spacemacs-whitespace-cleanup-mode
       :status spacemacs-whitespace-cleanup-mode
-      :on (let ((spacemacs-whitespace-cleanup-globally t))
-            (spacemacs-whitespace-cleanup-mode))
-      :off (let ((spacemacs-whitespace-cleanup-globally t))
-             (spacemacs-whitespace-cleanup-mode -1))
       :on-message (spacemacs-whitespace-cleanup/on-message t)
       :documentation "Global automatic whitespace clean up."
       :evil-leader "t C-S-w")
     (with-eval-after-load 'ws-butler
+      ;; handle reloading configuration
+      (spacemacs/toggle-global-whitespace-cleanup-off)
       (when dotspacemacs-whitespace-cleanup
         (spacemacs/toggle-global-whitespace-cleanup-on)))
     :config