emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed

* "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

  • • * 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

  • * [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

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).