mirror of
https://github.com/org-roam/org-roam
synced 2025-09-24 16:30:55 -05:00
fix!(capture): move plain capture point past properties drawer
- Fix org-roam-capture--adjust-point-for-capture-type to properly handle cases where point is positioned after head content but not at a heading - Remove redundant location-type logic that incorrectly assumed point > 1 means we're at a heading (file+head can position point after keywords) - Restore call to adjust-point-for-capture-type that was removed ined94524
- Add comprehensive tests for plain capture ordering and assertion error fix - Refactor adjust-point-for-capture-type for better readability Amend:ed94524964
This commit is contained in:
@@ -468,7 +468,9 @@ Note: During the capture process this function is run by
|
|||||||
capture target."
|
capture target."
|
||||||
(if-let ((id (run-hook-with-args-until-success 'org-roam-capture-preface-hook)))
|
(if-let ((id (run-hook-with-args-until-success 'org-roam-capture-preface-hook)))
|
||||||
(org-roam-capture--put :id id)
|
(org-roam-capture--put :id id)
|
||||||
(org-roam-capture--setup-target-location))
|
(org-roam-capture--setup-target-location)
|
||||||
|
;; Adjust point for plain captures to skip past metadata (e.g. properties drawer)
|
||||||
|
(org-roam-capture--adjust-point-for-capture-type))
|
||||||
(let ((template (org-capture-get :template)))
|
(let ((template (org-capture-get :template)))
|
||||||
(when (stringp template)
|
(when (stringp template)
|
||||||
(org-capture-put
|
(org-capture-put
|
||||||
@@ -505,10 +507,12 @@ capture target."
|
|||||||
(set-buffer (org-capture-target-buffer path))
|
(set-buffer (org-capture-target-buffer path))
|
||||||
(when new-file-p
|
(when new-file-p
|
||||||
(org-roam-capture--put :new-file path)
|
(org-roam-capture--put :new-file path)
|
||||||
(insert (org-roam-capture--fill-template head 'ensure-newline)))
|
(insert (org-roam-capture--fill-template head 'ensure-newline))
|
||||||
|
(setq p (point-max)))
|
||||||
(widen)
|
(widen)
|
||||||
(setq p (goto-char (point-min))
|
(unless new-file-p
|
||||||
target-entry-p nil))
|
(setq p (goto-char (point-min))))
|
||||||
|
(setq target-entry-p nil))
|
||||||
(`(file+head+olp ,path ,head ,olp)
|
(`(file+head+olp ,path ,head ,olp)
|
||||||
(setq path (org-roam-capture--target-truepath path)
|
(setq path (org-roam-capture--target-truepath path)
|
||||||
new-file-p (org-roam-capture--new-file-p path))
|
new-file-p (org-roam-capture--new-file-p path))
|
||||||
@@ -686,27 +690,17 @@ POS is the current position of point (an integer) inside the
|
|||||||
currently active capture buffer, where the adjustment should
|
currently active capture buffer, where the adjustment should
|
||||||
start to begin from. If it's nil, then it will default to
|
start to begin from. If it's nil, then it will default to
|
||||||
the current value of `point'."
|
the current value of `point'."
|
||||||
(or pos (setq pos (point)))
|
(goto-char (or pos (point)))
|
||||||
(goto-char pos)
|
(pcase (org-capture-get :type)
|
||||||
(let ((location-type (if (= pos 1) 'beginning-of-file 'heading-at-point)))
|
(`plain
|
||||||
(and (eq location-type 'heading-at-point)
|
(if (org-capture-get :prepend)
|
||||||
(cl-assert (org-at-heading-p)))
|
(let ((el (org-element-at-point)))
|
||||||
(pcase (org-capture-get :type)
|
(while (and (not (eobp))
|
||||||
(`plain
|
(memq (org-element-type el)
|
||||||
(cl-case location-type
|
'(drawer property-drawer keyword comment comment-block horizontal-rule)))
|
||||||
(beginning-of-file
|
(goto-char (org-element-property :end el))
|
||||||
(if (org-capture-get :prepend)
|
(setq el (org-element-at-point))))
|
||||||
(let ((el (org-element-at-point)))
|
(goto-char (org-entry-end-position)))))
|
||||||
(while (and (not (eobp))
|
|
||||||
(memq (org-element-type el)
|
|
||||||
'(drawer property-drawer keyword comment comment-block horizontal-rule)))
|
|
||||||
(goto-char (org-element-property :end el))
|
|
||||||
(setq el (org-element-at-point))))
|
|
||||||
(goto-char (org-entry-end-position))))
|
|
||||||
(heading-at-point
|
|
||||||
(if (org-capture-get :prepend)
|
|
||||||
(org-end-of-meta-data t)
|
|
||||||
(goto-char (org-entry-end-position))))))))
|
|
||||||
(point))
|
(point))
|
||||||
|
|
||||||
;;; Capture implementation
|
;;; Capture implementation
|
||||||
|
@@ -245,6 +245,143 @@
|
|||||||
(expect (org-roam-capture--get :target-entry-p) :to-be nil))))
|
(expect (org-roam-capture--get :target-entry-p) :to-be nil))))
|
||||||
(delete-directory temp-dir t)))))
|
(delete-directory temp-dir t)))))
|
||||||
|
|
||||||
|
(describe "org-roam-capture plain type ordering"
|
||||||
|
:var ((temp-dir) (org-roam-directory) (org-roam-db-location))
|
||||||
|
|
||||||
|
(before-each
|
||||||
|
(setq temp-dir (make-temp-file "org-roam-test" t))
|
||||||
|
(setq org-roam-directory temp-dir)
|
||||||
|
(setq org-roam-db-location (expand-file-name "org-roam.db" temp-dir))
|
||||||
|
(org-roam-db-sync))
|
||||||
|
|
||||||
|
(after-each
|
||||||
|
(delete-directory temp-dir t))
|
||||||
|
|
||||||
|
(it "places properties drawer before captured content for plain type with file target"
|
||||||
|
(let* ((test-file (expand-file-name "test-plain-file.org" temp-dir))
|
||||||
|
(test-content "Test plain :target file")
|
||||||
|
(node (org-roam-node-create :title "Test Plain"))
|
||||||
|
(org-roam-capture--node node)
|
||||||
|
(org-roam-capture--info (make-hash-table :test 'equal)))
|
||||||
|
|
||||||
|
;; Call the setup directly to simulate capture without user interaction
|
||||||
|
(cl-letf* (((symbol-function 'org-capture-get)
|
||||||
|
(lambda (prop)
|
||||||
|
(pcase prop
|
||||||
|
(:type 'plain)
|
||||||
|
(:target-file test-file)
|
||||||
|
(_ nil))))
|
||||||
|
((symbol-function 'org-roam-capture--get-target)
|
||||||
|
(lambda () `(file ,test-file))))
|
||||||
|
|
||||||
|
;; Run the setup and insert content
|
||||||
|
(with-current-buffer (find-file-noselect test-file)
|
||||||
|
(org-roam-capture--setup-target-location)
|
||||||
|
(org-roam-capture--adjust-point-for-capture-type)
|
||||||
|
(insert test-content)
|
||||||
|
(save-buffer)))
|
||||||
|
|
||||||
|
;; Read the created file and check its structure
|
||||||
|
(with-temp-buffer
|
||||||
|
(insert-file-contents test-file)
|
||||||
|
(let ((buffer-content (buffer-string)))
|
||||||
|
|
||||||
|
;; The expected format is:
|
||||||
|
;; :PROPERTIES:
|
||||||
|
;; :ID: some-id
|
||||||
|
;; :END:
|
||||||
|
;; Test plain :target file
|
||||||
|
|
||||||
|
;; Check that properties come first
|
||||||
|
(expect buffer-content
|
||||||
|
:to-match
|
||||||
|
(rx bol ":PROPERTIES:"))
|
||||||
|
|
||||||
|
;; Verify ordering: properties, then content
|
||||||
|
(let ((props-pos (string-match ":PROPERTIES:" buffer-content))
|
||||||
|
(end-pos (string-match ":END:" buffer-content))
|
||||||
|
(content-pos (string-match (regexp-quote test-content) buffer-content)))
|
||||||
|
|
||||||
|
(expect props-pos :to-be 0)
|
||||||
|
(expect end-pos :not :to-be nil)
|
||||||
|
(expect end-pos :to-be-greater-than props-pos)
|
||||||
|
(expect content-pos :not :to-be nil)
|
||||||
|
(expect content-pos :to-be-greater-than end-pos))))))
|
||||||
|
|
||||||
|
(it "correctly orders buffer elements for plain type with file+head target"
|
||||||
|
(let* ((test-file (expand-file-name "test-plain-file-head.org" temp-dir))
|
||||||
|
(test-content "Test plain :target file+head")
|
||||||
|
(node (org-roam-node-create :title "plain file+head"))
|
||||||
|
(org-roam-capture--node node)
|
||||||
|
(org-roam-capture--info (make-hash-table :test 'equal)))
|
||||||
|
|
||||||
|
;; Populate capture info with title for template expansion
|
||||||
|
(puthash :title "plain file+head" org-roam-capture--info)
|
||||||
|
|
||||||
|
;; Call the setup directly to simulate capture without user interaction
|
||||||
|
(cl-letf* (((symbol-function 'org-capture-get)
|
||||||
|
(lambda (prop)
|
||||||
|
(pcase prop
|
||||||
|
(:type 'plain)
|
||||||
|
(:target-file test-file)
|
||||||
|
(_ nil))))
|
||||||
|
((symbol-function 'org-roam-capture--get-target)
|
||||||
|
(lambda () `(file+head ,test-file "#+title: ${title}\n"))))
|
||||||
|
|
||||||
|
;; Run the setup and insert content
|
||||||
|
(with-current-buffer (find-file-noselect test-file)
|
||||||
|
(org-roam-capture--setup-target-location)
|
||||||
|
(org-roam-capture--adjust-point-for-capture-type)
|
||||||
|
(insert test-content)
|
||||||
|
(save-buffer)))
|
||||||
|
|
||||||
|
;; Read the created file and check its structure
|
||||||
|
(with-temp-buffer
|
||||||
|
(insert-file-contents test-file)
|
||||||
|
(let ((buffer-content (buffer-string)))
|
||||||
|
|
||||||
|
;; The actual format according to org-mode property syntax is:
|
||||||
|
;; :PROPERTIES:
|
||||||
|
;; :ID: some-id
|
||||||
|
;; :END:
|
||||||
|
;; #+title: plain file+head
|
||||||
|
;; Test plain :target file+head
|
||||||
|
;;
|
||||||
|
;; This is correct - buffer-level properties must be at the top
|
||||||
|
|
||||||
|
;; Check that properties come first
|
||||||
|
(expect buffer-content
|
||||||
|
:to-match
|
||||||
|
(rx bol ":PROPERTIES:"))
|
||||||
|
|
||||||
|
;; Verify ordering: properties are at the top
|
||||||
|
(let ((props-pos (string-match ":PROPERTIES:" buffer-content))
|
||||||
|
(end-pos (string-match ":END:" buffer-content)))
|
||||||
|
|
||||||
|
;; Properties drawer should be first
|
||||||
|
(expect props-pos :to-be 0)
|
||||||
|
(expect end-pos :not :to-be nil)
|
||||||
|
(expect end-pos :to-be-greater-than props-pos))))))
|
||||||
|
|
||||||
|
(it "tests org-roam-capture--adjust-point-for-capture-type behavior"
|
||||||
|
;; Simple test to verify the fix for assertion error
|
||||||
|
(with-temp-buffer
|
||||||
|
(org-mode)
|
||||||
|
(insert "#+title: Test\n")
|
||||||
|
(goto-char (point-max))
|
||||||
|
|
||||||
|
;; Mock org-capture-get to return plain type
|
||||||
|
(cl-letf (((symbol-function 'org-capture-get)
|
||||||
|
(lambda (prop) (when (eq prop :type) 'plain))))
|
||||||
|
|
||||||
|
;; Document current state that previously caused assertion error
|
||||||
|
(let ((point-before (point))
|
||||||
|
(at-heading-before (org-at-heading-p)))
|
||||||
|
;; This should no longer trigger assertion error with our fix
|
||||||
|
(org-roam-capture--adjust-point-for-capture-type)
|
||||||
|
(expect point-before :to-be-greater-than 1)
|
||||||
|
(expect at-heading-before :to-be nil))))))
|
||||||
|
|
||||||
(provide 'test-org-roam-capture)
|
(provide 'test-org-roam-capture)
|
||||||
|
|
||||||
;;; test-org-roam-capture.el ends here
|
;;; test-org-roam-capture.el ends here
|
||||||
|
Reference in New Issue
Block a user