diff options
| -rw-r--r-- | modules/quick-video-capture.el | 28 | ||||
| -rw-r--r-- | tests/test-quick-video-capture--url-state.el | 75 |
2 files changed, 90 insertions, 13 deletions
diff --git a/modules/quick-video-capture.el b/modules/quick-video-capture.el index 0e9567dd..d74082e3 100644 --- a/modules/quick-video-capture.el +++ b/modules/quick-video-capture.el @@ -35,8 +35,12 @@ "JavaScript bookmarklet for triggering video downloads from the browser. Add this as a bookmark in your browser to enable one-click video downloads.") -(defvar cj/video-download-current-url nil - "Temporary storage for URL passed via org-protocol.") +(defvar cj/--video-download-url nil + "URL for the active video capture, dynamically bound around `org-capture'. +The org-protocol entry point `let'-binds this for the dynamic extent of +its capture call, so the capture handler sees the URL while the capture +runs, and an aborted or erroring capture unwinds the binding instead of +leaving a stale URL for the next manual capture.") (defvar cj/video-download-initialized nil "Flag to track if video download has been initialized.") @@ -46,23 +50,21 @@ Add this as a bookmark in your browser to enable one-click video downloads.") INFO is a plist containing :url from the org-protocol call." ;; Ensure we're initialized when called via protocol (cj/ensure-video-download-initialized) - (let ((url (plist-get info :url))) - (when url - ;; Store the URL for the capture template to use - (setq cj/video-download-current-url url)) - ;; Trigger the capture - (org-capture nil "v") - nil)) ; Return nil to indicate we handled it + ;; Bind the URL for the dynamic extent of the capture call only. The + ;; capture handler reads it while the capture runs; an aborted or + ;; erroring capture unwinds this binding, so nothing stale survives. + (let ((cj/--video-download-url (plist-get info :url))) + (org-capture nil "v")) + nil) ; Return nil to indicate we handled it (defun cj/video-download-capture-handler () "Handle video download during org-capture. -This function is called from the capture template." +This function is called from the capture template. It uses the URL +bound by the org-protocol entry point, or prompts when invoked manually." ;; Ensure media-utils is loaded when we actually need to download (require 'media-utils) - (let ((url (or cj/video-download-current-url + (let ((url (or cj/--video-download-url (read-string "Video URL: ")))) - ;; Clear the stored URL after using it - (setq cj/video-download-current-url nil) (if (string-empty-p url) (error "No URL provided for download") (cj/yt-dl-it url) diff --git a/tests/test-quick-video-capture--url-state.el b/tests/test-quick-video-capture--url-state.el new file mode 100644 index 00000000..6a0cd583 --- /dev/null +++ b/tests/test-quick-video-capture--url-state.el @@ -0,0 +1,75 @@ +;;; test-quick-video-capture--url-state.el --- Tests for video-capture URL state -*- lexical-binding: t; -*- + +;;; Commentary: +;; Unit tests for the URL handoff between the org-protocol entry point and the +;; capture handler in quick-video-capture.el. The URL is passed via a +;; dynamically-bound variable scoped to the org-capture call, so an aborted or +;; erroring capture cannot leave a stale URL behind for the next manual capture. + +;;; Code: + +(require 'ert) +(require 'cl-lib) + +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(require 'quick-video-capture) + +;;; cj/video-download-capture-handler + +(ert-deftest test-qvc-handler-downloads-bound-url () + "Normal: the handler downloads the dynamically-bound URL and saves nothing." + (let (downloaded) + (cl-letf (((symbol-function 'require) (lambda (&rest _) nil)) + ((symbol-function 'cj/yt-dl-it) (lambda (u) (setq downloaded u)))) + (let ((cj/--video-download-url "https://example.com/v")) + (should (equal "" (cj/video-download-capture-handler)))) + (should (equal "https://example.com/v" downloaded))))) + +(ert-deftest test-qvc-handler-prompts-when-no-bound-url () + "Boundary: with no bound URL the handler prompts the user." + (let (downloaded) + (cl-letf (((symbol-function 'require) (lambda (&rest _) nil)) + ((symbol-function 'cj/yt-dl-it) (lambda (u) (setq downloaded u))) + ((symbol-function 'read-string) (lambda (&rest _) "https://typed/v"))) + (let ((cj/--video-download-url nil)) + (cj/video-download-capture-handler)) + (should (equal "https://typed/v" downloaded))))) + +(ert-deftest test-qvc-handler-empty-url-errors () + "Error: an empty URL signals rather than downloading nothing." + (cl-letf (((symbol-function 'require) (lambda (&rest _) nil)) + ((symbol-function 'read-string) (lambda (&rest _) ""))) + (let ((cj/--video-download-url "")) + (should-error (cj/video-download-capture-handler))))) + +;;; cj/org-protocol-video-download — no global leak + +(ert-deftest test-qvc-protocol-binds-url-during-capture () + "Normal: the protocol handler makes the URL visible to the capture call." + (let (seen-during) + (cl-letf (((symbol-function 'cj/ensure-video-download-initialized) #'ignore) + ((symbol-function 'org-capture) + (lambda (&rest _) (setq seen-during cj/--video-download-url)))) + (cj/org-protocol-video-download '(:url "https://example.com/p"))) + (should (equal "https://example.com/p" seen-during)))) + +(ert-deftest test-qvc-protocol-leaves-no-stale-url () + "Boundary: after the protocol capture returns, no URL remains bound." + (cl-letf (((symbol-function 'cj/ensure-video-download-initialized) #'ignore) + ((symbol-function 'org-capture) #'ignore)) + (cj/org-protocol-video-download '(:url "https://example.com/p"))) + (should (null cj/--video-download-url))) + +(ert-deftest test-qvc-protocol-aborted-capture-clears-url () + "Error: a capture that errors/aborts mid-flow still leaves no stale URL. +This is the regression the dynamic binding prevents: the old global +setq survived an interrupted capture into the next manual one." + (cl-letf (((symbol-function 'cj/ensure-video-download-initialized) #'ignore) + ((symbol-function 'org-capture) + (lambda (&rest _) (error "simulated capture abort")))) + (ignore-errors + (cj/org-protocol-video-download '(:url "https://example.com/p")))) + (should (null cj/--video-download-url))) + +(provide 'test-quick-video-capture--url-state) +;;; test-quick-video-capture--url-state.el ends here |