From 7e0ac9d9fd2d7f539e07d785e4c26aa520513195 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Thu, 16 Jul 2026 14:35:56 -0500 Subject: fix(org-agenda): hoist cj/timeformat out of the defun it was buried in cj/timeformat was defvar'd inside cj/add-timestamp-to-org-entry, so the symbol wasn't special until the command had run once. Until then a let around the call bound it lexically and never reached the function. I moved it to top level with a docstring. It has no other callers. The nested defvar also poisons tests. ERT runs alphabetically, so an earlier test that calls the command makes the symbol special retroactively. A later special-variable-p check then passes on test ordering rather than on the code, reading green in a full-file run and red in isolation. The new guard snapshots special-variable-p into a defconst at load, before any test body runs. I filled in the missing coverage: point and the following line survive the insert, empty and unicode time strings, an empty buffer, and a read-only buffer signalling rather than dropping the stamp. The empty-string case pins a trailing space (current behavior, characterized rather than changed). --- modules/org-agenda-config.el | 5 +- tests/test-org-agenda-config-commands.el | 84 ++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/modules/org-agenda-config.el b/modules/org-agenda-config.el index d4407b28..632184e5 100644 --- a/modules/org-agenda-config.el +++ b/modules/org-agenda-config.el @@ -383,11 +383,14 @@ The agenda is rebuilt from all sources before display, including: ;; ------------------------- Add Timestamp To Org Entry ------------------------ ;; simply adds a timestamp to put the org entry on an agenda +(defvar cj/timeformat "%Y-%m-%d %a" + "Date format for the stamp `cj/add-timestamp-to-org-entry' inserts. +Must stay an org-readable date so the stamped line reaches the agenda.") + (defun cj/add-timestamp-to-org-entry (s) "Add an event with time S to appear underneath the line-at-point. This allows a line to show in an agenda without being scheduled or a deadline." (interactive "sTime: ") - (defvar cj/timeformat "%Y-%m-%d %a") (org-end-of-line) (save-excursion (open-line 1) diff --git a/tests/test-org-agenda-config-commands.el b/tests/test-org-agenda-config-commands.el index 76407439..139bf90f 100644 --- a/tests/test-org-agenda-config-commands.el +++ b/tests/test-org-agenda-config-commands.el @@ -176,5 +176,89 @@ large), so the standalone OVERDUE section was redundant." (should (string-match-p "<[0-9]\\{4\\}-[0-9]\\{2\\}-[0-9]\\{2\\} [A-Za-z]\\{3\\} 09:00>" text))))) +(ert-deftest test-org-agenda-add-timestamp-preserves-point-and-following-line () + "Normal: the stamp lands between the entry and the next line, point unmoved. +The docstring promises the event appears \"underneath the line-at-point\", +so neither the entry nor whatever follows it may be disturbed." + (with-temp-buffer + (insert "* First\n* Second") + (goto-char (point-min)) + (let ((start (progn (org-end-of-line) (point)))) + (goto-char (point-min)) + (cj/add-timestamp-to-org-entry "09:00") + ;; Point is left at the end of the entry it stamped. + (should (= (point) start))) + (let ((lines (split-string (buffer-string) "\n"))) + (should (equal (nth 0 lines) "* First")) + (should (string-match-p "\\`<[0-9]\\{4\\}-[0-9]\\{2\\}-[0-9]\\{2\\} [A-Za-z]\\{3\\} 09:00>\\'" + (nth 1 lines))) + (should (equal (nth 2 lines) "* Second"))))) + +(ert-deftest test-org-agenda-add-timestamp-empty-time-string () + "Boundary: an empty time yields a bare date stamp with a trailing space. +Characterizes current behavior -- the separator space is unconditional, so +an empty S produces `' rather than `'. Harmless in an agenda +\(org reads the date\), and pinned here so a future format change is a +deliberate one." + (with-temp-buffer + (insert "* Heading here") + (goto-char (point-min)) + (cj/add-timestamp-to-org-entry "") + (should (string-match-p "<[0-9]\\{4\\}-[0-9]\\{2\\}-[0-9]\\{2\\} [A-Za-z]\\{3\\} >" + (buffer-string))))) + +(ert-deftest test-org-agenda-add-timestamp-unicode-time-string () + "Boundary: non-ASCII in S survives into the stamp uncorrupted." + (with-temp-buffer + (insert "* Heading here") + (goto-char (point-min)) + (cj/add-timestamp-to-org-entry "09:00 café ☕") + (should (string-match-p "09:00 café ☕>" (buffer-string))))) + +(ert-deftest test-org-agenda-add-timestamp-empty-buffer () + "Boundary: an empty buffer still gets a stamp rather than signalling. +`org-end-of-line' and `open-line' both no-op safely at point-min." + (with-temp-buffer + (cj/add-timestamp-to-org-entry "09:00") + (should (string-match-p "<[0-9]\\{4\\}-[0-9]\\{2\\}-[0-9]\\{2\\} [A-Za-z]\\{3\\} 09:00>" + (buffer-string))))) + +(ert-deftest test-org-agenda-add-timestamp-read-only-buffer-signals () + "Error: a read-only buffer signals rather than silently dropping the stamp." + (with-temp-buffer + (insert "* Heading") + (goto-char (point-min)) + (setq buffer-read-only t) + (should-error (cj/add-timestamp-to-org-entry "09:00") :type 'buffer-read-only))) + +(defconst test-org-agenda--timeformat-special-at-load + (special-variable-p 'cj/timeformat) + "Whether `cj/timeformat' was special immediately after loading the module. +Captured here at load, before any test body runs, because the fact under +test is destroyed by observing it late: calling +`cj/add-timestamp-to-org-entry' once would execute a `defvar' nested in the +defun and make the symbol special retroactively. ERT runs tests +alphabetically, so an in-test `special-variable-p' check passes on the +strength of whichever test ran first -- green in a full-file run, red in +isolation. Snapshotting at load makes the guard order-independent.") + +(ert-deftest test-org-agenda-timeformat-is-a-top-level-special-variable () + "Normal: `cj/timeformat' is special and bound at load, not first call. +It used to be `defvar'd inside `cj/add-timestamp-to-org-entry', so it was +unbound until the command ran once and a `let' around the call bound it +lexically instead of dynamically. Pinning both halves of the fix: the +symbol is special at load time, and rebinding it actually reaches the +command." + (should test-org-agenda--timeformat-special-at-load) + (should (equal (default-value 'cj/timeformat) "%Y-%m-%d %a")) + ;; The dynamic binding must reach the insertion. + (with-temp-buffer + (insert "* Heading") + (goto-char (point-min)) + (let ((cj/timeformat "%Y")) + (cj/add-timestamp-to-org-entry "09:00")) + (should (string-match-p "\\`<[0-9]\\{4\\} 09:00>\\'" + (nth 1 (split-string (buffer-string) "\n")))))) + (provide 'test-org-agenda-config-commands) ;;; test-org-agenda-config-commands.el ends here -- cgit v1.2.3