Hi Ihor,
I apologize for the delay. I've been busy these last few days and also
encountered some technical difficulties with my device.
On Sun, May 11 2025, Ihor Radchenko wrote:
> Kristoffer Balintona <krisbalint...@gmail.com> writes:
>
>>> Why not just using nil value as you initially suggested?
>>
>> Currently, both `org-capture-expand-headline' and
>> `org-capture-expand-olp' error when the expanded headline or olp,
>> respectively, return nil. I wanted to preserve that assuming there was a
>> good reason behind this decision. Do you think interpreting a nil value
>> in this way is more appropriate?
>
> There is no need to preserve the behavior of
> `org-capture-expand-headline'/`org-capture-expand-olp'. Their purpose is
> basically parsing allowed values of olp/headline target. You are
> proposing to extend the allowed values, so it is 100% fine to handle nil
> without error - I will be a valid value now. The error should only be
> thrown when the user supplies invalid target.
Thank you for the clarification. WDYT of the patches attached?
>>> We used to have file+datetree target, and a number of others. They have
>>> been consolidated into a single file+olp+datetree in Org 9.1, 8 years ago.
>>> See
>>> https://list.orgmode.org/orgmode/CADn3Z2+AanLPvUO9ENhZ_2tkAvsmdnq9ewR3sFasn=zv--s...@mail.gmail.com/
>>
>> Should we do the same with file and file+olp? Or would that be too
>> backwards incompatible?
>
> Backwards compatibility per se is not a problem. For example,
> file+datetree target is still working. It is just deprecated.
> However, forcing users to change their setup (even just because they
> start seeing deprecation warning) without an obvious counter-benefit is
> not a good idea. I do not see any significant gain from consolidating
> file+olp and file targets.
I see. That's reasonable. I am not accustomed to the kinds of
considerations a package as big as org has, so I appreciate the insight.
--
With appreciation,
Kristoffer
From 6268653bb8ce866742401612e2fe8b4353884d6b Mon Sep 17 00:00:00 2001
From: Kristoffer Balintona <krisbalint...@gmail.com>
Date: Fri, 9 May 2025 11:40:13 -0500
Subject: [PATCH 1/2] lisp/org-capture.el: Accept optional and nil olp for the
file+olp+datetree and file+olp specifications
* lisp/org-capture.el (org-capture-expand-olp): When the supplied olp is nil,
org-capture-expand-olp now returns nil.
(org-capture-set-target-location): The file+olp+datetree and file+olp target
specifications now accept a nil or omitted olp. In either case, insert the
capture entry at the end of the buffer, on top level. Update docstring to
reflect these new behaviors.
* testing/lisp/test-org-capture.el (org-capture-expand-olp): Add test case for
org-capture-expand-olp now accepting a nil olp.
---
lisp/org-capture.el | 68 +++++++++++++++++++++-----------
testing/lisp/test-org-capture.el | 11 +++++-
2 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 6d395406c..3be1cf57c 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -207,20 +207,30 @@ target Specification of where the captured item should be placed.
(file+headline <file-spec> symbol-containing-string)
Fast configuration if the target heading is unique in the file
+ (file+olp <file-spec>)
(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 no headings are given, or if
+ function-returning-list-of-strings or
+ symbol-containing-list-of-strings return nil, 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
+ 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 nil. To
+ prompt for date instead of using TODAY, use the
:time-prompt property. To create a week-tree, use the
:tree-type property.
@@ -1065,8 +1075,13 @@ Store them in the capture property list."
(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 nil, then create the datetree at
+ ;; the end of the file specified by expanded-file-path
+ (m (if expanded-olp
+ (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 +1101,16 @@ Store them in the capture property list."
(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 nil, then file-level-datetree-p
+ ;; is non-nil and the datetree should be created at
+ ;; the end of the file specified by expanded-file-path
+ (file-level-datetree-p (not expanded-olp))
+ (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 +1171,7 @@ Store them in the capture property list."
(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)
@@ -1201,20 +1220,21 @@ an error."
(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."
- (let* ((first (car olp))
- (final-olp (cond ((not (memq nil (mapcar #'stringp olp))) olp)
- ((and (not (cdr olp)) (functionp first))
- (with-current-buffer (find-file-noselect file)
- (funcall first)))
- ((and (not (cdr olp)) (symbolp first) (boundp first))
- (symbol-value first))
- (t nil))))
- (or final-olp
- (error "org-capture: Invalid outline path target: %S" olp))))
+value. When it is a list of strings, return it. When OLP is nil,
+return nil. In any other case, signal an error."
+ (let* ((first (car olp)))
+ (cond ((and (= 1 (length olp)) (null first)) nil)
+ ((not (memq nil (mapcar #'stringp olp))) olp)
+ ((and (not (cdr olp)) (functionp first))
+ (with-current-buffer (find-file-noselect file)
+ (funcall first)))
+ ((and (not (cdr olp)) (symbolp first) (boundp first))
+ (symbol-value first))
+ (t (error "org-capture: Invalid outline path target: %S" olp)))))
(defun org-capture-expand-file (file)
"Expand functions, symbols and file names for FILE.
diff --git a/testing/lisp/test-org-capture.el b/testing/lisp/test-org-capture.el
index 7673347ed..a30549846 100644
--- a/testing/lisp/test-org-capture.el
+++ b/testing/lisp/test-org-capture.el
@@ -1010,7 +1010,7 @@ before\nglobal-before\nafter\nglobal-after"
(buffer-string))))))
(ert-deftest test-org-capture/org-capture-expand-olp ()
- "Test org-capture-expand-olp."
+ "Test `org-capture-expand-olp'."
;; `org-capture-expand-olp' accepts inlined outline path.
(should
(equal
@@ -1019,6 +1019,15 @@ before\nglobal-before\nafter\nglobal-after"
(unwind-protect
(org-capture-expand-olp file "A" "B" "C")
(delete-file file)))))
+ ;; `org-capture-expand-olp' should return nil if the outline path is
+ ;; nil
+ (should
+ (equal
+ nil
+ (let ((file (make-temp-file "org-test")))
+ (unwind-protect
+ (org-capture-expand-olp file nil)
+ (delete-file file)))))
;; The current buffer during the funcall of the lambda is the temporary
;; test file.
(should
--
2.49.0
From 4eecde72b939a871dc8076d13b6617e9e7d22bf9 Mon Sep 17 00:00:00 2001
From: Kristoffer Balintona <krisbalint...@gmail.com>
Date: Fri, 9 May 2025 11:40:13 -0500
Subject: [PATCH 2/2] lisp/org-capture.el: Accept optional or nil headline for
the file+headline specification
* lisp/org-capture.el (org-capture-expand-headline): When the supplied headline
is nil, org-capture-expand-olp now returns nil.
(org-capture-set-target-location): The file+headline target specification now
accepts a nil or omitted headline. In either case, insert the capture entry at
the end of the buffer, on top level. Update docstring to reflect these new
behaviors.
* testing/lisp/test-org-capture.el
(test-org-capture/org-capture-expand-headline): Add new test for
org-capture-expand-headline that tests these new behaviors as well as several
previously existing ones.
---
lisp/org-capture.el | 54 +++++++++++++++++++-------------
testing/lisp/test-org-capture.el | 42 +++++++++++++++++++++++++
2 files changed, 74 insertions(+), 22 deletions(-)
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 3be1cf57c..eb7f8394e 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -202,10 +202,15 @@ target Specification of where the captured item should be placed.
(id \"id of existing Org entry\")
File as child of this entry, or in the body of the entry
+ (file+headline <file-spec>)
(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 no heading is given, or if
+ function-returning-string or symbol-containing-string
+ return nil, the entry will be inserted at the end of
+ <file-spec> on top level.
(file+olp <file-spec>)
(file+olp <file-spec> \"Level 1 heading\" \"Level 2\" ...)
@@ -1050,7 +1055,7 @@ Store them in the capture property list."
(org-capture-put-target-region-and-position)
(goto-char position))
(_ (error "Cannot find target ID \"%s\"" id))))
- (`(file+headline ,path ,headline)
+ (`(file+headline ,path . ,headline)
(set-buffer (org-capture-target-buffer path))
;; Org expects the target file to be in Org mode, otherwise
;; it throws an error. However, the default notes files
@@ -1064,16 +1069,20 @@ Store them in the capture property list."
(org-capture-put-target-region-and-position)
(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)))
- (`(file+olp ,path . ,(and outline-path (guard outline-path)))
+ (setq headline (org-capture-expand-headline (car headline)))
+ (pcase headline
+ ((pred not) (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 . ,outline-path)
(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 nil, then create the datetree at
@@ -1207,16 +1216,17 @@ Store them in the capture property list."
(defun org-capture-expand-headline (headline)
"Expand functions, symbols and headline names for 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."
- (let* ((final-headline (cond ((stringp headline) headline)
- ((functionp headline) (funcall headline))
- ((and (symbolp headline) (boundp headline))
- (symbol-value headline))
- (t nil))))
- (or final-headline
- (error "org-capture: Invalid headline target: %S" 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. When headline is nil, return
+nil. In any other case, signal an error."
+ (cond ((null headline) nil)
+ ((stringp headline) headline)
+ ((functionp headline) (funcall headline))
+ ((and (symbolp headline) (boundp headline))
+ (symbol-value headline))
+ (t (error "org-capture: Invalid headline target: %S" headline))))
(defun org-capture-expand-olp (file &rest olp)
"Expand functions, symbols and outline paths in FILE for OLP.
diff --git a/testing/lisp/test-org-capture.el b/testing/lisp/test-org-capture.el
index a30549846..c518bfa1d 100644
--- a/testing/lisp/test-org-capture.el
+++ b/testing/lisp/test-org-capture.el
@@ -1009,6 +1009,48 @@ before\nglobal-before\nafter\nglobal-after"
(org-capture nil "t")
(buffer-string))))))
+(ert-deftest test-org-capture/org-capture-expand-headline ()
+ "Test `org-capture-expand-headline'."
+ ;; `org-capture-expand-headline' should return nil when headline is
+ ;; nil
+ (should
+ (equal
+ nil
+ (let ((file (make-temp-file "org-test")))
+ (unwind-protect
+ (org-capture-expand-headline nil)
+ (delete-file file)))))
+ ;; `org-capture-expand-headline' should return headline if it is a
+ ;; string
+ (should
+ (equal
+ "A"
+ (let ((file (make-temp-file "org-test")))
+ (unwind-protect
+ (org-capture-expand-headline "A")
+ (delete-file file)))))
+ ;; `org-capture-expand-headline' should evaluate headline if it is a
+ ;; function and return its value
+ (should
+ (equal
+ "A"
+ (let ((file (make-temp-file "org-test")))
+ (unwind-protect
+ (org-capture-expand-headline (lambda () "A"))
+ (delete-file file)))))
+ ;; `org-capture-expand-headline' should return the value of headline
+ ;; if it is a symbol
+ (should
+ (equal
+ "A"
+ (let ((file (make-temp-file "org-test")))
+ (unwind-protect
+ (progn
+ (setq temp "A")
+ (org-capture-expand-headline 'temp))
+ (makunbound 'temp)
+ (delete-file file))))))
+
(ert-deftest test-org-capture/org-capture-expand-olp ()
"Test `org-capture-expand-olp'."
;; `org-capture-expand-olp' accepts inlined outline path.
--
2.49.0
