diff options
| -rw-r--r-- | modules/org-agenda-config.el | 22 | ||||
| -rw-r--r-- | tests/test-org-agenda-config-category.el | 111 |
2 files changed, 119 insertions, 14 deletions
diff --git a/modules/org-agenda-config.el b/modules/org-agenda-config.el index 599a8abf..20714d5d 100644 --- a/modules/org-agenda-config.el +++ b/modules/org-agenda-config.el @@ -199,17 +199,27 @@ nil so the org default category applies." (defun cj/--org-set-todo-category () "Set buffer-local `org-category' to the project name for a todo.org buffer. -Runs from `org-mode-hook'. Only overrides when `org-category' is still -the default-from-filename (\"todo\"), so an explicit `#+CATEGORY:' in -the file keeps precedence." +Runs from `org-mode-hook'. Only overrides when nothing has set +`org-category', so an explicit `#+CATEGORY:' in the file keeps precedence. + +The nil test is the whole guard, and it took a while to get right. Org does +not assign the filename fallback to `org-category': with no `#+CATEGORY:' the +variable stays nil and `org-get-category' derives \"todo\" at read time. An +earlier version guarded on `(string= \"todo\" org-category)', which is a state +org never produces, so this hook did nothing from the day it shipped." (when (and buffer-file-name (boundp 'org-category) - (stringp org-category) - (string= "todo" org-category)) + (null org-category)) (when-let* ((project (cj/--org-todo-category-from-file buffer-file-name))) (setq-local org-category project)))) -(add-hook 'org-mode-hook #'cj/--org-set-todo-category) +;; Depth -100 so this runs FIRST, and it is load-bearing. `org-get-category' +;; resolves a deferred `:CATEGORY' that the org-element cache then holds, so any +;; hook that reads the category before this one freezes "todo" in that cache and +;; leaves the `setq-local' below inert -- `org-category' reads correct while the +;; agenda still shows "todo". `add-hook' prepends by default, so without the +;; depth every hook added later would run earlier, and three already do. +(add-hook 'org-mode-hook #'cj/--org-set-todo-category -100) ;; ------------------------ Org Agenda File List Cache ------------------------- ;; Cache agenda file list to avoid expensive directory scanning on every view. diff --git a/tests/test-org-agenda-config-category.el b/tests/test-org-agenda-config-category.el index 6a54d9e6..918d342a 100644 --- a/tests/test-org-agenda-config-category.el +++ b/tests/test-org-agenda-config-category.el @@ -88,12 +88,11 @@ Suppresses other org-mode hooks to keep the test isolated." (text-mode-hook nil)) (org-mode)) (setq buffer-file-name ,path) - ;; mimic org's default category (filename-sans-extension) so the - ;; hook's "only override the default" guard is exercised. - (setq-local org-category - (and ,path - (file-name-sans-extension - (file-name-nondirectory ,path)))) + ;; `org-category' is deliberately left nil, which is what org actually + ;; does with no `#+CATEGORY:'. An earlier version of this fixture set it + ;; to the filename base to "mimic org's default"; org never does that, so + ;; these tests passed against a state that cannot occur while the feature + ;; did nothing in practice. ,body-form)) ;;; Normal Cases @@ -106,11 +105,14 @@ Suppresses other org-mode hooks to keep the test isolated." (should (equal "emacs.d" org-category))))) (ert-deftest test-org-agenda-config-category-hook-normal-leaves-inbox-alone () - "Normal: hook leaves inbox.org's category at its filename default." + "Normal: hook declines on a non-todo file, leaving `org-category' unset. +Leaving it nil is the correct outcome, not a gap: `org-get-category' then +derives \"inbox\" from the filename, which is already a useful label. The +end-to-end test below checks that derivation on a real file." (test-org-agenda-config-category--with-file "/home/cjennings/sync/org/roam/inbox.org" (progn (cj/--org-set-todo-category) - (should (equal "inbox" org-category))))) + (should (null org-category))))) ;;; Boundary Cases @@ -133,5 +135,98 @@ Suppresses other org-mode hooks to keep the test isolated." ;; no error and no spurious mutation (should t))) +;;; ---------- End-to-end: a real file through the real hook ---------- +;; The fixture above sets `org-category' by hand to the filename base, to +;; "mimic org's default". Org does not do that: with no `#+CATEGORY:' it +;; leaves `org-category' nil and derives the fallback inside +;; `org-get-category' at read time. So those tests exercise a precondition +;; that never occurs, and passed while the feature did nothing in practice. +;; +;; These drive a real file on disk through `find-file-noselect' (which runs +;; `org-mode-hook' for real) and assert on `org-get-category', which is what +;; the agenda's %c column actually reads. + +(defmacro test-org-agenda-config-category--with-real-file (spec &rest body) + "Create FILE under a temp project dir and visit it, then run BODY. +SPEC is (VAR DIRNAME FILENAME CONTENT). VAR is bound to the live buffer." + (declare (indent 1)) + (let ((var (nth 0 spec)) (dirname (nth 1 spec)) + (filename (nth 2 spec)) (content (nth 3 spec))) + ;; The outer `unwind-protect' covers `find-file-noselect' itself, so a + ;; signal there still removes the temp tree rather than leaking it. + `(let* ((root (make-temp-file "cj-cat-" t)) + (project (expand-file-name ,dirname root)) + (path (expand-file-name ,filename project)) + (,var nil)) + (unwind-protect + (progn + (make-directory project t) + (with-temp-file path (insert ,content)) + (setq ,var (find-file-noselect path)) + ,@body) + (when (buffer-live-p ,var) + (with-current-buffer ,var (set-buffer-modified-p nil)) + (kill-buffer ,var)) + (delete-directory root t))))) + +(ert-deftest test-org-agenda-config-category-endtoend-todo-shows-project () + "Normal: visiting a project's todo.org makes the agenda show the project. +This is the whole point of the feature, and it is what the hand-built +fixture above could not check." + (test-org-agenda-config-category--with-real-file + (buffer "myproject" "todo.org" "* TODO a task\n") + (with-current-buffer buffer + (should (equal "myproject" (org-get-category (point-min))))))) + +(ert-deftest test-org-agenda-config-category-endtoend-explicit-category-wins () + "Boundary: an explicit `#+CATEGORY:' still beats the derived name. + +Note what this does and does not prove. It confirms the user-visible contract, +but not that the hook's guard is what enforces it: `org-element--get-category' +searches the buffer for `#+CATEGORY:' itself and returns that before consulting +`org-category', so the directive survives even with the guard deleted. The +guard's own job is covered by +`test-org-agenda-config-category-hook-boundary-respects-explicit'." + (test-org-agenda-config-category--with-real-file + (buffer "myproject" "todo.org" "#+CATEGORY: Personal\n* TODO a task\n") + (with-current-buffer buffer + (should (equal "Personal" (org-get-category (point-min))))))) + +(ert-deftest test-org-agenda-config-category-endtoend-survives-an-earlier-hook () + "Error: the override still lands when another hook reads the category first. + +`org-get-category' resolves a deferred `:CATEGORY' that the org-element cache +then holds. A hook running BEFORE ours that reads it freezes \"todo\" in that +cache, and our later `setq-local' becomes inert -- `org-category' reads correct +while the agenda still shows \"todo\". + +This is not hypothetical. `add-hook' prepends by default, so every hook added +after ours runs before it, and the live config already has three ahead of it. +The fix is the explicit depth on our `add-hook'; this test is what holds it +there." + (test-org-agenda-config-category--with-real-file + (buffer "projX" "todo.org" "* TODO a task\n") + (ignore buffer)) + ;; Re-visit with a nosy reader installed ahead of ours. + (let ((reader (lambda () (ignore (org-get-category (point-min)))))) + (unwind-protect + (progn + (add-hook 'org-mode-hook reader) + (test-org-agenda-config-category--with-real-file + (buffer "projX" "todo.org" "* TODO a task\n") + (with-current-buffer buffer + (should (equal "projX" (org-get-category (point-min))))))) + (remove-hook 'org-mode-hook reader)))) + +(ert-deftest test-org-agenda-config-category-endtoend-other-filename-untouched () + "Boundary: a distinctively-named file keeps its filename category. +Only todo.org is generic enough to be worth replacing. Files like +schedule.org or gcal.org already say something useful, and deriving from +the directory would collapse several of them onto one name." + (test-org-agenda-config-category--with-real-file + (buffer "data" "gcal.org" "* TODO an event\n") + (with-current-buffer buffer + (should (equal "gcal" (org-get-category (point-min))))))) + (provide 'test-org-agenda-config-category) ;;; test-org-agenda-config-category.el ends here |