I was recently trying to diagnose some weird behaviour I was seeing with
org-roam-capture where I was trying to insert a checkitem into an item list
under a specific heading in a specific file.
Sometimes it would correctly insert the item under the existing heading,
and sometimes it would fail to find the heading and instead append a new
heading to the end of the org file with the same name and insert the
checkitem under that new heading.
I eventually tracked down the problem to an issue with
org-capture--fill-template which is used to expand % placeholders in the
outline path components of an org-roam-capture-templates target of type
file+head+olp.
In org-capture--fill-template there is a section that creates a new buffer
named "*Capture*" and then activates this buffer so that it can insert the
template and perform various regexp search/replace operations on the
placeholders. However, the activation of this temporary buffer is performed
using (switch-to-buffer-other-window (get-buffer-create "*Capture*")) which
internally ends up dispatching to (display-buffer).
The problem with this is that it can modify the window layout, which can
cause existing windows to be resized, even if only temporarily. When a
window is resized, this can in turn cause (point) in the displayed buffer
to be changed so that (point) remains on-screen.
This means that, even though the (switch-to-buffer-other-window) occurs
within a (save-window-excursion) block which restores the original window
layout, it can still have the side effect of moving (point) in the buffer
of any active window.
This causes problems for org-roam's org-roam-capture-find-or-create-olp
function, which ends up calling (org-capture--fill-template) interleaved
with an incremental regex search through the target org-mode buffer. If the
target buffer is visible at the time of the capture, then the call to
(org-capture--fill-template) can potentially move its (point), affecting
the current position of the regex search.
The relevant part of org-roam-capture-find-or-create-olp (in
org-roam-capture.el) looks like this:
(org-with-wide-buffer
(goto-char start) ; <- here it
sets (point) to (point-min)
(dolist (heading olp) ;
(setq heading (org-roam-capture--fill-template heading)) ; <- Calls into
(org-capture--fill-template) which can move (point)
(let ((re (format org-complex-heading-regexp-format
(regexp-quote heading)))
(cnt 0))
(while (re-search-forward re end t) ; <- searching
from (point) to `end'
; might not
be searching from start of buffer
...) ; and so
might fail to find a match
...
...)
...))
In my case, I was observing the call to (org-roam-capture--fill-template
heading) moving (point) to a position after the heading that should have
matched the olp target and it subsequently failed to find a match and fell
through to the code-path that inserted a new heading at the end of the file
instead of inserting into the existing heading.
The behaviour does not always manifest as it depends on the window layout,
configuration of display-buffer-alist, whether the target buffer is
on-screen, the contents of the target buffer and the scroll position of the
buffer.
I managed to fix the behaviour locally by applying the following change to
org-capture--fill-template:
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1696,7 +1696,7 @@ Expansion occurs in a temporary Org mode buffer."
(message "no template") (ding)
(sit-for 1))
(save-window-excursion
- (switch-to-buffer-other-window (get-buffer-create "*Capture*"))
+ (set-buffer (get-buffer-create "*Capture*"))
(erase-buffer)
(setq buffer-file-name nil)
(setq mark-active nil)
This activates the buffer without changing the window layout.
Does this change/fix seem reasonable?
I suspect that the (save-window-excursion) should probably also be changed
to a (save-excursion), but was unable to convince myself so far that there
weren't other window-affecting operations in there.
Thanks,
Lewis.
