On Thu, May 08 2025, Ihor Radchenko wrote:
> Kristoffer Balintona <[email protected]> writes:
>
>> Currently, `org-find-olp' assumes that it is being passed a file path +
>> an outline path, but when the olp function returns nil, only a file path
>> is returned, so `org-find-olp' ends up returning nil when
>> `org-capture-set-target-location' expects it to return a marker. Since
>> `org-find-olp' is always run when the outline-path parameter is
>> provided, we run into issues when it is a function returning nil.
>
>> Taking a look at the code, it seems like file+olp+datetree and file+olp
>> both behave like this.
>>
>> So my question is: which parts of the code should handle an outline-path
>> function returning nil? Should we change the behavior of `org-find-olp'
>> and/or `'org-capture-expand-olp', or should we change things just in
>> `org-capture-set-target-location'?
>
> I think that we should leave `org-find-olp' alone - it is used in more
> general scope (outside org-capture), and it is not at all clear where
> the marker to file-only path should point.
>
> In contrast, `org-capture-exapnd-olp' was introduced to handle various
> forms of the outline path specification in the capture template. If we
> want to add a new meaning to that specification (returned or explicit
> nil value), it is perfectly fine to change that function.
>
>> With respect to file+headline: I think a similar dilemma applies there.
>> It looks like the relevant function is `org-capture-expand-headline':
>> should we change that function to handle the case of its argument being
>> a function that returns nil or have that handling in
>> `org-capture-set-target-location'?
>
> `org-capture-expand-headline' serves the same purpose as
> `org-capture-expand-olp' - helper function to parse possible capture
> template specifications. We can extend it.
Thank you for the help. I've attached a diff for a set of proposed
changes. It has `org-capture-expand-olp' and
`org-capture-expand-headline' return the symbol 'file in certain cases
and has `org-capture-set-target-location' treat those cases specially,
inserting the capture entry at the top level. Additionally, I updated
the docstring of `org-capture-templates' appropriately and added an
additional test in `test-org-capture/org-capture-expand-olp'.
What do you think? If these changes are okay, I can turn them into an
actual patch. Although I've never contributed to org before, so I will
have to read up on the conventions for patches.
A separate matter:
I find it unusual that we have the file+olp and file target
specifications but not a file+datetree target specification. As a
result, file+olp+datetree accepts a nil olp to insert the datetree on
the top level but file+olp does not because that would just be
equivalent to the file target specification. I feel like we should
either add one or do something like removing the file specification and
having file+olp cover its case.
--
With appreciation,
Kristoffer
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 6d395406cf..f418e9fba9 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -205,24 +205,36 @@
(file+headline <file-spec> \"node headline\")
(file+headline <file-spec> function-returning-string)
(file+headline <file-spec> symbol-containing-string)
- Fast configuration if the target heading is unique in the file
+ Fast configuration if the target heading is unique in
+ the file. If function-returning-list-of-strings or
+ symbol-containing-list-of-strings return the symbol
+ \\='file,the entry will be inserted at the end of
+ <file-spec> on top level.
(file+olp <file-spec> \"Level 1 heading\" \"Level 2\" ...)
(file+olp <file-spec> function-returning-list-of-strings)
(file+olp <file-spec> symbol-containing-list-of-strings)
- For non-unique headings, the full outline path is safer
+ For non-unique headings, the full outline path is
+ safer. If function-returning-list-of-strings or
+ symbol-containing-list-of-strings return the symbol
+ \\='file,the entry will be inserted at the end of
+ <file-spec> on top level.
(file+regexp <file-spec> \"regexp to find location\")
File to the entry matching regexp
+ (file+olp+datetree <file-spec>)
(file+olp+datetree <file-spec> \"Level 1 heading\" ...)
(file+olp+datetree <file-spec> function-returning-list-of-strings)
(file+olp+datetree <file-spec> symbol-containing-list-of-strings)
Will create a heading in a date tree for today's date.
- If no heading is given, the tree will be on top level.
- To prompt for date instead of using TODAY, use the
- :time-prompt property. To create a week-tree, use the
- :tree-type property.
+ The tree will be inserted at the end of <file-spec> on
+ top level if no heading is given or if
+ function-returning-list-of-strings or
+ symbol-containing-list-of-strings return the symbol
+ \\='file. To prompt for date instead of using TODAY,
+ use the :time-prompt property. To create a week-tree,
+ use the :tree-type property.
(file+function <file-spec> function-finding-location)
A function to find the right location in the file
@@ -1055,18 +1067,28 @@
(widen)
(goto-char (point-min))
(setq headline (org-capture-expand-headline headline))
- (if (re-search-forward (format org-complex-heading-regexp-format
- (regexp-quote headline))
- nil t)
- (forward-line 0)
- (goto-char (point-max))
- (unless (bolp) (insert "\n"))
- (insert "* " headline "\n")
- (forward-line -1)))
+ (pcase headline
+ ('file (goto-char (point-max)))
+ ((pred stringp)
+ (re-search-forward (format org-complex-heading-regexp-format
+ (regexp-quote headline))
+ nil t)
+ (forward-line 0))
+ (_
+ (goto-char (point-max))
+ (unless (bolp) (insert "\n"))
+ (insert "* " headline "\n")
+ (forward-line -1))))
(`(file+olp ,path . ,(and outline-path (guard outline-path)))
(let* ((expanded-file-path (org-capture-expand-file path))
- (m (org-find-olp (cons expanded-file-path
- (apply #'org-capture-expand-olp expanded-file-path outline-path)))))
+ (expanded-olp (apply #'org-capture-expand-olp expanded-file-path outline-path))
+ ;; If expanded-olp is 'file, then create the datetree
+ ;; at the end of the file specified by
+ ;; expanded-file-path
+ (m (if (not (eq expanded-olp 'file))
+ (org-find-olp (cons expanded-file-path expanded-olp))
+ (set-buffer (org-capture-target-buffer expanded-file-path))
+ (save-restriction (widen) (point-max-marker)))))
(set-buffer (marker-buffer m))
(org-capture-put-target-region-and-position)
(widen)
@@ -1086,12 +1108,16 @@
(setq target-entry-p
(and (derived-mode-p 'org-mode) (org-at-heading-p)))))
(`(file+olp+datetree ,path . ,outline-path)
- (let ((m (if outline-path
- (let ((expanded-file-path (org-capture-expand-file path)))
- (org-find-olp (cons expanded-file-path
- (apply #'org-capture-expand-olp expanded-file-path outline-path))))
- (set-buffer (org-capture-target-buffer path))
- (point-marker))))
+ (let* ((expanded-file-path (org-capture-expand-file path))
+ (expanded-olp (apply #'org-capture-expand-olp expanded-file-path outline-path))
+ ;; If expanded-olp is 'file, then create the datetree
+ ;; at the end of the file specified by
+ ;; expanded-file-path
+ (file-level-datetree-p (eq expanded-olp 'file))
+ (m (if (not file-level-datetree-p)
+ (org-find-olp (cons expanded-file-path expanded-olp))
+ (set-buffer (org-capture-target-buffer expanded-file-path))
+ (save-restriction (widen) (point-max-marker)))))
(set-buffer (marker-buffer m))
(org-capture-put-target-region-and-position)
(widen)
@@ -1152,7 +1178,7 @@
(org-today))))
;; the following is the keep-restriction argument for
;; org-datetree-find-date-create
- (when outline-path 'subtree-at-point))))
+ (unless file-level-datetree-p 'subtree-at-point))))
(`(file+function ,path ,(and function (pred functionp)))
(set-buffer (org-capture-target-buffer path))
(org-capture-put-target-region-and-position)
@@ -1188,9 +1214,18 @@
(defun org-capture-expand-headline (headline)
"Expand functions, symbols and headline names for HEADLINE.
+Return a string representing a headline.
+
When HEADLINE is a function, call it. When it is a variable, return
its value. When it is a string, return it. In any other case, signal
-an error."
+an error.
+
+This function may also return the symbol \\='file if HEADLINE, when it
+is a function or variable, returns that symbol. Such cases will be
+interpreted by `org-capture-set-target-location' as a request to target
+the file directly (rather than a specific headline). This behavior
+allows users to dynamically set a template\\='s target location to a
+specific headline or the top-level of a file."
(let* ((final-headline (cond ((stringp headline) headline)
((functionp headline) (funcall headline))
((and (symbolp headline) (boundp headline))
@@ -1201,12 +1236,22 @@
(defun org-capture-expand-olp (file &rest olp)
"Expand functions, symbols and outline paths in FILE for OLP.
+Return a list of strings representing an outline path (OLP) in FILE.
+
When OLP is a function, call it with no arguments while the current
buffer is the FILE-visiting buffer. When it is a variable, return its
-value. When it is a list of string, return it. In any other case,
-signal an error."
+value. When it is a list of strings, return it. In any other case,
+signal an error.
+
+This function may also return the symbol \\='file if OLP, when it is a
+function or variable, returns that symbol. Such cases will be
+interpreted by `org-capture-set-target-location' as a request to target
+the file directly (rather than a specific outline path). This behavior
+allows users to dynamically set a template\\='s target location to a
+specific outline path or the top-level of a file."
(let* ((first (car olp))
- (final-olp (cond ((not (memq nil (mapcar #'stringp olp))) olp)
+ (final-olp (cond ((equal olp '(nil)) 'file)
+ ((not (memq nil (mapcar #'stringp olp))) olp)
((and (not (cdr olp)) (functionp first))
(with-current-buffer (find-file-noselect file)
(funcall first)))
