From a70e8ee603b312f28c703f858a42a945f2ca56d5 Mon Sep 17 00:00:00 2001
From: Craig Jennings <c@cjennings.net>
Date: Sat, 16 May 2026 03:45:29 -0500
Subject: refactor(org-workflow): four hygiene fixes from the module-by-module
 re-review

- org-roam-config.el: extract `cj/--org-roam-should-copy-completed-task-p'
  and gate the `org-after-todo-state-change-hook' on it.  Skips
  fileless buffers (org-capture, indirect, temp Org) where
  `buffer-file-name' is nil and the downstream copy used to crash.
  Same gcal.org skip preserved.  Five existing tests updated to
  bind `buffer-file-name' inside `run-hooks' so the positive-case
  hook still fires.

- org-webclipper.el: drop the redundant
  `org-protocol-protocol-alist' registration inside
  `cj/webclipper-ensure-initialized'.  The
  `with-eval-after-load 'org-protocol' block at the bottom of the
  module is the single registration site now; comment in the
  initializer explains why.  Split the matching test into two:
  one for template registration (the initializer's actual job) and
  one for protocol registration (which now fires from the
  after-load block when `org-protocol' provides).

- org-webclipper.el: validate `:url' and `:title' in
  `cj/org-protocol-webclip'.  `:url' must be a non-empty string;
  `:title' must be a string when provided.  Signals `user-error'
  with the unexpected value instead of silently setting the
  globals to nil and failing downstream in the capture handler.

- mu4e-org-contacts-integration.el: declare `contacts-file' (via
  `eval-when-compile (defvar ...)') and `cj/get-all-contact-emails'
  (via `declare-function') near the top of the file.  Byte-compile
  in isolation no longer warns about free variables / unknown
  functions; the cross-module dependency is explicit at the top.
---
 tests/test-org-webclipper-commands.el | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

(limited to 'tests/test-org-webclipper-commands.el')

diff --git a/tests/test-org-webclipper-commands.el b/tests/test-org-webclipper-commands.el
index 3871774c..57c7b5fc 100644
--- a/tests/test-org-webclipper-commands.el
+++ b/tests/test-org-webclipper-commands.el
@@ -29,19 +29,27 @@
 
 ;;; cj/webclipper-ensure-initialized
 
-(ert-deftest test-webclipper-ensure-initialized-registers-protocol-and-templates ()
-  "Normal: first call sets up the protocol entry + W and w capture templates,
-and flips the initialized flag."
+(ert-deftest test-webclipper-ensure-initialized-registers-templates ()
+  "Normal: first call sets up the W and w capture templates and flips the
+initialized flag.  Protocol registration lives in the
+`with-eval-after-load 'org-protocol' block at the bottom of the module --
+asserted separately below in `test-webclipper-protocol-registered-via-after-load'."
   (let ((cj/webclipper-initialized nil)
         (org-protocol-protocol-alist nil)
         (org-capture-templates nil))
     (cl-letf (((symbol-function 'require) (lambda (&rest _) t)))
       (cj/webclipper-ensure-initialized))
     (should cj/webclipper-initialized)
-    (should (assoc "webclip" org-protocol-protocol-alist))
     (should (assoc "W" org-capture-templates))
     (should (assoc "w" org-capture-templates))))
 
+(ert-deftest test-webclipper-protocol-registered-via-after-load ()
+  "Loading org-webclipper installs a `with-eval-after-load 'org-protocol' block
+that registers the webclip entry.  Providing `'org-protocol' fires the block."
+  (let ((org-protocol-protocol-alist nil))
+    (provide 'org-protocol)
+    (should (assoc "webclip" org-protocol-protocol-alist))))
+
 (ert-deftest test-webclipper-ensure-initialized-is-idempotent ()
   "Boundary: second call doesn't re-register or duplicate templates."
   (let ((cj/webclipper-initialized nil)
-- 
cgit v1.2.3