Fixed org-roam-promote-entire-buffer structure errors#2091
Fixed org-roam-promote-entire-buffer structure errors#2091jethrokuan merged 9 commits intoorg-roam:masterfrom
Conversation
- Safety checks before executing the transformation. - Insert the new TITLE line directly below the file-level properties drawer.
|
This might also fix #1870 |
- buffer must be saved for org operations to be reliable - don't use org-roam-set-keyword here, it shifts point - ensure that database is updated with changes afterwards
jethrokuan
left a comment
There was a problem hiding this comment.
I think this PR definitely fixes a bunch of issues existing in org-roam-extract-subtree and will make it strictly better, but fundamentally I think the approach of cut-paste-promote is brittle and needs to change. Open to merging after the comments have been addressed, but I'll need some time to rethink the right way to go about extraction.
org-roam-node.el
Outdated
|
|
||
| (defun org-roam--heading-levels () | ||
| "Extract the heading level structure from the current file." | ||
| (org-map-entries (lambda () (org-current-level)) |
There was a problem hiding this comment.
we don't want to be using org-map-entries here since it introduces dependencies on org-agenda related configuration. Use org-map-region istead as per line 848 to 854.
org-roam-node.el
Outdated
| "Verify that this buffer is promoteable: | ||
| There is a single level-1 heading | ||
| and no extra content before the first heading." | ||
| (let ((level-1-headings-count (-count (lambda (level) (eq 1 level)) (org-roam--heading-levels))) |
There was a problem hiding this comment.
let's not use anything from dash here, we have no explicit dependency on dash at the moment, although we do, I think, pull it in transitively.
org-roam-node.el
Outdated
| (and | ||
| (eq level-1-headings-count 1) | ||
| is-top-content-empty))) | ||
|
|
org-roam-node.el
Outdated
| (kill-line 1) | ||
| (org-roam-set-keyword "title" title) | ||
| (when tags (org-roam-set-keyword "filetags" tags))))) | ||
| (save-buffer) |
There was a problem hiding this comment.
as far as possible I want to avoid calling save-buffer here, is it necessary?
There was a problem hiding this comment.
I've removed it, but I'm worried that I ran into issues earlier where an unsaved buffer had node ID values in some state that broke things when the buffer was promoted. I can't reproduce the situation now, but I'll keep watching in case I can cause errors again. The org-roam-db-update-file is definitely needed to ensure that node state is persisted properly to the database.
org-roam-node.el
Outdated
| (insert "#+title: " title "\n") | ||
| (when tags (insert "#+filetags: " tags "\n")) | ||
| (org-roam-db-update-file))) | ||
| (user-error "Cannot promote: Multiple root headings or there is extra file-level text"))) |
There was a problem hiding this comment.
Let's error at the beginning in a when block instead.
org-roam-node.el
Outdated
| ;;(org-roam-set-keyword "title" title) | ||
| ;;(when tags (org-roam-set-keyword "filetags" tags)))) | ||
| (insert "#+title: " title "\n") | ||
| (when tags (insert "#+filetags: " tags "\n")) |
|
I agree that this warrants a proper rethink. Maybe this should really be treated as a special case of refile. |
- use org-map-region instead of org-map-entries - remove save-buffer - use org-roam-tag-add and org-get-tags to transfer tags from heading to file - avoid use of other packages (-count from dash)
|
I wonder if this fixes #2154 as well? |
ce5dbd1 to
7cb811e
Compare
7cb811e to
1ee4e16
Compare
Motivation for this change
Fixes #1857 and #2058
Changes
Run safety checks before executing
org-roam-promote-entire-buffer. These are unnecessary when it is called fromorg-roam-extract-subtree, but provide some extract protection if invoked directly.Replaced an invocation of
(org-roam-set-keyword "title" title)with explicit code, to avoid the situation whereorg-roam-set-keywordsteps over a line and injects the#+title:line into the middle of body text instead of at the top.Added a call to
save-bufferto preventorg-map-entriesfrom triggering a confusing & unwanted org-agenda prompt "Non-existent agenda file. [R]emove from list or [A]bort?".