From a005c7636f30b710e27a6812ca989506d5df7531 Mon Sep 17 00:00:00 2001 From: Craig Jennings 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. --- modules/org-webclipper.el | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) (limited to 'modules/org-webclipper.el') diff --git a/modules/org-webclipper.el b/modules/org-webclipper.el index be4c1cb6..cf4cdbf0 100644 --- a/modules/org-webclipper.el +++ b/modules/org-webclipper.el @@ -68,12 +68,12 @@ (require 'org-web-tools) (require 'user-constants) ;; for webclipped-file - ;; Register the org-protocol handler - (add-to-list 'org-protocol-protocol-alist - '("webclip" - :protocol "webclip" - :function cj/org-protocol-webclip - :kill-client t)) + ;; The org-protocol handler registration lives in the + ;; `with-eval-after-load 'org-protocol' block at the bottom of + ;; this module -- that's the more robust home (it survives + ;; org-protocol being loaded before or after this module). Two + ;; registration sites would silently drift if the alist entry + ;; shape ever changes. ;; Add capture templates if not already present (unless (assoc "W" org-capture-templates) @@ -119,14 +119,23 @@ Returns the processed content as a string with: (defun cj/org-protocol-webclip (info) "Process org-protocol webclip requests. -INFO is a plist containing :url and :title from the org-protocol call." +INFO is a plist containing :url and :title from the org-protocol call. +Signals `user-error' when :url is missing, nil, empty, or non-string -- an +unexpected plist shape used to silently set the globals to nil and fail +downstream inside the capture handler with confusing messages." (cj/webclipper-ensure-initialized) (let ((url (plist-get info :url)) (title (plist-get info :title))) - (when url - ;; Store the URL and title for the capture template to use - (setq cj/webclip-current-url url - cj/webclip-current-title (or title "Untitled"))) + (unless (and (stringp url) (not (string-empty-p url))) + (user-error + "org-protocol webclip: expected non-empty :url string, got %S" url)) + (when (and title (not (stringp title))) + (user-error + "org-protocol webclip: :title must be a string when provided, got %S" + title)) + ;; Store the URL and title for the capture template to use + (setq cj/webclip-current-url url + cj/webclip-current-title (or title "Untitled")) ;; Trigger the capture (org-capture nil "W") nil)) ; Return nil to indicate we handled it -- cgit v1.2.3