diff options
| -rw-r--r-- | modules/org-roam-config.el | 65 | ||||
| -rw-r--r-- | tests/test-org-roam-config-copy-and-move.el | 20 |
2 files changed, 63 insertions, 22 deletions
diff --git a/modules/org-roam-config.el b/modules/org-roam-config.el index da4b0657..c6e8977f 100644 --- a/modules/org-roam-config.el +++ b/modules/org-roam-config.el @@ -278,12 +278,23 @@ Returns the complete file content as a string." "#+FILETAGS: Topic\n\n" content)) +(defconst cj/move-org-branch-confirm-lines 30 + "Subtree size (in lines) at or above which `cj/move-org-branch-to-roam' confirms. +Cutting a large subtree by accident is jarring even though the move is +undoable, so the command asks first past this threshold.") + (defun cj/move-org-branch-to-roam () "Move the org subtree at point to a new org-roam node. The node filename will be timestamp-based with the heading name. The heading becomes the node title, and the entire subtree is demoted to level 1. If the heading contains a link, extract the description for the -title." +title. + +Safety: the new node file is written and verified on disk *before* the +subtree is cut from the source, so a failed write or db-sync can't lose +the subtree. The source buffer is left modified (not auto-saved) so the +cut stays undoable. A confirmation prompt guards large subtrees (see +`cj/move-org-branch-confirm-lines') and buffers with unsaved changes." (interactive) ;; Lazy load org and org-roam when needed (require 'org) @@ -304,29 +315,39 @@ title." (filename (format "%s-%s.org" timestamp title-slug)) (filepath (expand-file-name filename org-roam-directory)) ;; Generate a unique ID for the node - (node-id (org-id-new)) - ;; Store the subtree in a temporary buffer - subtree-content) - - ;; Copy the subtree content - (org-copy-subtree) - (setq subtree-content (current-kill 0)) + (node-id (org-id-new))) - ;; Now cut it to remove from original buffer - (org-cut-subtree) + ;; Refuse to overwrite an existing node file (timestamp collision, re-run). + (when (file-exists-p filepath) + (user-error "Roam node file already exists: %s" filepath)) - ;; Process the subtree to demote it to level 1 - (setq subtree-content (cj/--demote-org-subtree subtree-content current-level 1)) - - ;; Create the new org-roam file - (with-temp-file filepath - (insert (cj/--format-roam-node title node-id subtree-content))) - - ;; Sync the org-roam database - (org-roam-db-sync) - - ;; Message to user - (message "'%s' added as an org-roam node." title))) + ;; Copy the subtree (non-destructive) and build the node text up front. + (org-copy-subtree) + (let* ((subtree-content (cj/--demote-org-subtree (current-kill 0) current-level 1)) + (node-text (cj/--format-roam-node title node-id subtree-content)) + (line-count (length (split-string subtree-content "\n" t)))) + + ;; Confirm before the destructive cut for a large subtree or a buffer + ;; with unsaved changes, where recovery is murkier. + (when (or (>= line-count cj/move-org-branch-confirm-lines) + (buffer-modified-p)) + (unless (yes-or-no-p + (format "Move subtree \"%s\" (%d lines) to a new roam node? " + title line-count)) + (user-error "Aborted"))) + + ;; Write the new file BEFORE removing the source, and verify it landed, + ;; so a write failure can't lose the subtree. + (with-temp-file filepath + (insert node-text)) + (unless (file-exists-p filepath) + (error "Failed to create roam node file: %s" filepath)) + + ;; The subtree is safely on disk -- now cut it. The buffer is left + ;; modified and undoable (not auto-saved), so the move is reversible. + (org-cut-subtree) + (org-roam-db-sync) + (message "'%s' moved to a new org-roam node (%s)." title filename)))) ;; TASK: Need to decide keybindings before implementation and testing ;; (use-package consult-org-roam diff --git a/tests/test-org-roam-config-copy-and-move.el b/tests/test-org-roam-config-copy-and-move.el index 729afaa8..8b7c963f 100644 --- a/tests/test-org-roam-config-copy-and-move.el +++ b/tests/test-org-roam-config-copy-and-move.el @@ -119,6 +119,7 @@ syncs the roam db." (lambda () "11111111-2222-3333-4444-555555555555")) ((symbol-function 'org-roam-db-sync) (lambda (&rest _) (setq synced t))) + ((symbol-function 'yes-or-no-p) (lambda (&rest _) t)) ((symbol-function 'message) #'ignore)) (with-temp-buffer (org-mode) @@ -144,6 +145,25 @@ syncs the roam db." (should (string-match-p "^\\*\\* Sub heading" text)))))) (delete-directory roam-dir t)))) +(ert-deftest test-org-roam-move-branch-write-failure-preserves-source () + "Error: if writing the roam file fails, the source subtree is NOT cut. +The destructive cut must come after a successful write, so a failed write +(here: an unwritable roam directory) can't lose the subtree." + (let ((org-roam-directory "/no/such/cj-roam-move-dir/")) + (cl-letf (((symbol-function 'require) (lambda (&rest _) t)) + ((symbol-function 'org-id-new) (lambda () "id-1")) + ((symbol-function 'org-roam-db-sync) #'ignore) + ((symbol-function 'yes-or-no-p) (lambda (&rest _) t)) + ((symbol-function 'message) #'ignore)) + (with-temp-buffer + (org-mode) + (insert "* My Heading\n** Sub heading\nbody text\n") + (goto-char (point-min)) + (ignore-errors (cj/move-org-branch-to-roam)) + ;; The write failed before the cut, so the subtree is intact. + (should (string-match-p "My Heading" (buffer-string))) + (should (string-match-p "body text" (buffer-string))))))) + (ert-deftest test-org-roam-move-branch-errors-outside-heading () "Error: move-branch outside an org heading signals `user-error'." (cl-letf (((symbol-function 'require) (lambda (&rest _) t))) |