From 31edc86a54d20c3c73d0ebad247fde2c35e6a964 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 3 May 2026 21:11:26 -0500 Subject: fix: scope projectile cache revert state to each compile The projectile compile/test/run cache-revert protection in `dev-fkeys.el` used a single global variable, `cj/--projectile-revert-state`. Two overlapping compiles could clobber each other's state. The second compile's capture would overwrite the first's. So when the first compile finished and ran the global finish-hook, it'd act on the wrong project's state, or revert nothing because the keys had drifted. I moved the state into a closure. `cj/--projectile-capture-cmd` now returns the state plist instead of mutating the global. `cj/--projectile-around-revert` captures the state into a local, calls the projectile cmd-runner, and installs a one-shot buffer-local finish hook on the returned compilation buffer. The hook closes over its own state plist, so two compiles can finish in any order and each one acts on the right project. I extracted three small helpers along the way. `cj/--projectile-revert-state-on-fail` is the pure decision (revert when failed AND modified AND prior was non-nil). `cj/--projectile-make-revert-on-fail-hook` builds the closure-based one-shot hook. `cj/--projectile-compilation-buffer` normalizes a buffer-or-process result from projectile into a buffer. The legacy `cj/--projectile-revert-on-fail` function still reads the global `cj/--projectile-revert-state`. It stays around for the existing direct tests, but its core logic now delegates to the extracted state-on-fail helper. No production caller adds it to `compilation-finish-functions` anymore. I added one regression test in `test-dev-fkeys--projectile-around-revert.el`: two projectile invocations on different projects, finishes triggered out of order, each compile reverts its own project's cache and leaves the other alone. The capture and around-advice tests were rewritten to match the new return-style API and to assert hooks land buffer-locally rather than globally. 19 projectile-related tests pass together. --- tests/test-dev-fkeys--projectile-around-revert.el | 79 +++++++++++++++++++---- tests/test-dev-fkeys--projectile-capture-cmd.el | 28 ++++---- 2 files changed, 82 insertions(+), 25 deletions(-) (limited to 'tests') diff --git a/tests/test-dev-fkeys--projectile-around-revert.el b/tests/test-dev-fkeys--projectile-around-revert.el index bdcb11a1..012c4258 100644 --- a/tests/test-dev-fkeys--projectile-around-revert.el +++ b/tests/test-dev-fkeys--projectile-around-revert.el @@ -33,29 +33,81 @@ (should (equal calls '((arg1 arg2)))))) (ert-deftest test-dev-fkeys-projectile-around-revert-captures-prior () - "Normal: advice captures the prior cmd into the revert state." + "Normal: advice captures the prior cmd into the buffer-local hook." (let ((cj/--projectile-revert-state nil) (compilation-finish-functions nil) (projectile-compile-cmd-map (make-hash-table :test 'equal))) (puthash "/p/" "make build" projectile-compile-cmd-map) (cl-letf (((symbol-function 'cj/--f4-project-root) (lambda () "/p/"))) - (cj/--projectile-around-revert - 'projectile-compile-cmd-map - (lambda (&rest _) nil))) - (should (equal (plist-get cj/--projectile-revert-state :prior) - "make build")))) + (let ((compile-buffer (get-buffer-create " *compile-capture*"))) + (unwind-protect + (progn + (cj/--projectile-around-revert + 'projectile-compile-cmd-map + (lambda (&rest _) compile-buffer)) + (puthash "/p/" "make typo" projectile-compile-cmd-map) + (with-current-buffer compile-buffer + (run-hook-with-args 'compilation-finish-functions + compile-buffer "exited abnormally\n")) + (should (equal (gethash "/p/" projectile-compile-cmd-map) + "make build"))) + (kill-buffer compile-buffer)))))) (ert-deftest test-dev-fkeys-projectile-around-revert-installs-finish-hook () - "Normal: advice adds the revert-on-fail hook to compilation-finish-functions." + "Normal: advice adds a buffer-local revert hook to the compilation buffer." (let ((cj/--projectile-revert-state nil) (compilation-finish-functions nil) (projectile-compile-cmd-map (make-hash-table :test 'equal))) + (puthash "/p/" "make build" projectile-compile-cmd-map) (cl-letf (((symbol-function 'cj/--f4-project-root) (lambda () "/p/"))) - (cj/--projectile-around-revert - 'projectile-compile-cmd-map - (lambda (&rest _) nil))) - (should (member #'cj/--projectile-revert-on-fail - compilation-finish-functions)))) + (with-current-buffer (get-buffer-create " *compile-a*") + (setq-local compilation-finish-functions nil)) + (unwind-protect + (let ((compile-buffer + (cj/--projectile-around-revert + 'projectile-compile-cmd-map + (lambda (&rest _) (get-buffer-create " *compile-a*"))))) + (should-not compilation-finish-functions) + (with-current-buffer compile-buffer + (should compilation-finish-functions))) + (kill-buffer " *compile-a*"))))) + +(ert-deftest test-dev-fkeys-projectile-around-revert-overlapping-compiles-use-own-state () + "Regression: overlapping compiles finishing out of order use their own state." + (let ((cj/--projectile-revert-state nil) + (compilation-finish-functions nil) + (projectile-compile-cmd-map (make-hash-table :test 'equal)) + (roots '("/one/" "/two/"))) + (puthash "/one/" "make one" projectile-compile-cmd-map) + (puthash "/two/" "make two" projectile-compile-cmd-map) + (cl-letf (((symbol-function 'cj/--f4-project-root) + (lambda () (pop roots)))) + (let ((buf-one (get-buffer-create " *compile-one*")) + (buf-two (get-buffer-create " *compile-two*"))) + (unwind-protect + (progn + (cj/--projectile-around-revert + 'projectile-compile-cmd-map + (lambda (&rest _) buf-one)) + (cj/--projectile-around-revert + 'projectile-compile-cmd-map + (lambda (&rest _) buf-two)) + (puthash "/one/" "make one typo" projectile-compile-cmd-map) + (puthash "/two/" "make two typo" projectile-compile-cmd-map) + (with-current-buffer buf-two + (run-hook-with-args 'compilation-finish-functions + buf-two "exited abnormally\n")) + (should (string= (gethash "/two/" projectile-compile-cmd-map) + "make two")) + (should (string= (gethash "/one/" projectile-compile-cmd-map) + "make one typo")) + (with-current-buffer buf-one + (run-hook-with-args 'compilation-finish-functions + buf-one "exited abnormally\n")) + (should (string= (gethash "/one/" projectile-compile-cmd-map) + "make one"))) + (kill-buffer buf-one) + (kill-buffer buf-two)))))) ;;; Boundary Cases @@ -70,7 +122,8 @@ The state stays nil so the finish hook will be a no-op too." 'projectile-compile-cmd-map (lambda (&rest _) (cl-incf calls)))) (should (= calls 1)) - (should (null cj/--projectile-revert-state)))) + (should (null cj/--projectile-revert-state)) + (should-not compilation-finish-functions))) (provide 'test-dev-fkeys--projectile-around-revert) ;;; test-dev-fkeys--projectile-around-revert.el ends here diff --git a/tests/test-dev-fkeys--projectile-capture-cmd.el b/tests/test-dev-fkeys--projectile-capture-cmd.el index 92309198..bc4c7684 100644 --- a/tests/test-dev-fkeys--projectile-capture-cmd.el +++ b/tests/test-dev-fkeys--projectile-capture-cmd.el @@ -2,9 +2,9 @@ ;;; Commentary: ;; Tests for the prior-cmd capture helper used by the auto-revert advice. -;; Captures the current cached cmd at the project root into -;; `cj/--projectile-revert-state' so a later finish-hook can restore it -;; if the compile fails after the cmd was modified. +;; Captures the current cached cmd at the project root into a plist so a +;; later finish-hook can restore it if the compile fails after the cmd was +;; modified. ;;; Code: @@ -24,25 +24,29 @@ (ert-deftest test-dev-fkeys-projectile-capture-cmd-stores-prior-value () "Normal: captures the cached cmd at the project root into the state plist." (let* ((cj/--projectile-revert-state nil) - (projectile-compile-cmd-map (make-hash-table :test 'equal))) + (projectile-compile-cmd-map (make-hash-table :test 'equal)) + state) (puthash "/p/" "make build" projectile-compile-cmd-map) (cl-letf (((symbol-function 'cj/--f4-project-root) (lambda () "/p/"))) - (cj/--projectile-capture-cmd 'projectile-compile-cmd-map)) - (should (equal (plist-get cj/--projectile-revert-state :map) + (setq state (cj/--projectile-capture-cmd 'projectile-compile-cmd-map))) + (should (equal (plist-get state :map) 'projectile-compile-cmd-map)) - (should (equal (plist-get cj/--projectile-revert-state :root) "/p/")) - (should (equal (plist-get cj/--projectile-revert-state :prior) "make build")))) + (should (equal (plist-get state :root) "/p/")) + (should (equal (plist-get state :prior) "make build")) + (should (null cj/--projectile-revert-state)))) (ert-deftest test-dev-fkeys-projectile-capture-cmd-no-prior-stores-nil () "Normal: when no cmd is cached, captures :prior nil — distinct from \"didn't capture at all\" because :map and :root are still set." (let* ((cj/--projectile-revert-state nil) - (projectile-test-cmd-map (make-hash-table :test 'equal))) + (projectile-test-cmd-map (make-hash-table :test 'equal)) + state) (cl-letf (((symbol-function 'cj/--f4-project-root) (lambda () "/p/"))) - (cj/--projectile-capture-cmd 'projectile-test-cmd-map)) - (should (eq (plist-get cj/--projectile-revert-state :map) + (setq state (cj/--projectile-capture-cmd 'projectile-test-cmd-map))) + (should (eq (plist-get state :map) 'projectile-test-cmd-map)) - (should (null (plist-get cj/--projectile-revert-state :prior))))) + (should (null (plist-get state :prior))) + (should (null cj/--projectile-revert-state)))) ;;; Boundary Cases -- cgit v1.2.3