* "Bad template" when creating org-capture for table-line without '|'
@ 2024-03-27 10:47 Rens Oliemans
2024-03-27 12:37 ` Ihor Radchenko
0 siblings, 1 reply; 6+ messages in thread
From: Rens Oliemans @ 2024-03-27 10:47 UTC (permalink / raw)
To: emacs-orgmode
Hi,
I was trying to create an org-capture for a table-line type, and I noticed that
org-capture inserts the string "| Bad template |" when the template does not start with
'|'. This could make sense, since a line is (only) considered part of a table when it
starts with '|'.
However, I did not know that the _template_ also has to start with '|', and org-capture
still inserts the string "| Bad template |", which does not suggest in any way that the
lacking '|' would be the problem (after all, it is smart enough to prepend a '|').
Use this template to reproduce the behaviour, present on 9.7-pre
(release_9.6.23-1318-g990b89):
(setq org-capture-templates '(("t" "Test" table-line (file "test.org")
"%^t")))
I did not see this documented anywhere: this was quite confusing, and the only way I got a
hint at what I did wrong was to see the default template for table-line: "| %? |". It
seems that this behaviour is intended, looking at org-capture.el:1358 @ 990b89d3:
...
(pcase (org-trim (org-capture-get :template))
((pred (string-match-p org-table-border-regexp))
"| %?Bad template |")
(text (concat text "\n"))))
...
however, is this also what's ideal? I would suggest one of the following alternatives:
- An error is signalled to the user with the root cause of the error: no '|' at start of
template. In addition, the org-capture-templates variable documentation string and the
manual would be updated. I am happy to create a patch for this.
- org-capture could pre- and suffix a '|' if the user did not supply it in their template.
I am also happy to create a patch for this, however that would be my first so such a
patch would perhaps have a few iterations ;)
What are your thoughts on this?
Best
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: "Bad template" when creating org-capture for table-line without '|'
2024-03-27 10:47 "Bad template" when creating org-capture for table-line without '|' Rens Oliemans
@ 2024-03-27 12:37 ` Ihor Radchenko
2024-04-14 11:24 ` [PATCH 1/2] org-capture: Allow entry template to start without heading Rens Oliemans
2024-04-14 11:24 ` [PATCH 2/2] org-capture: Allow table-line entry to start without | Rens Oliemans
0 siblings, 2 replies; 6+ messages in thread
From: Ihor Radchenko @ 2024-03-27 12:37 UTC (permalink / raw)
To: Rens Oliemans; +Cc: emacs-orgmode
Rens Oliemans <hallo@rensoliemans.nl> writes:
> ...
> However, I did not know that the _template_ also has to start with '|', and org-capture
> still inserts the string "| Bad template |", which does not suggest in any way that the
> lacking '|' would be the problem (after all, it is smart enough to prepend a '|').
> ...
> however, is this also what's ideal? I would suggest one of the following alternatives:
>
> - An error is signalled to the user with the root cause of the error: no '|' at start of
> template. In addition, the org-capture-templates variable documentation string and the
> manual would be updated. I am happy to create a patch for this.
>
> - org-capture could pre- and suffix a '|' if the user did not supply it in their template.
>
> What are your thoughts on this?
Org mode is inconsistent here.
`org-capture-place-entry' uses `org-capture-verify-tree' and throws an
error when the template does not start from a headline or contains
headings with lower level after higher level.
Yet, `org-capture-place-item' prepends "-" when the template does not
start from a list item.
And, as you noticed, `org-capture-place-table-line' replaces the
template with "Bad template".
I think that the best course of action would be automatically fixing the
templates, as you propose in option (2), changing
`org-capture-place-entry' to automatically prepend "* " if necessary
(still leaving the subtree check though).
> I am also happy to create a patch for this, however that would be my first so such a
> patch would perhaps have a few iterations ;)
That would be welcome. We can take as many iterations as necessary.
You may refer to
https://orgmode.org/worg/org-contribute.html#first-patch for instructions.
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] org-capture: Allow entry template to start without heading
2024-03-27 12:37 ` Ihor Radchenko
@ 2024-04-14 11:24 ` Rens Oliemans
2024-04-14 13:41 ` Ihor Radchenko
2024-04-14 11:24 ` [PATCH 2/2] org-capture: Allow table-line entry to start without | Rens Oliemans
1 sibling, 1 reply; 6+ messages in thread
From: Rens Oliemans @ 2024-04-14 11:24 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
* lisp/org-capture.el (org-capture-place-entry): Prepend heading to
template if the template does not yet start with a heading.
* testing/lisp/test-org-capture.el (test-org-capture/entry): Add two
tests: no error is raised when org-capture is called with a template
that does not start with a heading; and org-capture should error with
a template with a lower heading after a higher heading.
Link: https://list.orgmode.org/877chnc0lr.fsf@localhost/
---
First iteration of these patches, please let me know if anything can be improved, either
about the code or patches themselves (I am not used to sending patches via email).
lisp/org-capture.el | 1 +
testing/lisp/test-org-capture.el | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index da14f45c0..750778f8b 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1198,6 +1198,7 @@ may have been stored before."
(exact-position (org-capture-get :exact-position))
(insert-here? (org-capture-get :insert-here))
(level 1))
+ (unless (string-match "^*" template) (setq template (concat "* " template)))
(org-capture-verify-tree template)
(when exact-position (goto-char exact-position))
(cond
diff --git a/testing/lisp/test-org-capture.el b/testing/lisp/test-org-capture.el
index 0ed44c6af..9ab078193 100644
--- a/testing/lisp/test-org-capture.el
+++ b/testing/lisp/test-org-capture.el
@@ -244,6 +244,23 @@
:immediate-finish t))))
(org-capture nil "t")
(buffer-string))))
+ ;; Do not raise an error on templates that do not start with a heading.
+ (should
+ (org-test-with-temp-text-in-file ""
+ (let* ((file (buffer-file-name))
+ (org-capture-templates
+ `(("t" "Test" entry (file ,file) "Foo"
+ :immediate-finish t))))
+ (org-capture nil "t"))))
+ ;; Raise an error on templates with a lower level heading after a
+ ;; higher level one.
+ (should-error
+ (org-test-with-temp-text-in-file ""
+ (let* ((file (buffer-file-name))
+ (org-capture-templates
+ `(("t" "Test" entry (file ,file) "** X\n* Y"
+ :immediate-finish t))))
+ (org-capture nil "t"))))
;; With a 0 prefix argument, ignore surrounding lists.
(should
(equal "Foo\n* X\nBar\n"
--
2.44.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] org-capture: Allow table-line entry to start without |
2024-03-27 12:37 ` Ihor Radchenko
2024-04-14 11:24 ` [PATCH 1/2] org-capture: Allow entry template to start without heading Rens Oliemans
@ 2024-04-14 11:24 ` Rens Oliemans
1 sibling, 0 replies; 6+ messages in thread
From: Rens Oliemans @ 2024-04-14 11:24 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
* lisp/org-capture.el (org-capture-place-table-line): Prepend
table-line begin ('|') if the template does not start with it.
* testing/lisp/test-org-capture.el (test-org-capture/table-line):
Verify that a template gets prepended with a '|' if it does not start
with it.
---
lisp/org-capture.el | 15 ++++++++-------
testing/lisp/test-org-capture.el | 10 ++++++++++
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 750778f8b..a56e75fda 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1355,13 +1355,14 @@ may have been stored before."
(defun org-capture-place-table-line ()
"Place the template as a table line."
(require 'org-table)
- (let ((text
- (pcase (org-trim (org-capture-get :template))
- ((pred (string-match-p org-table-border-regexp))
- "| %?Bad template |")
- (text (concat text "\n"))))
- (table-line-pos (org-capture-get :table-line-pos))
- beg end)
+ (let* ((template (org-trim (org-capture-get :template)))
+ (text
+ (pcase template
+ ((pred (string-match-p org-table-border-regexp))
+ (concat "| " template))
+ (text (concat text "\n"))))
+ (table-line-pos (org-capture-get :table-line-pos))
+ beg end)
(cond
((org-capture-get :exact-position)
(org-with-point-at (org-capture-get :exact-position)
diff --git a/testing/lisp/test-org-capture.el b/testing/lisp/test-org-capture.el
index 9ab078193..6c13e8283 100644
--- a/testing/lisp/test-org-capture.el
+++ b/testing/lisp/test-org-capture.el
@@ -606,6 +606,16 @@
"| 2 |" :immediate-finish t))))
(org-capture nil "t"))
(buffer-string))))
+ ;; Prepend | when the template does not start with it
+ (should
+ (equal "| 1 |\n| 2 |\n"
+ (org-test-with-temp-text-in-file "| 1 |\n"
+ (let* ((file (buffer-file-name))
+ (org-capture-templates
+ `(("t" "Table" table-line (file ,file)
+ "2 |" :immediate-finish t))))
+ (org-capture nil "t")
+ (buffer-string)))))
;; When `:prepend' is nil, add the row at the end of the table.
(should
(equal "| a |\n| x |\n"
--
2.44.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] org-capture: Allow entry template to start without heading
2024-04-14 11:24 ` [PATCH 1/2] org-capture: Allow entry template to start without heading Rens Oliemans
@ 2024-04-14 13:41 ` Ihor Radchenko
2024-04-14 18:39 ` Rens Oliemans
0 siblings, 1 reply; 6+ messages in thread
From: Ihor Radchenko @ 2024-04-14 13:41 UTC (permalink / raw)
To: Rens Oliemans; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]
Rens Oliemans <hallo@rensoliemans.nl> writes:
> * lisp/org-capture.el (org-capture-place-entry): Prepend heading to
> template if the template does not yet start with a heading.
>
> * testing/lisp/test-org-capture.el (test-org-capture/entry): Add two
> tests: no error is raised when org-capture is called with a template
> that does not start with a heading; and org-capture should error with
> a template with a lower heading after a higher heading.
>
> Link: https://list.orgmode.org/877chnc0lr.fsf@localhost/
> ---
> First iteration of these patches, please let me know if anything can be improved, either
> about the code or patches themselves (I am not used to sending patches via email).
Thanks!
I have improved your patches a little, fixing the regular expression
used to match headings ("^*" is not accurate, you need
org-outline-regexp-bol), and adding another test case.
See the attached.
Before I install the patches, may I know if you have FSF copyright
assignment? See https://orgmode.org/worg/org-contribute.html#copyright
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-org-capture-Allow-entry-template-to-start-without.patch --]
[-- Type: text/x-patch, Size: 2830 bytes --]
From 36639ac711f099b49900d886ad28d29abc1b29ed Mon Sep 17 00:00:00 2001
Message-ID: <36639ac711f099b49900d886ad28d29abc1b29ed.1713102029.git.yantar92@posteo.net>
From: Rens Oliemans <hallo@rensoliemans.nl>
Date: Sun, 14 Apr 2024 13:24:49 +0200
Subject: [PATCH v2 1/2] org-capture: Allow entry template to start without
heading
* lisp/org-capture.el (org-capture-place-entry): Prepend heading to
template if the template does not yet start with a heading.
* testing/lisp/test-org-capture.el (test-org-capture/entry): Add two
tests: no error is raised when org-capture is called with a template
that does not start with a heading; and org-capture should error with
a template with a lower heading after a higher heading.
Link: https://list.orgmode.org/877chnc0lr.fsf@localhost/
---
lisp/org-capture.el | 2 ++
testing/lisp/test-org-capture.el | 24 ++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index da14f45c0..a95a38162 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1198,6 +1198,8 @@ (defun org-capture-place-entry ()
(exact-position (org-capture-get :exact-position))
(insert-here? (org-capture-get :insert-here))
(level 1))
+ (unless (string-match org-outline-regexp-bol template)
+ (setq template (concat "* " template)))
(org-capture-verify-tree template)
(when exact-position (goto-char exact-position))
(cond
diff --git a/testing/lisp/test-org-capture.el b/testing/lisp/test-org-capture.el
index 0ed44c6af..4e9139c40 100644
--- a/testing/lisp/test-org-capture.el
+++ b/testing/lisp/test-org-capture.el
@@ -244,6 +244,30 @@ (ert-deftest test-org-capture/entry ()
:immediate-finish t))))
(org-capture nil "t")
(buffer-string))))
+ ;; Do not raise an error on templates that do not start with a heading.
+ (should
+ (org-test-with-temp-text-in-file ""
+ (let* ((file (buffer-file-name))
+ (org-capture-templates
+ `(("t" "Test" entry (file ,file) "Foo"
+ :immediate-finish t))))
+ (org-capture nil "t"))))
+ (should
+ (org-test-with-temp-text-in-file ""
+ (let* ((file (buffer-file-name))
+ (org-capture-templates
+ `(("t" "Test" entry (file ,file) "*bold*"
+ :immediate-finish t))))
+ (org-capture nil "t"))))
+ ;; Raise an error on templates with a lower level heading after a
+ ;; higher level one.
+ (should-error
+ (org-test-with-temp-text-in-file ""
+ (let* ((file (buffer-file-name))
+ (org-capture-templates
+ `(("t" "Test" entry (file ,file) "** X\n* Y"
+ :immediate-finish t))))
+ (org-capture nil "t"))))
;; With a 0 prefix argument, ignore surrounding lists.
(should
(equal "Foo\n* X\nBar\n"
--
2.44.0
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: v2-0002-org-capture-Allow-table-line-entry-to-start-witho.patch --]
[-- Type: text/x-patch, Size: 2775 bytes --]
From af0b56f3338b8700bd6096e4963c95436b1a14b0 Mon Sep 17 00:00:00 2001
Message-ID: <af0b56f3338b8700bd6096e4963c95436b1a14b0.1713102029.git.yantar92@posteo.net>
In-Reply-To: <36639ac711f099b49900d886ad28d29abc1b29ed.1713102029.git.yantar92@posteo.net>
References: <36639ac711f099b49900d886ad28d29abc1b29ed.1713102029.git.yantar92@posteo.net>
From: Rens Oliemans <hallo@rensoliemans.nl>
Date: Sun, 14 Apr 2024 13:24:51 +0200
Subject: [PATCH v2 2/2] org-capture: Allow table-line entry to start without |
* lisp/org-capture.el (org-capture-place-table-line): Prepend
table-line begin ('|') if the template does not start with it.
* testing/lisp/test-org-capture.el (test-org-capture/table-line):
Verify that a template gets prepended with a '|' if it does not start
with it.
---
lisp/org-capture.el | 15 ++++++++-------
testing/lisp/test-org-capture.el | 10 ++++++++++
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index a95a38162..205d09da8 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1356,13 +1356,14 @@ (defun org-capture-place-item ()
(defun org-capture-place-table-line ()
"Place the template as a table line."
(require 'org-table)
- (let ((text
- (pcase (org-trim (org-capture-get :template))
- ((pred (string-match-p org-table-border-regexp))
- "| %?Bad template |")
- (text (concat text "\n"))))
- (table-line-pos (org-capture-get :table-line-pos))
- beg end)
+ (let* ((template (org-trim (org-capture-get :template)))
+ (text
+ (pcase template
+ ((pred (string-match-p org-table-border-regexp))
+ (concat "| " template))
+ (text (concat text "\n"))))
+ (table-line-pos (org-capture-get :table-line-pos))
+ beg end)
(cond
((org-capture-get :exact-position)
(org-with-point-at (org-capture-get :exact-position)
diff --git a/testing/lisp/test-org-capture.el b/testing/lisp/test-org-capture.el
index 4e9139c40..f97d08bce 100644
--- a/testing/lisp/test-org-capture.el
+++ b/testing/lisp/test-org-capture.el
@@ -613,6 +613,16 @@ (ert-deftest test-org-capture/table-line ()
"| 2 |" :immediate-finish t))))
(org-capture nil "t"))
(buffer-string))))
+ ;; Prepend | when the template does not start with it
+ (should
+ (equal "| 1 |\n| 2 |\n"
+ (org-test-with-temp-text-in-file "| 1 |\n"
+ (let* ((file (buffer-file-name))
+ (org-capture-templates
+ `(("t" "Table" table-line (file ,file)
+ "2 |" :immediate-finish t))))
+ (org-capture nil "t")
+ (buffer-string)))))
;; When `:prepend' is nil, add the row at the end of the table.
(should
(equal "| a |\n| x |\n"
--
2.44.0
[-- Attachment #4: Type: text/plain, Size: 224 bytes --]
--
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] org-capture: Allow entry template to start without heading
2024-04-14 13:41 ` Ihor Radchenko
@ 2024-04-14 18:39 ` Rens Oliemans
0 siblings, 0 replies; 6+ messages in thread
From: Rens Oliemans @ 2024-04-14 18:39 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
Ihor Radchenko <yantar92@posteo.net> writes:
> I have improved your patches a little, fixing the regular expression
> used to match headings ("^*" is not accurate, you need
> org-outline-regexp-bol), and adding another test case.
> See the attached.
Ah I see the difference, good catch. Thank you for the improvements!
> Before I install the patches, may I know if you have FSF copyright
> assignment? See https://orgmode.org/worg/org-contribute.html#copyright
I had not done that yet. I have just sent the form to the FSF. I will let
you know when the process is complete.
Best,
Rens
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-14 18:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-27 10:47 "Bad template" when creating org-capture for table-line without '|' Rens Oliemans
2024-03-27 12:37 ` Ihor Radchenko
2024-04-14 11:24 ` [PATCH 1/2] org-capture: Allow entry template to start without heading Rens Oliemans
2024-04-14 13:41 ` Ihor Radchenko
2024-04-14 18:39 ` Rens Oliemans
2024-04-14 11:24 ` [PATCH 2/2] org-capture: Allow table-line entry to start without | Rens Oliemans
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).