From ed94524964a0b03538a84ae03c89ec61527ffe7d Mon Sep 17 00:00:00 2001 From: Jethro Kuan Date: Thu, 20 Jan 2022 14:46:22 -0800 Subject: [PATCH] (feat!)capture: change id creation to headline on entry-type capture-templates --- CHANGELOG.md | 3 + extensions/org-roam-dailies.el | 7 +- org-roam-capture.el | 79 +++++++++----- tests/test-org-roam-capture.el | 187 +++++++++++++++++++++++++++++++++ 4 files changed, 248 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd06186..10dfd5f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 2.3.2 +- [#2056](https://github.com/org-roam/org-roam/pull/2056) capture: IDs are now created for entries in capture templates + ## 2.3.1 (2025-06-26) * (fix): Use correct type specifications to suppress warnings by @okomestudio in https://github.com/org-roam/org-roam/pull/2522 diff --git a/extensions/org-roam-dailies.el b/extensions/org-roam-dailies.el index 01d56f2..64ab8c1 100644 --- a/extensions/org-roam-dailies.el +++ b/extensions/org-roam-dailies.el @@ -339,11 +339,12 @@ In this case, interactive selection will be bypassed." (when goto (run-hooks 'org-roam-dailies-find-file-hook))) (add-hook 'org-roam-capture-preface-hook #'org-roam-dailies--override-capture-time-h) + (defun org-roam-dailies--override-capture-time-h () "Override the `:default-time' with the time from `:override-default-time'." - (prog1 nil - (when (org-roam-capture--get :override-default-time) - (org-capture-put :default-time (org-roam-capture--get :override-default-time))))) + (when (org-roam-capture--get :override-default-time) + (org-capture-put :default-time (org-roam-capture--get :override-default-time))) + nil) ;;; Bindings (defvar org-roam-dailies-map (make-sparse-keymap) diff --git a/org-roam-capture.el b/org-roam-capture.el index 9fe047b..f1e4fe5 100644 --- a/org-roam-capture.el +++ b/org-roam-capture.el @@ -466,22 +466,21 @@ processing by `org-capture'. Note: During the capture process this function is run by `org-capture-set-target-location', as a (function ...) based capture target." - (let ((id (cond ((run-hook-with-args-until-success 'org-roam-capture-preface-hook)) - (t (org-roam-capture--setup-target-location))))) - (org-roam-capture--adjust-point-for-capture-type) - (let ((template (org-capture-get :template))) - (when (stringp template) - (org-capture-put - :template - (org-roam-capture--fill-template template)))) - (org-roam-capture--put :id id) - (org-roam-capture--put :finalize (or (org-capture-get :finalize) - (org-roam-capture--get :finalize))))) + (if-let ((id (run-hook-with-args-until-success 'org-roam-capture-preface-hook))) + (org-roam-capture--put :id id) + (org-roam-capture--setup-target-location)) + (let ((template (org-capture-get :template))) + (when (stringp template) + (org-capture-put + :template + (org-roam-capture--fill-template template)))) + (org-roam-capture--put :finalize (or (org-capture-get :finalize) + (org-roam-capture--get :finalize)))) (defun org-roam-capture--setup-target-location () - "Initialize the buffer, and goto the location of the new capture. -Return the ID of the location." - (let (p new-file-p) + "Initialize the buffer, and goto the location of the new capture." + (let ((target-entry-p t) + p new-file-p id) (pcase (org-roam-capture--get-target) (`(file ,path) (setq path (org-roam-capture--target-truepath path) @@ -489,7 +488,8 @@ Return the ID of the location." (when new-file-p (org-roam-capture--put :new-file path)) (set-buffer (org-capture-target-buffer path)) (widen) - (setq p (goto-char (point-min)))) + (setq p (goto-char (point-min)) + target-entry-p nil)) (`(file+olp ,path ,olp) (setq path (org-roam-capture--target-truepath path) new-file-p (org-roam-capture--new-file-p path)) @@ -507,7 +507,8 @@ Return the ID of the location." (org-roam-capture--put :new-file path) (insert (org-roam-capture--fill-template head 'ensure-newline))) (widen) - (setq p (goto-char (point-min)))) + (setq p (goto-char (point-min)) + target-entry-p nil)) (`(file+head+olp ,path ,head ,olp) (setq path (org-roam-capture--target-truepath path) new-file-p (org-roam-capture--new-file-p path)) @@ -569,17 +570,45 @@ Return the ID of the location." (user-error "No node with title or id \"%s\"" title-or-id)))) (set-buffer (org-capture-target-buffer (org-roam-node-file node))) (goto-char (org-roam-node-point node)) - (setq p (org-roam-node-point node))))) + (setq p (org-roam-node-point node) + target-entry-p (and (derived-mode-p 'org-mode) (org-at-heading-p)))))) ;; Setup `org-id' for the current capture target and return it back to the ;; caller. - (save-excursion - (goto-char p) - (if-let ((id (org-entry-get p "ID"))) - (setf (org-roam-node-id org-roam-capture--node) id) - (org-entry-put p "ID" (org-roam-node-id org-roam-capture--node))) - (prog1 - (org-id-get) - (run-hooks 'org-roam-capture-new-node-hook))))) + ;; Unless it's an entry type, then we want to create an ID for the entry instead + (pcase (org-capture-get :type) + ('entry + (advice-add #'org-capture-place-entry :after #'org-roam-capture--create-id-for-entry) + (org-roam-capture--put :new-node-p t) + (setq id (org-roam-node-id org-roam-capture--node))) + (_ + (save-excursion + (goto-char p) + (unless (org-entry-get p "ID") + (org-roam-capture--put :new-node-p t)) + (setq id (or (org-entry-get p "ID") + (org-roam-node-id org-roam-capture--node))) + (setf (org-roam-node-id org-roam-capture--node) id) + (org-entry-put p "ID" id)))) + (org-roam-capture--put :id id) + (org-roam-capture--put :target-entry-p target-entry-p) + (advice-add #'org-capture-place-template :before #'org-roam-capture--set-target-entry-p-a) + (advice-add #'org-capture-place-template :after #'org-roam-capture-run-new-node-hook-a))) + +(defun org-roam-capture--set-target-entry-p-a (_) + "Correct `:target-entry-p' in Org-capture template based on `:target.'." + (org-capture-put :target-entry-p (org-roam-capture--get :target-entry-p)) + (advice-remove #'org-capture-place-template #'org-roam-capture--set-target-entry-p-a)) + +(defun org-roam-capture-run-new-node-hook-a (_) + "Advice to run after the Org-capture template is placed." + (when (org-roam-capture--get :new-node-p) + (run-hooks 'org-roam-capture-new-node-hook)) + (advice-remove #'org-capture-place-template #'org-roam-capture--place-template-a)) + +(defun org-roam-capture--create-id-for-entry () + "Create the ID for the new entry." + (org-entry-put (point) "ID" (org-roam-capture--get :id)) + (advice-remove #'org-capture-place-entry #'org-roam-capture--create-id-for-entry)) (defun org-roam-capture--get-target () "Get the current capture :target for the capture template in use." diff --git a/tests/test-org-roam-capture.el b/tests/test-org-roam-capture.el index 85ea547..716f4e1 100644 --- a/tests/test-org-roam-capture.el +++ b/tests/test-org-roam-capture.el @@ -58,6 +58,193 @@ (org-roam-capture--fill-template (lambda () "foo")) :to-equal "foo"))) +(describe "org-roam-capture entry-type ID creation" + (it "creates ID for entry-type captures" + (let* ((temp-dir (make-temp-file "org-roam-test" t)) + (test-file (expand-file-name "test.org" temp-dir)) + (org-roam-directory temp-dir) + (org-roam-capture--node (org-roam-node-create :id (org-id-new))) + (org-roam-capture--info (make-hash-table :test 'equal)) + capture-id) + (unwind-protect + (progn + ;; Create initial file content + (with-temp-file test-file + (insert "#+TITLE: Test File\n\n* Parent Heading\n:PROPERTIES:\n:ID: parent-id\n:END:\n")) + + ;; Mock org-capture context and get-target + (cl-letf* (((symbol-function 'org-capture-get) + (lambda (prop) + (pcase prop + (:type 'entry) + (_ nil)))) + ((symbol-function 'org-roam-capture--get-target) + (lambda () `(file ,test-file)))) + + ;; Call the setup function + (with-current-buffer (find-file-noselect test-file) + (org-roam-capture--setup-target-location) + (setq capture-id (org-roam-capture--get :id))) + + ;; Verify ID was created and stored for entry type + (expect capture-id :not :to-be nil) + (expect (org-roam-capture--get :new-node-p) :to-be t))) + (delete-directory temp-dir t)))) + + (it "creates ID at target for non-entry-type captures" + (let* ((temp-dir (make-temp-file "org-roam-test" t)) + (test-file (expand-file-name "test-plain.org" temp-dir)) + (org-roam-directory temp-dir) + (org-roam-capture--node (org-roam-node-create :id (org-id-new))) + (org-roam-capture--info (make-hash-table :test 'equal)) + capture-id) + (unwind-protect + (progn + ;; Create initial empty file + (with-temp-file test-file + (insert "#+TITLE: Test\n")) + + ;; Mock org-capture context for plain type + (cl-letf* (((symbol-function 'org-capture-get) + (lambda (prop) + (pcase prop + (:type 'plain) + (_ nil)))) + ((symbol-function 'org-roam-capture--get-target) + (lambda () `(file ,test-file)))) + + ;; Call the setup function + (with-current-buffer (find-file-noselect test-file) + (org-roam-capture--setup-target-location) + (setq capture-id (org-roam-capture--get :id))) + + ;; For non-entry types, ID should be created at the target location + (expect capture-id :not :to-be nil) + (expect (org-roam-capture--get :new-node-p) :to-be t))) + (delete-directory temp-dir t))))) + +(describe "org-roam-capture advice functions" + :var ((org-roam-capture--info)) + + (before-each + (setq org-roam-capture--info (make-hash-table :test 'equal))) + + (it "org-roam-capture--create-id-for-entry creates and removes advice" + (cl-letf* ((entry-id nil) + ((symbol-function 'org-entry-put) + (lambda (pom prop val) + (when (string= prop "ID") + (setq entry-id val)))) + ((symbol-function 'org-roam-capture--get) + (lambda (prop) + (if (eq prop :id) + "test-id-123" + nil)))) + + ;; Add the advice + (advice-add #'org-capture-place-entry :after #'org-roam-capture--create-id-for-entry) + + ;; Call the function + (org-roam-capture--create-id-for-entry) + + ;; Verify ID was set and advice removed + (expect entry-id :to-equal "test-id-123") + (expect (advice-member-p #'org-roam-capture--create-id-for-entry + #'org-capture-place-entry) + :to-be nil))) + + (it "org-roam-capture--set-target-entry-p-a sets and removes advice" + (cl-letf* ((captured-value nil) + ((symbol-function 'org-capture-put) + (lambda (prop val) + (when (eq prop :target-entry-p) + (setq captured-value val)))) + ((symbol-function 'org-roam-capture--get) + (lambda (prop) + (if (eq prop :target-entry-p) + t + nil)))) + + ;; Add the advice + (advice-add #'org-capture-place-template :before #'org-roam-capture--set-target-entry-p-a) + + ;; Call the function + (org-roam-capture--set-target-entry-p-a nil) + + ;; Verify value was set and advice removed + (expect captured-value :to-be t) + (expect (advice-member-p #'org-roam-capture--set-target-entry-p-a + #'org-capture-place-template) + :to-be nil))) + + (it "org-roam-capture-run-new-node-hook-a runs hook when new node" + (let ((hook-ran nil)) + (cl-letf* (((symbol-function 'org-roam-capture--get) + (lambda (prop) + (if (eq prop :new-node-p) + t + nil)))) + + ;; Add test hook + (add-hook 'org-roam-capture-new-node-hook + (lambda () (setq hook-ran t))) + + ;; Add the advice + (advice-add #'org-capture-place-template :after #'org-roam-capture-run-new-node-hook-a) + + ;; Call the function + (org-roam-capture-run-new-node-hook-a nil) + + ;; Verify hook ran + (expect hook-ran :to-be t) + + ;; Clean up + (remove-hook 'org-roam-capture-new-node-hook + (lambda () (setq hook-ran t))))))) + +(describe "org-roam-capture target-entry-p detection" + (it "detects entry target for file+olp" + (let* ((temp-dir (make-temp-file "org-roam-test" t)) + (test-file (expand-file-name "test-olp.org" temp-dir)) + (org-roam-directory temp-dir) + (org-roam-capture--node (org-roam-node-create :id (org-id-new))) + (org-roam-capture--info (make-hash-table :test 'equal))) + (unwind-protect + (progn + (with-temp-file test-file + (insert "* Level 1\n** Level 2\n")) + + (cl-letf* (((symbol-function 'org-roam-capture--get-target) + (lambda () `(file+olp ,test-file ("Level 1" "Level 2")))) + ((symbol-function 'org-capture-get) + (lambda (prop) nil))) + + (with-current-buffer (find-file-noselect test-file) + (org-roam-capture--setup-target-location) + (expect (org-roam-capture--get :target-entry-p) :to-be t)))) + (delete-directory temp-dir t)))) + + (it "detects non-entry target for file" + (let* ((temp-dir (make-temp-file "org-roam-test" t)) + (test-file (expand-file-name "test-file.org" temp-dir)) + (org-roam-directory temp-dir) + (org-roam-capture--node (org-roam-node-create :id (org-id-new))) + (org-roam-capture--info (make-hash-table :test 'equal))) + (unwind-protect + (progn + (with-temp-file test-file + (insert "#+TITLE: Test\n")) + + (cl-letf* (((symbol-function 'org-roam-capture--get-target) + (lambda () `(file ,test-file))) + ((symbol-function 'org-capture-get) + (lambda (prop) nil))) + + (with-current-buffer (find-file-noselect test-file) + (org-roam-capture--setup-target-location) + (expect (org-roam-capture--get :target-entry-p) :to-be nil)))) + (delete-directory temp-dir t))))) + (provide 'test-org-roam-capture) ;;; test-org-roam-capture.el ends here