From 2f8d8989856073cbed5f9159d02089903bc5343e Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Sun, 3 May 2026 21:17:30 -0500 Subject: refactor: defer projectile revert advice until projectile loads `dev-fkeys.el` was wiring its three Projectile cache-revert advices via top-level `advice-add` calls using `apply-partially #'cj/--projectile-around-revert `. That had three problems. The advice values were anonymous closures, so `advice-member-p` couldn't find them and a re-load would silently double-install. The implicit dependency on Projectile was load-ordered by accident. If `dev-fkeys.el` happened to require before Projectile loaded, the advice still attached to unbound symbols. And a fresh batch require of `dev-fkeys.el` for tests would always force the advice attempt regardless of whether Projectile was around. I gave each Projectile target a named advice wrapper (`cj/--projectile-compile-around-revert`, `cj/--projectile-test-around-revert`, `cj/--projectile-run-around-revert`) and put the (target . advice) pairs in a `cj/--projectile-revert-advice-specs` defconst. `cj/--projectile-install-revert-advice` walks the specs, checks `fboundp` plus `advice-member-p`, and only adds advice that's missing. The installer is idempotent on reload, and the named wrappers make it easy to tear down later by symbol name. `cj/--projectile-register-revert-advice` is the entry point at module load time. It installs immediately when Projectile is already a `featurep`, otherwise it schedules the installer through `eval-after-load 'projectile`. Either way the advice is in place once Projectile is available, and `dev-fkeys.el` no longer relies on a particular load order. Tests in the new `tests/test-dev-fkeys--projectile-advice-install.el` cover four cases. Registration defers via `eval-after-load` when Projectile isn't a feature yet. Registration installs immediately when it is. Install skips unbound Projectile functions. Install advises each bound Projectile command runner with the matching named wrapper. 23 projectile-related tests pass together. --- tests/test-dev-fkeys--projectile-advice-install.el | 86 ++++++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/test-dev-fkeys--projectile-advice-install.el (limited to 'tests') diff --git a/tests/test-dev-fkeys--projectile-advice-install.el b/tests/test-dev-fkeys--projectile-advice-install.el new file mode 100644 index 00000000..bfa9b691 --- /dev/null +++ b/tests/test-dev-fkeys--projectile-advice-install.el @@ -0,0 +1,86 @@ +;;; test-dev-fkeys--projectile-advice-install.el --- Tests for Projectile advice setup -*- lexical-binding: t -*- + +;;; Commentary: +;; Smoke tests for the load-order contract between dev-fkeys.el and Projectile. +;; Requiring dev-fkeys should not force Projectile to load, but the command-cache +;; revert advice should be installed once Projectile is available. + +;;; Code: + +(require 'ert) +(require 'cl-lib) +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'dev-fkeys) + +(ert-deftest test-dev-fkeys-projectile-advice-register-defers-until-projectile-loads () + "When Projectile is not loaded, registration should use `eval-after-load'." + (let (registered-feature registered-form install-called) + (cl-letf (((symbol-function 'featurep) + (lambda (feature) (and (not (eq feature 'projectile)) + (featurep feature)))) + ((symbol-function 'eval-after-load) + (lambda (feature form) + (setq registered-feature feature + registered-form form))) + ((symbol-function 'cj/--projectile-install-revert-advice) + (lambda () (setq install-called t)))) + (cj/--projectile-register-revert-advice)) + (should (eq registered-feature 'projectile)) + (should (equal registered-form '(cj/--projectile-install-revert-advice))) + (should-not install-called))) + +(ert-deftest test-dev-fkeys-projectile-advice-register-installs-when-projectile-loaded () + "When Projectile is already loaded, registration should install immediately." + (let (install-called eval-after-load-called) + (cl-letf (((symbol-function 'featurep) + (lambda (feature) (eq feature 'projectile))) + ((symbol-function 'eval-after-load) + (lambda (&rest _args) (setq eval-after-load-called t))) + ((symbol-function 'cj/--projectile-install-revert-advice) + (lambda () (setq install-called t)))) + (cj/--projectile-register-revert-advice)) + (should install-called) + (should-not eval-after-load-called))) + +(ert-deftest test-dev-fkeys-projectile-advice-install-skips-unbound-projectile-functions () + "The installer should not advise Projectile functions that are not bound." + (let (advised) + (cl-letf (((symbol-function 'fboundp) + (lambda (symbol) + (and (not (memq symbol '(projectile-compile-project + projectile-test-project + projectile-run-project))) + (fboundp symbol)))) + ((symbol-function 'advice-add) + (lambda (symbol &rest _args) + (push symbol advised)))) + (cj/--projectile-install-revert-advice)) + (should-not advised))) + +(ert-deftest test-dev-fkeys-projectile-advice-install-advises-bound-projectile-functions () + "The installer should advise each available Projectile command runner." + (let (advised) + (cl-letf (((symbol-function 'fboundp) + (lambda (symbol) + (or (memq symbol '(projectile-compile-project + projectile-test-project + projectile-run-project)) + (fboundp symbol)))) + ((symbol-function 'advice-member-p) + (lambda (&rest _args) nil)) + ((symbol-function 'advice-add) + (lambda (symbol _where function &rest _args) + (push (list symbol function) advised)))) + (cj/--projectile-install-revert-advice)) + (should (member '(projectile-compile-project + cj/--projectile-compile-around-revert) + advised)) + (should (member '(projectile-test-project + cj/--projectile-test-around-revert) + advised)) + (should (member '(projectile-run-project + cj/--projectile-run-around-revert) + advised)))) + +(provide 'test-dev-fkeys--projectile-advice-install) +;;; test-dev-fkeys--projectile-advice-install.el ends here -- cgit v1.2.3