Allowing multiple date trees in a single file

Allowing multiple date trees in a single file

[Carsten Dominik]

[-- Attachment #1.1: Type: text/plain, Size: 425 bytes --]

Hi everyone,

here is a patch that allows to have multiple date trees or iso-week trees
in a single file by allowing to specify the property name that will be used
to place the tree.  The property name can be set in org-capture-templates,
individually for every template if so desired.  I use that to have three
different kind of journals in a single file.

Any objections gains me to merge this change into Master?

Carsten

[-- Attachment #1.2: Type: text/html, Size: 555 bytes --]

[-- Attachment #2: patch-datetree --]
[-- Type: application/octet-stream, Size: 4333 bytes --]

Changes in master
	Modified doc/org.texi
diff --git a/doc/org.texi b/doc/org.texi
index f68ef6e..d59976d 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -7253,6 +7253,11 @@ separator line.
 @item :kill-buffer
 If the target file was not yet visited when capture was invoked, kill the
 buffer again after capture is completed.
+
+@item :tree-property
+If the target is in a date or week tree, this property will be used instead
+of DATE_TREE or WEEK_TREE to identify the org entry under which the tree
+should be built.
 @end table
 @end table
 
	Modified lisp/org-capture.el
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index a8b9879..1202664 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -227,6 +227,10 @@ properties are:
                      capture was invoked, kill the buffer again after capture
                      is finalized.
 
+ :tree-property      The property that identifies the entry where the date or
+                     iso-week tree should be placed.  When given, overrules
+                     the default DATE_TREE or WEEK_TREE properties.
+
 The template defines the text to be inserted.  Often this is an
 Org mode entry (so the first line should start with a star) that
 will be filed as a child of the target headline.  It can also be
@@ -379,7 +383,8 @@ you can escape ambiguous cases with a backward slash, e.g., \\%i."
 				   ((const :format "%v " :clock-resume) (const t))
 				   ((const :format "%v " :unnarrowed) (const t))
 				   ((const :format "%v " :table-line-pos) (const t))
-				   ((const :format "%v " :kill-buffer) (const t)))))))))
+				   ((const :format "%v " :kill-buffer) (const t))
+				   ((const :format "%v " :tree-property) (string "")))))))))
 
 (defcustom org-capture-before-finalize-hook nil
   "Hook that is run right before a capture process is finalized.
@@ -974,7 +979,8 @@ Store them in the capture property list."
 	    (t
 	     ;; Current date, possibly corrected for late night
 	     ;; workers.
-	     (org-today))))))
+	     (org-today))))
+	  nil (org-capture-get :tree-property)))
 	(`(file+function ,path ,function)
 	 (set-buffer (org-capture-target-buffer path))
 	 (org-capture-put-target-region-and-position)
	Modified lisp/org-datetree.el
diff --git a/lisp/org-datetree.el b/lisp/org-datetree.el
index 540753d..d9aa665 100644
--- a/lisp/org-datetree.el
+++ b/lisp/org-datetree.el
@@ -50,15 +50,18 @@ Added time stamp is active unless value is `inactive'."
 	  (const :tag "Add an active time stamp" active)))
 
 ;;;###autoload
-(defun org-datetree-find-date-create (d &optional keep-restriction)
+(defun org-datetree-find-date-create (d &optional keep-restriction
+					tree-property)
   "Find or create an entry for date D.
 If KEEP-RESTRICTION is non-nil, do not widen the buffer.
 When it is nil, the buffer will be widened to make sure an existing date
-tree can be found."
+tree can be found.
+If TREE-PROPERTY is given, place the tree under an entry that has this
+property, instead of the default DATE_TREE property."
   (setq-local org-datetree-base-level 1)
   (or keep-restriction (widen))
   (save-restriction
-    (let ((prop (org-find-property "DATE_TREE")))
+    (let ((prop (org-find-property (or tree-property "DATE_TREE"))))
       (when prop
 	(goto-char prop)
 	(setq-local org-datetree-base-level
@@ -80,17 +83,20 @@ tree can be found."
        year month day))))
 
 ;;;###autoload
-(defun org-datetree-find-iso-week-create (d &optional keep-restriction)
+(defun org-datetree-find-iso-week-create (d &optional keep-restriction
+					    tree-property)
   "Find or create an ISO week entry for date D.
 Compared to `org-datetree-find-date-create' this function creates
 entries ordered by week instead of months.
 If KEEP-RESTRICTION is non-nil, do not widen the buffer.  When it
 is nil, the buffer will be widened to make sure an existing date
-tree can be found."
+tree can be found.
+If TREE-PROPERTY is given, place the tree under an entry that has this
+property, instead of the default DATE_TREE property."
   (setq-local org-datetree-base-level 1)
   (or keep-restriction (widen))
   (save-restriction
-    (let ((prop (org-find-property "WEEK_TREE")))
+    (let ((prop (org-find-property (or tree-property "WEEK_TREE"))))
       (when prop
 	(goto-char prop)
 	(setq-local org-datetree-base-level

Re: Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik <dominik@uva.nl> writes:

> here is a patch that allows to have multiple date trees or iso-week trees
> in a single file by allowing to specify the property name that will be used
> to place the tree.  The property name can be set in org-capture-templates,
> individually for every template if so desired.  I use that to have three
> different kind of journals in a single file.
>
> Any objections gains me to merge this change into Master?

AFAIR, WEEK_TREE and DATE_TREE allow to select the type of tree being
inserted. How is the new property allowing to do the same?

Also, there are some tests defined in "test-org-capture.el" and
"test-org-datetree.el". Would you mind extending those so as to include
your new property ?

Regards,

-- 
Nicolas Goaziou

Re: Allowing multiple date trees in a single file

[Carsten Dominik]

[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]

Hi Nicolas,

thanks for your reaction.

By default, a day or week tree will assume that it is the only thing in a
file when it is created and handled.
The exception is that if you have an Org entry with a property

DATE_TREE: t

that the date tree will be placed under that specific node.  But since this
is a specific property (key, not value), there was no way to target a
different Org node as parent for a tree.  This is now possible.

Another way to implement this would be to use different VALUES for the
DATE_TREE/WEEK_TREE property - maybe that would actually be a somewhat
cleaner implementation.

I'll rethink that and I will also define a test.

Carsten

On Mon, Jan 16, 2017 at 8:45 AM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Carsten Dominik <dominik@uva.nl> writes:
>
> > here is a patch that allows to have multiple date trees or iso-week trees
> > in a single file by allowing to specify the property name that will be
> used
> > to place the tree.  The property name can be set in
> org-capture-templates,
> > individually for every template if so desired.  I use that to have three
> > different kind of journals in a single file.
> >
> > Any objections gains me to merge this change into Master?
>
> AFAIR, WEEK_TREE and DATE_TREE allow to select the type of tree being
> inserted. How is the new property allowing to do the same?
>
> Also, there are some tests defined in "test-org-capture.el" and
> "test-org-datetree.el". Would you mind extending those so as to include
> your new property ?
>
> Regards,
>
> --
> Nicolas Goaziou
>

[-- Attachment #2: Type: text/html, Size: 2292 bytes --]

Re: Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik <dominik@uva.nl> writes:

> Another way to implement this would be to use different VALUES for the
> DATE_TREE/WEEK_TREE property - maybe that would actually be a somewhat
> cleaner implementation.

I think ":DATE_TREE: week" ":DATE_TREE: t" is cleaner, indeed.

> I'll rethink that and I will also define a test.

I also agree there's an opportunity to refactor this and come out with
a more generic interface.

Yet another option is to define new capture targets, e.g.

 - file+datetree+olp
 - file+datetree+olp+date
 - file+datetree+regexp
 - file+datetree+regexp+date
 - ...
 - file+weektree+olp
 - ...

Those would ignore WEEK_TREE and DATE_TREE properties altogether.

Regards,


-- 
Nicolas Goaziou

Re: Allowing multiple date trees in a single file

[Carsten Dominik]

[-- Attachment #1: Type: text/plain, Size: 1357 bytes --]

On Tue, Jan 17, 2017 at 6:50 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Carsten Dominik <dominik@uva.nl> writes:
>
> > Another way to implement this would be to use different VALUES for the
> > DATE_TREE/WEEK_TREE property - maybe that would actually be a somewhat
> > cleaner implementation.
>
> I think ":DATE_TREE: week" ":DATE_TREE: t" is cleaner, indeed.
>

This is not quite what I meant.

I meant

:DATE_TREE: my_diary
:DATE_TREE: food_and_health
:DATE_TREE: movies watched


>
> > I'll rethink that and I will also define a test.
>
> I also agree there's an opportunity to refactor this and come out with
> a more generic interface.
>
> Yet another option is to define new capture targets, e.g.
>
>  - file+datetree+olp
>  - file+datetree+olp+date
>  - file+datetree+regexp
>  - file+datetree+regexp+date
>  - ...
>  - file+weektree+olp
>  - ...
>
> Those would ignore WEEK_TREE and DATE_TREE properties altogether.
>

Yes, one could have additional ways - but I am not going to take out the
existing ones, which would needlessly break things with various users.

Another thing I was thinking is a way to force prompting for a date, for
example through a prefix argument, so that a single capture template could
be used for using the current date and optionally a set one.

Carsten


>
> Regards,
>
>
> --
> Nicolas Goaziou
>

[-- Attachment #2: Type: text/html, Size: 2389 bytes --]

Re: Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik <dominik@uva.nl> writes:

> I meant
>
> :DATE_TREE: my_diary
> :DATE_TREE: food_and_health
> :DATE_TREE: movies watched

It sounds less useful because we already have ways to identify uniquely
a heading.

> Another thing I was thinking is a way to force prompting for a date, for
> example through a prefix argument, so that a single capture template could
> be used for using the current date and optionally a set one.

It is nice, too. It would reduce the number of new capture target types
required.

Regards,

-- 
Nicolas Goaziou

Re: Allowing multiple date trees in a single file

[Samuel Wales]

as an aside, just wanted to point out an alternative that i use
exclusively instead of date trees.  of course it is not for everybody
and i do not want to hijack the thread, so followups should have a
different header.

  ***** CONVERSATION [2017-01-15 Sun 14:09] talked with carsten

i sort these in the outline.  they are always visible at the bottom of
every subtree, in time order.  i can follow a thread by just reading
the headers.  i can quickly do a binary search to find a date.  i can
select a region without selecting parents.  they show up nicely in the
agenda.  i sort them in the agenda also.  links to them work fine.
the date information i need is there without changing visibility, yet
unobtrusive.

it might sound silly to bring it up, but everybody i have told to this
has had an aha moment and been enthusiastic.

just as an aside.

-- 
The Kafka Pandemic: http://thekafkapandemic.blogspot.com

The disease DOES progress.  MANY people have died from it.  And
ANYBODY can get it.

Denmark: free Karina Hansen NOW.
  UPDATE 2016-10: home, but not fully free

Re: Allowing multiple date trees in a single file

[Carsten Dominik]

[-- Attachment #1: Type: text/plain, Size: 1443 bytes --]

Hi Samuel,

Yes, this is of course also a very good solution that avoids the
overhead of a date tree.  One of the reasons why the dates
in Org are in ISO format was to make them sortable by
a simple text sort. Thanks for sharing.

Carsten

On Wed, Jan 18, 2017 at 9:19 PM, Samuel Wales <samologist@gmail.com> wrote:

> as an aside, just wanted to point out an alternative that i use
> exclusively instead of date trees.  of course it is not for everybody
> and i do not want to hijack the thread, so followups should have a
> different header.
>
>   ***** CONVERSATION [2017-01-15 Sun 14:09] talked with carsten
>
> i sort these in the outline.  they are always visible at the bottom of
> every subtree, in time order.  i can follow a thread by just reading
> the headers.  i can quickly do a binary search to find a date.  i can
> select a region without selecting parents.  they show up nicely in the
> agenda.  i sort them in the agenda also.  links to them work fine.
> the date information i need is there without changing visibility, yet
> unobtrusive.
>
> it might sound silly to bring it up, but everybody i have told to this
> has had an aha moment and been enthusiastic.
>
> just as an aside.
>
> --
> The Kafka Pandemic: http://thekafkapandemic.blogspot.com
>
> The disease DOES progress.  MANY people have died from it.  And
> ANYBODY can get it.
>
> Denmark: free Karina Hansen NOW.
>   UPDATE 2016-10: home, but not fully free
>

[-- Attachment #2: Type: text/html, Size: 2064 bytes --]

Re: Allowing multiple date trees in a single file

[Carsten Dominik]

[-- Attachment #1.1: Type: text/plain, Size: 1668 bytes --]

Hi Nicolas,

ok, here is where I have gotten with this:

Attached is a patch that does the following:

It consolidates all four different org-capture target types that have to do
with
date/week trees into a single one, called `file+olp+datetree'.  This target
allows for an optional outline path specification to tell capture to build
the
datetree under a specific headline.  To switch to a week tree, or to force
a date prompt is now the matter of setting one of the properties in the
org-capture-template variable.

Everything works transparently, so users can update the way they
write their datetree captures, but they don't have to - the old syntax
remains
supported and will automatically switched when one uses customize to change
the variable.

After a bit more testing, I'd like to apply this patch.  Please let me know
if you agree. And additional testers would be useful.  Anyone?  Make sure
to backup your capture templates if something goes wrong.

Cheers

Carsten





On Wed, Jan 18, 2017 at 12:23 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Carsten Dominik <dominik@uva.nl> writes:
>
> > I meant
> >
> > :DATE_TREE: my_diary
> > :DATE_TREE: food_and_health
> > :DATE_TREE: movies watched
>
> It sounds less useful because we already have ways to identify uniquely
> a heading.
>
> > Another thing I was thinking is a way to force prompting for a date, for
> > example through a prefix argument, so that a single capture template
> could
> > be used for using the current date and optionally a set one.
>
> It is nice, too. It would reduce the number of new capture target types
> required.
>
> Regards,
>
> --
> Nicolas Goaziou
>

[-- Attachment #1.2: Type: text/html, Size: 2540 bytes --]

[-- Attachment #2: datetree-patch --]
[-- Type: application/octet-stream, Size: 17208 bytes --]

Changes in master
	Modified doc/org.texi
diff --git a/doc/org.texi b/doc/org.texi
index f68ef6e..3219d94 100644
--- a/doc/org.texi
+++ b/doc/org.texi
@@ -7065,7 +7065,7 @@ would look like:
 (setq org-capture-templates
  '(("t" "Todo" entry (file+headline "~/org/gtd.org" "Tasks")
         "* TODO %?\n  %i\n  %a")
-   ("j" "Journal" entry (file+datetree "~/org/journal.org")
+   ("j" "Journal" entry (file+olp+datetree "~/org/journal.org")
         "* %?\nEntered on %U\n  %i\n  %a")))
 @end group
 @end smalllisp
@@ -7173,21 +7173,13 @@ For non-unique headings, the full path is safer.
 @item (file+regexp  "path/to/file" "regexp to find location")
 Use a regular expression to position the cursor.
 
-@item (file+datetree "path/to/file")
+@item (file+olp+datetree "path/to/file" [ "Level 1 heading" ....])
 Will create a heading in a date tree for today's date@footnote{Datetree
 headlines for years accept tags, so if you use both @code{* 2013 :noexport:}
 and @code{* 2013} in your file, the capture will refile the note to the first
-one matched.}.
-
-@item (file+datetree+prompt "path/to/file")
-Will create a heading in a date tree, but will prompt for the date.
-
-@item (file+weektree "path/to/file")
-Will create a heading in a week tree for today's date.  Week trees are sorted
-by week and not by month unlike datetrees.
-
-@item (file+weektree+prompt "path/to/file")
-Will create a heading in a week tree, but will prompt for the date.
+one matched.}.  If the optional outline path is given, the tree will be built
+under the node it is pointing to. Check out the @code{:time-prompt} and
+@code{:tree-type} below for modifying the tree.
 
 @item (file+function "path/to/file" function-finding-location)
 A function to find the right location in the file.
@@ -7239,6 +7231,14 @@ with the capture.  Note that @code{:clock-keep} has precedence over
 @code{:clock-resume}.  When setting both to @code{t}, the current clock will
 run and the previous one will not be resumed.
 
+@item :time-prompt
+Prompt for a date/time to be used for date/week trees and when filling the
+template.
+
+@item :tree-type
+When `week', make a week tree instead of the month tree, i.e. place the
+headings for each day under a heading with the current iso week.
+
 @item :unnarrowed
 Do not narrow the target buffer, simply show the full buffer.  Default is to
 narrow it so that you only see the new material.
	Modified lisp/org-capture.el
diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index 1a1a500..3afbe24 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -84,6 +84,25 @@
   :tag "Org Capture"
   :group 'org)
 
+(defun org-capture-upgrade-templates (templates)
+  "Update the template list to the new format.
+The new format unifies all the date/week tree targets into one that
+also allows for an optional outline path to specify a target."
+  (let (target props)
+    (mapcar
+     (lambda (ee)
+       (setq target (car (nth 3 ee)))
+       (when (memq target '(file+datetree file+datetree+prompt
+					  file+weektree file+weektree+prompt))
+	 (setq target (symbol-name target) props nil)
+	 (if (string-match "prompt" target) (setq props '(:time-prompt t)))
+	 (if (string-match "week" target)
+	     (setq props (append '(:tree-type week) props)))
+	 (setcar (nth 3 ee) 'file+olp+datetree)
+	 (setcdr (nthcdr 4 ee) (append props (nthcdr 5 ee))))
+       ee)
+     templates)))
+
 (defcustom org-capture-templates nil
   "Templates for the creation of new entries.
 
@@ -141,22 +160,17 @@ target       Specification of where the captured item should be placed.
                  Fast configuration if the target heading is unique in the file
 
              (file+olp \"path/to/file\" \"Level 1 heading\" \"Level 2\" ...)
-                 For non-unique headings, the full path is safer
+                 For non-unique headings, the full outline path is safer
 
              (file+regexp  \"path/to/file\" \"regexp to find location\")
                  File to the entry matching regexp
 
-             (file+datetree \"path/to/file\")
-                 Will create a heading in a date tree for today's date
-
-             (file+datetree+prompt \"path/to/file\")
-                 Will create a heading in a date tree, prompts for date
-
-             (file+weektree \"path/to/file\")
-                 Will create a heading in a week tree for today's date
-
-             (file+weektree+prompt \"path/to/file\")
-                 Will create a heading in a week tree, prompts for date
+             (file+olp+datetree \"path/to/file\" \"Level 1 heading\" ...)
+                 Will create a heading in a date tree for today's date.
+                 If no headings are 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.
 
              (file+function \"path/to/file\" function-finding-location)
                  A function to find the right location in the file
@@ -214,6 +228,11 @@ properties are:
                      When setting both to t, the current clock will run and
                      the previous one will not be resumed.
 
+ :time-prompt        Prompt for a date/time to be used for date/week trees
+                     and when filling the template.
+
+ :tree-type          When `week', make a week tree instead of the month tree.
+
  :unnarrowed         Do not narrow the target buffer, simply show the
                      full buffer.  Default is to narrow it so that you
                      only see the new stuff.
@@ -297,6 +316,7 @@ When you need to insert a literal percent sign in the template,
 you can escape ambiguous cases with a backward slash, e.g., \\%i."
   :group 'org-capture
   :version "24.1"
+  :set (lambda (s v) (set s (org-capture-upgrade-templates v)))
   :type
   (let ((file-variants '(choice :tag "Filename       "
 				(file :tag "Literal")
@@ -337,18 +357,11 @@ you can escape ambiguous cases with a backward slash, e.g., \\%i."
 				(const :format "" file+regexp)
 				,file-variants
 				(regexp :tag "  Regexp"))
-			  (list :tag "File & Date tree"
-				(const :format "" file+datetree)
-				,file-variants)
-			  (list :tag "File & Date tree, prompt for date"
-				(const :format "" file+datetree+prompt)
-				,file-variants)
-			  (list :tag "File & Week tree"
-				(const :format "" file+weektree)
-				,file-variants)
-			  (list :tag "File & Week tree, prompt for date"
-				(const :format "" file+weektree+prompt)
-				,file-variants)
+			  (list :tag "File [ & Outline path ] & Date tree"
+				(const :format "" file+olp+datetree)
+				,file-variants
+				(repeat :tag "Outline path" :inline t
+					(string :tag "Headline")))
 			  (list :tag "File & function"
 				(const :format "" file+function)
 				,file-variants
@@ -377,8 +390,10 @@ you can escape ambiguous cases with a backward slash, e.g., \\%i."
 				   ((const :format "%v " :clock-in) (const t))
 				   ((const :format "%v " :clock-keep) (const t))
 				   ((const :format "%v " :clock-resume) (const t))
+				   ((const :format "%v " :time-prompt) (const t))
+				   ((const :format "%v " :tree-type) (const week))
 				   ((const :format "%v " :unnarrowed) (const t))
-				   ((const :format "%v " :table-line-pos) (const t))
+				   ((const :format "%v " :table-line-pos) (string))
 				   ((const :format "%v " :kill-buffer) (const t)))))))))
 
 (defcustom org-capture-before-finalize-hook nil
@@ -562,6 +577,9 @@ the last note stored.
 
 When called with a `C-0' (zero) prefix, insert a template at point.
 
+When called with a `C-1' (one) prefix, force prompting for a date when
+a datetree entry is made.
+
 ELisp programs can set KEYS to a string associated with a template
 in `org-capture-templates'.  In this case, interactive selection
 will be bypassed.
@@ -902,6 +920,7 @@ Store them in the capture property list."
 	   (insert "* " headline "\n")
 	   (beginning-of-line 0)))
 	(`(file+olp ,path . ,outline-path)
+	 (setq outline-path (org-capture-sanitize-olp outline-path))
 	 (let ((m (org-find-olp (cons (org-capture-expand-file path)
 				      outline-path))))
 	   (set-buffer (marker-buffer m))
@@ -922,59 +941,65 @@ Store them in the capture property list."
 	   (org-capture-put :exact-position (point))
 	   (setq target-entry-p
 		 (and (derived-mode-p 'org-mode) (org-at-heading-p)))))
-	(`(,(and type (or `file+datetree
-			  `file+datetree+prompt
-			  `file+weektree
-			  `file+weektree+prompt))
-	   ,path)
-	 (set-buffer (org-capture-target-buffer path))
-	 (unless (derived-mode-p 'org-mode)
-	   (error "Target buffer \"%s\" for %s should be in Org mode"
-		  (current-buffer)
-		  type))
-	 (require 'org-datetree)
-	 (org-capture-put-target-region-and-position)
-	 (widen)
-	 ;; Make a date/week tree entry, with the current date (or
-	 ;; yesterday, if we are extending dates for a couple of hours)
-	 (funcall
-	  (if (memq type '(file+weektree file+weektree+prompt))
-	      #'org-datetree-find-iso-week-create
-	    #'org-datetree-find-date-create)
-	  (calendar-gregorian-from-absolute
-	   (cond
-	    (org-overriding-default-time
-	     ;; Use the overriding default time.
-	     (time-to-days org-overriding-default-time))
-	    ((memq type '(file+datetree+prompt file+weektree+prompt))
-	     ;; Prompt for date.
-	     (let ((prompt-time (org-read-date
-				 nil t nil "Date for tree entry:"
-				 (current-time))))
-	       (org-capture-put
-		:default-time
-		(cond ((and (or (not (boundp 'org-time-was-given))
-				(not org-time-was-given))
-			    (not (= (time-to-days prompt-time) (org-today))))
-		       ;; Use 00:00 when no time is given for another
-		       ;; date than today?
-		       (apply #'encode-time
-			      (append '(0 0 0)
-				      (cl-cdddr (decode-time prompt-time)))))
-		      ((string-match "\\([^ ]+\\)--?[^ ]+[ ]+\\(.*\\)"
-				     org-read-date-final-answer)
-		       ;; Replace any time range by its start.
-		       (apply #'encode-time
-			      (org-read-date-analyze
-			       (replace-match "\\1 \\2" nil nil
-					      org-read-date-final-answer)
-			       prompt-time (decode-time prompt-time))))
-		      (t prompt-time)))
-	       (time-to-days prompt-time)))
-	    (t
-	     ;; Current date, possibly corrected for late night
-	     ;; workers.
-	     (org-today))))))
+	(`(file+olp+datetree ,path . ,outline-path)
+	 (setq outline-path (org-capture-sanitize-olp outline-path))
+	 (let ((m (if outline-path
+		      (org-find-olp (cons (org-capture-expand-file path)
+					  outline-path))
+		    (set-buffer (org-capture-target-buffer path))
+		    (move-marker (make-marker) (point)))))		    
+	   (set-buffer (marker-buffer m))
+	   (org-capture-put-target-region-and-position)
+	   (widen)
+	   (goto-char m)
+	   (set-marker m nil)
+	   (require 'org-datetree)
+	   (org-capture-put-target-region-and-position)
+	   (widen)
+	   ;; Make a date/week tree entry, with the current date (or
+	   ;; yesterday, if we are extending dates for a couple of hours)
+	   (funcall
+	    (if (eq (org-capture-get :tree-type) 'week)
+		#'org-datetree-find-iso-week-create
+	      #'org-datetree-find-date-create)
+	    (calendar-gregorian-from-absolute
+	     (cond
+	      (org-overriding-default-time
+	       ;; Use the overriding default time.
+	       (time-to-days org-overriding-default-time))
+	      ((org-capture-get :time-prompt)
+	       ;; Prompt for date.
+	       (let ((prompt-time (org-read-date
+				   nil t nil "Date for tree entry:"
+				   (current-time))))
+		 (org-capture-put
+		  :default-time
+		  (cond ((and (or (not (boundp 'org-time-was-given))
+				  (not org-time-was-given))
+			      (not (= (time-to-days prompt-time) (org-today))))
+			 ;; Use 00:00 when no time is given for another
+			 ;; date than today?
+			 (apply #'encode-time
+				(append '(0 0 0)
+					(cl-cdddr (decode-time prompt-time)))))
+			((string-match "\\([^ ]+\\)--?[^ ]+[ ]+\\(.*\\)"
+				       org-read-date-final-answer)
+			 ;; Replace any time range by its start.
+			 (apply #'encode-time
+				(org-read-date-analyze
+				 (replace-match "\\1 \\2" nil nil
+						org-read-date-final-answer)
+				 prompt-time (decode-time prompt-time))))
+			(t prompt-time)))
+		 (time-to-days prompt-time)))
+	      (t
+	       ;; Current date, possibly corrected for late night
+	       ;; workers.
+	       (org-today))))
+	    ;; the following is the keep-restriction argument for
+	    ;; org-datetree-find-date-create
+	    (if outline-path 'subtree-at-point)
+	    )))
 	(`(file+function ,path ,function)
 	 (set-buffer (org-capture-target-buffer path))
 	 (org-capture-put-target-region-and-position)
@@ -1432,6 +1457,13 @@ Use PREFIX as a prefix for the name of the indirect buffer."
   (unless (org-kill-is-subtree-p tree)
     (error "Template is not a valid Org entry or tree")))
 
+(defun org-capture-sanitize-olp (olp)
+  "Keep only non-white strings in the list OPL."
+  (let (res e)
+    (while (setq e (pop olp))
+      (and (stringp e) (string-match "\\S-" e) (push e res)))
+    (nreverse res)))
+
 (defun org-mks (table title &optional prompt specials)
   "Select a member of an alist with multiple keys.
 
@@ -1526,7 +1558,8 @@ is selected, only the bare key is returned."
 Lisp programs can force the template by setting KEYS to a string."
   (let ((org-capture-templates
 	 (or (org-contextualize-keys
-	      org-capture-templates org-capture-templates-contexts)
+	      (org-capture-upgrade-templates org-capture-templates)
+	      org-capture-templates-contexts)
 	     '(("t" "Task" entry (file+headline "" "Tasks")
 		"* TODO %?\n  %u\n  %a")))))
     (if keys
	Modified lisp/org-datetree.el
diff --git a/lisp/org-datetree.el b/lisp/org-datetree.el
index 540753d..1116377 100644
--- a/lisp/org-datetree.el
+++ b/lisp/org-datetree.el
@@ -54,16 +54,25 @@ Added time stamp is active unless value is `inactive'."
   "Find or create an entry for date D.
 If KEEP-RESTRICTION is non-nil, do not widen the buffer.
 When it is nil, the buffer will be widened to make sure an existing date
-tree can be found."
+tree can be found.  If it is the sympol `subtree-at-point', then the tree
+will be built under the headline at point."
   (setq-local org-datetree-base-level 1)
-  (or keep-restriction (widen))
   (save-restriction
-    (let ((prop (org-find-property "DATE_TREE")))
-      (when prop
-	(goto-char prop)
-	(setq-local org-datetree-base-level
-		    (org-get-valid-level (org-current-level) 1))
-	(org-narrow-to-subtree)))
+    (if (eq keep-restriction 'subtree-at-point)
+	(progn
+	  (or (org-at-heading-p) (error "Not at heading"))
+	  (widen)
+	  (org-narrow-to-subtree)
+	  (setq-local org-datetree-base-level
+		      (org-get-valid-level (org-current-level) 1)))
+      (or keep-restriction (widen))
+      ;; support the old way of tree placement, using a property
+      (let ((prop (org-find-property "DATE_TREE")))
+	(when prop
+	  (goto-char prop)
+	  (setq-local org-datetree-base-level
+		      (org-get-valid-level (org-current-level) 1))
+	  (org-narrow-to-subtree))))
     (goto-char (point-min))
     (let ((year (calendar-extract-year d))
 	  (month (calendar-extract-month d))
@@ -84,18 +93,26 @@ tree can be found."
   "Find or create an ISO week entry for date D.
 Compared to `org-datetree-find-date-create' this function creates
 entries ordered by week instead of months.
-If KEEP-RESTRICTION is non-nil, do not widen the buffer.  When it
-is nil, the buffer will be widened to make sure an existing date
-tree can be found."
+When it is nil, the buffer will be widened to make sure an existing date
+tree can be found.  If it is the sympol `subtree-at-point', then the tree
+will be built under the headline at point."
   (setq-local org-datetree-base-level 1)
-  (or keep-restriction (widen))
   (save-restriction
-    (let ((prop (org-find-property "WEEK_TREE")))
-      (when prop
-	(goto-char prop)
-	(setq-local org-datetree-base-level
-		    (org-get-valid-level (org-current-level) 1))
-	(org-narrow-to-subtree)))
+    (if (eq keep-restriction 'subtree-at-point)
+	(progn
+	  (or (org-at-heading-p) (error "Not at heading"))
+	  (widen)
+	  (org-narrow-to-subtree)
+	  (setq-local org-datetree-base-level
+		      (org-get-valid-level (org-current-level) 1)))
+      (or keep-restriction (widen))
+      ;; support the old way of tree placement, using a property
+      (let ((prop (org-find-property "WEEK_TREE")))
+	(when prop
+	  (goto-char prop)
+	  (setq-local org-datetree-base-level
+		      (org-get-valid-level (org-current-level) 1))
+	  (org-narrow-to-subtree))))
     (goto-char (point-min))
     (require 'cal-iso)
     (let* ((year (calendar-extract-year d))
	Modified lisp/org.el
diff --git a/lisp/org.el b/lisp/org.el
index 38fce70..ac56d71 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -16615,6 +16615,8 @@ only headings."
 	 end found flevel)
     (unless buffer (error "File not found :%s" file))
     (with-current-buffer buffer
+      (unless (derived-mode-p 'org-mode)
+	(error "Buffer %s needs to be in org-mode" buffer))
       (org-with-wide-buffer
        (goto-char (point-min))
        (dolist (heading path)

Re: Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik <dominik@uva.nl> writes:

> Attached is a patch that does the following:
>
> It consolidates all four different org-capture target types that have to do
> with
> date/week trees into a single one, called `file+olp+datetree'.  This target
> allows for an optional outline path specification to tell capture to build
> the
> datetree under a specific headline.  To switch to a week tree, or to force
> a date prompt is now the matter of setting one of the properties in the
> org-capture-template variable.

It sounds good. Thank you.

> Everything works transparently, so users can update the way they
> write their datetree captures, but they don't have to - the old syntax
> remains
> supported and will automatically switched when one uses customize to change
> the variable.

I am a bit worried by this compatibility layer. I mean, it is good to
preserve compatibility with old templates, but it ought to be an
ephemeral solution. I.e., no more
`org-table--error-on-old-row-references' lingering around for ages.

We could, for example, generate a deprecation warning when old templates
are used. Then we will be able to remove this unnecessary piece of code
in next major release.

See for example the end of `org-open-file', although it is a bit more
drastic (it raises an error, not a warning).

> After a bit more testing, I'd like to apply this patch.  Please let me know
> if you agree. And additional testers would be useful.  Anyone?  Make sure
> to backup your capture templates if something goes wrong.

Some comments follow.

> -@item (file+weektree+prompt "path/to/file")
> -Will create a heading in a week tree, but will prompt for the date.
> +one matched.}.  If the optional outline path is given, the tree will be built
> +under the node it is pointing to. Check out the @code{:time-prompt} and

There's a missing space above.

> +(defun org-capture-upgrade-templates (templates)
> +  "Update the template list to the new format.
> +The new format unifies all the date/week tree targets into one that
> +also allows for an optional outline path to specify a target."
> +  (let (target props)
> +    (mapcar
> +     (lambda (ee)
> +       (setq target (car (nth 3 ee)))
> +       (when (memq target '(file+datetree file+datetree+prompt
> +					  file+weektree file+weektree+prompt))
> +	 (setq target (symbol-name target) props nil)
> +	 (if (string-match "prompt" target) (setq props '(:time-prompt t)))
> +	 (if (string-match "week" target)
> +	     (setq props (append '(:tree-type week) props)))
> +	 (setcar (nth 3 ee) 'file+olp+datetree)
> +	 (setcdr (nthcdr 4 ee) (append props (nthcdr 5 ee))))
> +       ee)
> +     templates)))

I suggest the following. Less `setq', `setcar', `setcdr' makes Org
a better place.

(defun org-capture-upgrade-templates (templates)
  "Update the template list to the new format.
TEMPLATES is a template list, as in `org-capture-templates'. The
new format unifies all the date/week tree targets into one that
also allows for an optional outline path to specify a target."
  (mapcar
   (lambda (template)
     (pcase template
       ;; Match templates with an obsolete "tree" target type. Replace
       ;; it with common `file+olp-datetree'.  Add new properties
       ;; (i.e., `:time-prompt' and `:tree-type') if needed.
       (`(,key ,desc ,type (file+datetree . ,path) ,template . ,props)
        `(,key ,desc ,type (file+olp+datetree ,@path) ,@props))
       (`(,key ,desc ,type (file+datetree+prompt . ,path) ,template . ,props)
        `(,key ,desc ,type (file+olp+datetree ,@path) :time-prompt t ,@props))
       (`(,key ,desc ,type (file+weektree . ,path) ,template . ,props)
        `(,key ,desc ,type (file+olp+datetree ,@path) :tree-type week ,@props))
       (`(,key ,desc ,type (file+weektree+prompt . ,path) ,template . ,props)
        `(,key ,desc ,type (file+olp+datetree ,@path) :tree-type week :time-prompt t ,@props))
       ;; Other templates are left unchanged.
       (_ template)))
   templates))

> -             (file+weektree+prompt \"path/to/file\")
> -                 Will create a heading in a week tree, prompts for date
> +             (file+olp+datetree \"path/to/file\" \"Level 1 heading\" ...)
> +                 Will create a heading in a date tree for today's date.
> +                 If no headings are given, the tree will be on top level.

Nitpick: It may be just me, but "If no heading is given" sounds better.

>  ELisp programs can set KEYS to a string associated with a template
>  in `org-capture-templates'.  In this case, interactive selection
>  will be bypassed.
> @@ -902,6 +920,7 @@ Store them in the capture property list."
>  	   (insert "* " headline "\n")
>  	   (beginning-of-line 0)))
>  	(`(file+olp ,path . ,outline-path)
> +	 (setq outline-path (org-capture-sanitize-olp outline-path))

See below about `org-capture-sanitize-olp'.

Also, it is better to make the `setq' a let-binding when possible.

>  	 (let ((m (org-find-olp (cons (org-capture-expand-file path)
>  				      outline-path))))
>  	   (set-buffer (marker-buffer m))
> @@ -922,59 +941,65 @@ Store them in the capture property list."
>  	   (org-capture-put :exact-position (point))
>  	   (setq target-entry-p
>  		 (and (derived-mode-p 'org-mode) (org-at-heading-p)))))

[...]

> +	(`(file+olp+datetree ,path . ,outline-path)
> +	 (setq outline-path (org-capture-sanitize-olp outline-path))

Ditto.

> +	 (let ((m (if outline-path
> +		      (org-find-olp (cons (org-capture-expand-file path)
> +					  outline-path))
> +		    (set-buffer (org-capture-target-buffer path))
> +		    (move-marker (make-marker) (point)))))		    
> +	   (set-buffer (marker-buffer m))
> +	   (org-capture-put-target-region-and-position)
> +	   (widen)
> +	   (goto-char m)
> +	   (set-marker m nil)
> +	   (require 'org-datetree)
> +	   (org-capture-put-target-region-and-position)
> +	   (widen)
> +	   ;; Make a date/week tree entry, with the current date (or
> +	   ;; yesterday, if we are extending dates for a couple of hours)
> +	   (funcall
> +	    (if (eq (org-capture-get :tree-type) 'week)
> +		#'org-datetree-find-iso-week-create
> +	      #'org-datetree-find-date-create)
> +	    (calendar-gregorian-from-absolute
> +	     (cond
> +	      (org-overriding-default-time
> +	       ;; Use the overriding default time.
> +	       (time-to-days org-overriding-default-time))
> +	      ((org-capture-get :time-prompt)
> +	       ;; Prompt for date.
> +	       (let ((prompt-time (org-read-date
> +				   nil t nil "Date for tree entry:"
> +				   (current-time))))
> +		 (org-capture-put
> +		  :default-time
> +		  (cond ((and (or (not (boundp 'org-time-was-given))
> +				  (not org-time-was-given))
> +			      (not (= (time-to-days prompt-time) (org-today))))
> +			 ;; Use 00:00 when no time is given for another
> +			 ;; date than today?
> +			 (apply #'encode-time
> +				(append '(0 0 0)
> +					(cl-cdddr (decode-time prompt-time)))))
> +			((string-match "\\([^ ]+\\)--?[^ ]+[ ]+\\(.*\\)"
> +				       org-read-date-final-answer)
> +			 ;; Replace any time range by its start.
> +			 (apply #'encode-time
> +				(org-read-date-analyze
> +				 (replace-match "\\1 \\2" nil nil
> +						org-read-date-final-answer)
> +				 prompt-time (decode-time prompt-time))))
> +			(t prompt-time)))
> +		 (time-to-days prompt-time)))
> +	      (t
> +	       ;; Current date, possibly corrected for late night
> +	       ;; workers.
> +	       (org-today))))
> +	    ;; the following is the keep-restriction argument for
> +	    ;; org-datetree-find-date-create

  ;; The following...
  ;; ... `org-datetree-find-date-create'.

> +	    (if outline-path 'subtree-at-point)

  (and outline-path 'subtree-at-point)

> +	    )))

These trailing parens need to be moved above.

>  	(`(file+function ,path ,function)
>  	 (set-buffer (org-capture-target-buffer path))
>  	 (org-capture-put-target-region-and-position)
> @@ -1432,6 +1457,13 @@ Use PREFIX as a prefix for the name of the indirect buffer."
>    (unless (org-kill-is-subtree-p tree)
>      (error "Template is not a valid Org entry or tree")))
>  
> +(defun org-capture-sanitize-olp (olp)
> +  "Keep only non-white strings in the list OPL."
> +  (let (res e)
> +    (while (setq e (pop olp))
> +      (and (stringp e) (string-match "\\S-" e) (push e res)))
> +    (nreverse res)))

This function is really a one-liner:

  (cl-remove-if-not #'org-string-nw-p olp)

Therefore, I don't think it deserves a dedicated function.

BTW is there any reason to prune non-white strings from the list? Is an
empty string even invalid, since document can contain blank headlines?

The outline path is provided by the user. We can assume he knows what he
is doing.

Another option would be to barf if the outline path contains any invalid
value instead of silently "fixing" it on user's behalf. E.g.,

  (when (cl-some #'invalid-value-check olp)
    (user-error "Invalid outline path in template %S" template))

for some value of #'invalid-value-check.

WDYT?

>  When it is nil, the buffer will be widened to make sure an existing date
> -tree can be found."
> +tree can be found.  If it is the sympol `subtree-at-point', then the tree
> +will be built under the headline at point."
>    (setq-local org-datetree-base-level 1)
> -  (or keep-restriction (widen))
>    (save-restriction
> -    (let ((prop (org-find-property "DATE_TREE")))
> -      (when prop
> -	(goto-char prop)
> -	(setq-local org-datetree-base-level
> -		    (org-get-valid-level (org-current-level) 1))
> -	(org-narrow-to-subtree)))
> +    (if (eq keep-restriction 'subtree-at-point)
> +	(progn
> +	  (or (org-at-heading-p) (error "Not at heading"))

Nitpick:

  (unless (org-at-heading-p) (error "Not at heading"))

> +	  (widen)
> +	  (org-narrow-to-subtree)
> +	  (setq-local org-datetree-base-level
> +		      (org-get-valid-level (org-current-level) 1)))
> +      (or keep-restriction (widen))

  (unless keep-restriction (widen))

> +      ;; support the old way of tree placement, using a property

  ;; Support ... using a property.

> +      (let ((prop (org-find-property "DATE_TREE")))
> +	(when prop
> +	  (goto-char prop)
> +	  (setq-local org-datetree-base-level
> +		      (org-get-valid-level (org-current-level) 1))
> +	  (org-narrow-to-subtree))))
>      (goto-char (point-min))
>      (let ((year (calendar-extract-year d))
>  	  (month (calendar-extract-month d))
> @@ -84,18 +93,26 @@ tree can be found."
>    "Find or create an ISO week entry for date D.
>  Compared to `org-datetree-find-date-create' this function creates
>  entries ordered by week instead of months.
> -If KEEP-RESTRICTION is non-nil, do not widen the buffer.  When it
> -is nil, the buffer will be widened to make sure an existing date
> -tree can be found."
> +When it is nil, the buffer will be widened to make sure an existing date
> +tree can be found.  If it is the sympol `subtree-at-point', then the tree
> +will be built under the headline at point."
>    (setq-local org-datetree-base-level 1)
> -  (or keep-restriction (widen))
>    (save-restriction
> -    (let ((prop (org-find-property "WEEK_TREE")))
> -      (when prop
> -	(goto-char prop)
> -	(setq-local org-datetree-base-level
> -		    (org-get-valid-level (org-current-level) 1))
> -	(org-narrow-to-subtree)))
> +    (if (eq keep-restriction 'subtree-at-point)
> +	(progn
> +	  (or (org-at-heading-p) (error "Not at heading"))
> +	  (widen)
> +	  (org-narrow-to-subtree)
> +	  (setq-local org-datetree-base-level
> +		      (org-get-valid-level (org-current-level) 1)))
> +      (or keep-restriction (widen))
> +      ;; support the old way of tree placement, using a property
> +      (let ((prop (org-find-property "WEEK_TREE")))
> +	(when prop
> +	  (goto-char prop)
> +	  (setq-local org-datetree-base-level
> +		      (org-get-valid-level (org-current-level) 1))
> +	  (org-narrow-to-subtree))))

See above, since the same suggestions apply. Also, it looks like there
is some code duplication involved here. Would it make sense to factor
the common part out of them?

>      (goto-char (point-min))
>      (require 'cal-iso)
>      (let* ((year (calendar-extract-year d))
> 	Modified lisp/org.el
> diff --git a/lisp/org.el b/lisp/org.el
> index 38fce70..ac56d71 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -16615,6 +16615,8 @@ only headings."
>  	 end found flevel)
>      (unless buffer (error "File not found :%s" file))
>      (with-current-buffer buffer
> +      (unless (derived-mode-p 'org-mode)
> +	(error "Buffer %s needs to be in org-mode" buffer))

Nitpick: "Buffer %s needs to be in Org mode"

Ideally, a bunch of tests in test-org-capture.el would be nice.


Regards,

-- 
Nicolas Goaziou

Re: Allowing multiple date trees in a single file

[Carsten Dominik]

[-- Attachment #1: Type: text/plain, Size: 17802 bytes --]

Hi Nicolas,

thank you for taking the time to look at the proposed changes in detail.
My replies and comments are below.

On Sat, Feb 4, 2017 at 1:48 PM, Nicolas Goaziou <mail@nicolasgoaziou.fr>
wrote:

> Hello,
>
> Carsten Dominik <dominik@uva.nl> writes:
>
> > Attached is a patch that does the following:
> >
> > It consolidates all four different org-capture target types that have to
> do
> > with
> > date/week trees into a single one, called `file+olp+datetree'.  This
> target
> > allows for an optional outline path specification to tell capture to
> build
> > the
> > datetree under a specific headline.  To switch to a week tree, or to
> force
> > a date prompt is now the matter of setting one of the properties in the
> > org-capture-template variable.
>
> It sounds good. Thank you.
>

Allright.


>
> > Everything works transparently, so users can update the way they
> > write their datetree captures, but they don't have to - the old syntax
> > remains
> > supported and will automatically switched when one uses customize to
> change
> > the variable.
>
> I am a bit worried by this compatibility layer. I mean, it is good to
> preserve compatibility with old templates, but it ought to be an
> ephemeral solution. I.e., no more
> `org-table--error-on-old-row-references' lingering around for ages.
>
> We could, for example, generate a deprecation warning when old templates
> are used. Then we will be able to remove this unnecessary piece of code
> in next major release.
>

We do disagree here a bit.  This little bit of extra work just keeps the
existing templates working.  We do not introduce a really different
structure of the org-capture-templates.  Rather, the code introduces a new
target type, and it makes some older target types be implemented as special
versions of the new ones.  The old targets are no longer in the manual, any
customize user will be switched automatically.  What remains is a small bit
of code that makes sure that the setup of user who might have been using
that for a long time continues to work.

In my eyes, this is worth it.  Breaking something in a new version is no
big deal for people who use Org regularly, but I am sure there are a lot of
users out there who have not changed their setup for a long time, have not
followed the discussions here and would be frustrated if their setup breaks
after getting a new version of Emacs, for example.  So we can shoot a
warning, but I would vote for just keeping this piece of code indefinitely.


>
> See for example the end of `org-open-file', although it is a bit more
> drastic (it raises an error, not a warning).
>
> > After a bit more testing, I'd like to apply this patch.  Please let me
> know
> > if you agree. And additional testers would be useful.  Anyone?  Make sure
> > to backup your capture templates if something goes wrong.
>
> Some comments follow.
>
> > -@item (file+weektree+prompt "path/to/file")
> > -Will create a heading in a week tree, but will prompt for the date.
> > +one matched.}.  If the optional outline path is given, the tree will be
> built
> > +under the node it is pointing to. Check out the @code{:time-prompt} and
>
> There's a missing space above.
>

I will fix that in the next patch.


>
> > +(defun org-capture-upgrade-templates (templates)
> > +  "Update the template list to the new format.
> > +The new format unifies all the date/week tree targets into one that
> > +also allows for an optional outline path to specify a target."
> > +  (let (target props)
> > +    (mapcar
> > +     (lambda (ee)
> > +       (setq target (car (nth 3 ee)))
> > +       (when (memq target '(file+datetree file+datetree+prompt
> > +                                       file+weektree
> file+weektree+prompt))
> > +      (setq target (symbol-name target) props nil)
> > +      (if (string-match "prompt" target) (setq props '(:time-prompt t)))
> > +      (if (string-match "week" target)
> > +          (setq props (append '(:tree-type week) props)))
> > +      (setcar (nth 3 ee) 'file+olp+datetree)
> > +      (setcdr (nthcdr 4 ee) (append props (nthcdr 5 ee))))
> > +       ee)
> > +     templates)))
>
> I suggest the following. Less `setq', `setcar', `setcdr' makes Org
> a better place.
>
> (defun org-capture-upgrade-templates (templates)
>   "Update the template list to the new format.
> TEMPLATES is a template list, as in `org-capture-templates'. The
> new format unifies all the date/week tree targets into one that
> also allows for an optional outline path to specify a target."
>   (mapcar
>    (lambda (template)
>      (pcase template
>        ;; Match templates with an obsolete "tree" target type. Replace
>        ;; it with common `file+olp-datetree'.  Add new properties
>        ;; (i.e., `:time-prompt' and `:tree-type') if needed.
>        (`(,key ,desc ,type (file+datetree . ,path) ,template . ,props)
>         `(,key ,desc ,type (file+olp+datetree ,@path) ,@props))
>        (`(,key ,desc ,type (file+datetree+prompt . ,path) ,template .
> ,props)
>         `(,key ,desc ,type (file+olp+datetree ,@path) :time-prompt t
> ,@props))
>        (`(,key ,desc ,type (file+weektree . ,path) ,template . ,props)
>         `(,key ,desc ,type (file+olp+datetree ,@path) :tree-type week
> ,@props))
>        (`(,key ,desc ,type (file+weektree+prompt . ,path) ,template .
> ,props)
>         `(,key ,desc ,type (file+olp+datetree ,@path) :tree-type week
> :time-prompt t ,@props))
>        ;; Other templates are left unchanged.
>        (_ template)))
>    templates))
>

OK, no objections to a different implementation.  I am not familiar with
pcase, looks general and useful, should learn about it.


>
> > -             (file+weektree+prompt \"path/to/file\")
> > -                 Will create a heading in a week tree, prompts for date
> > +             (file+olp+datetree \"path/to/file\" \"Level 1 heading\"
> ...)
> > +                 Will create a heading in a date tree for today's date.
> > +                 If no headings are given, the tree will be on top
> level.
>
> Nitpick: It may be just me, but "If no heading is given" sounds better.
>

OK.


>
> >  ELisp programs can set KEYS to a string associated with a template
> >  in `org-capture-templates'.  In this case, interactive selection
> >  will be bypassed.
> > @@ -902,6 +920,7 @@ Store them in the capture property list."
> >          (insert "* " headline "\n")
> >          (beginning-of-line 0)))
> >       (`(file+olp ,path . ,outline-path)
> > +      (setq outline-path (org-capture-sanitize-olp outline-path))
>
> See below about `org-capture-sanitize-olp'.
>
> Also, it is better to make the `setq' a let-binding when possible.
>

Hmm, maybe I misunderstand pcase.  I was under the impression that when
pcase does the match, it will bind the path to outline-path locally (with
let or something similar), so that I can, in the scope of the current
match, use setq to change the variable.

Is my understanding incorrect here?


> >        (let ((m (org-find-olp (cons (org-capture-expand-file path)
> >                                     outline-path))))
> >          (set-buffer (marker-buffer m))
> > @@ -922,59 +941,65 @@ Store them in the capture property list."
> >          (org-capture-put :exact-position (point))
> >          (setq target-entry-p
> >                (and (derived-mode-p 'org-mode) (org-at-heading-p)))))
>
> [...]
>
> > +     (`(file+olp+datetree ,path . ,outline-path)
> > +      (setq outline-path (org-capture-sanitize-olp outline-path))
>
> Ditto.
>
> > +      (let ((m (if outline-path
> > +                   (org-find-olp (cons (org-capture-expand-file path)
> > +                                       outline-path))
> > +                 (set-buffer (org-capture-target-buffer path))
> > +                 (move-marker (make-marker) (point)))))
> > +        (set-buffer (marker-buffer m))
> > +        (org-capture-put-target-region-and-position)
> > +        (widen)
> > +        (goto-char m)
> > +        (set-marker m nil)
> > +        (require 'org-datetree)
> > +        (org-capture-put-target-region-and-position)
> > +        (widen)
> > +        ;; Make a date/week tree entry, with the current date (or
> > +        ;; yesterday, if we are extending dates for a couple of hours)
> > +        (funcall
> > +         (if (eq (org-capture-get :tree-type) 'week)
> > +             #'org-datetree-find-iso-week-create
> > +           #'org-datetree-find-date-create)
> > +         (calendar-gregorian-from-absolute
> > +          (cond
> > +           (org-overriding-default-time
> > +            ;; Use the overriding default time.
> > +            (time-to-days org-overriding-default-time))
> > +           ((org-capture-get :time-prompt)
> > +            ;; Prompt for date.
> > +            (let ((prompt-time (org-read-date
> > +                                nil t nil "Date for tree entry:"
> > +                                (current-time))))
> > +              (org-capture-put
> > +               :default-time
> > +               (cond ((and (or (not (boundp 'org-time-was-given))
> > +                               (not org-time-was-given))
> > +                           (not (= (time-to-days prompt-time)
> (org-today))))
> > +                      ;; Use 00:00 when no time is given for another
> > +                      ;; date than today?
> > +                      (apply #'encode-time
> > +                             (append '(0 0 0)
> > +                                     (cl-cdddr (decode-time
> prompt-time)))))
> > +                     ((string-match "\\([^ ]+\\)--?[^ ]+[ ]+\\(.*\\)"
> > +                                    org-read-date-final-answer)
> > +                      ;; Replace any time range by its start.
> > +                      (apply #'encode-time
> > +                             (org-read-date-analyze
> > +                              (replace-match "\\1 \\2" nil nil
> > +                                             org-read-date-final-answer)
> > +                              prompt-time (decode-time prompt-time))))
> > +                     (t prompt-time)))
> > +              (time-to-days prompt-time)))
> > +           (t
> > +            ;; Current date, possibly corrected for late night
> > +            ;; workers.
> > +            (org-today))))
> > +         ;; the following is the keep-restriction argument for
> > +         ;; org-datetree-find-date-create
>
>   ;; The following...
>   ;; ... `org-datetree-find-date-create'.
>
> > +         (if outline-path 'subtree-at-point)
>
>   (and outline-path 'subtree-at-point)
>
> > +         )))
>
> These trailing parens need to be moved above.
>

OK.


>
> >       (`(file+function ,path ,function)
> >        (set-buffer (org-capture-target-buffer path))
> >        (org-capture-put-target-region-and-position)
> > @@ -1432,6 +1457,13 @@ Use PREFIX as a prefix for the name of the
> indirect buffer."
> >    (unless (org-kill-is-subtree-p tree)
> >      (error "Template is not a valid Org entry or tree")))
> >
> > +(defun org-capture-sanitize-olp (olp)
> > +  "Keep only non-white strings in the list OPL."
> > +  (let (res e)
> > +    (while (setq e (pop olp))
> > +      (and (stringp e) (string-match "\\S-" e) (push e res)))
> > +    (nreverse res)))
>
> This function is really a one-liner:
>
>   (cl-remove-if-not #'org-string-nw-p olp)
>
> Therefore, I don't think it deserves a dedicated function.
>
> BTW is there any reason to prune non-white strings from the list? Is an
> empty string even invalid, since document can contain blank headlines?
>
> The outline path is provided by the user. We can assume he knows what he
> is doing.
>
> Another option would be to barf if the outline path contains any invalid
> value instead of silently "fixing" it on user's behalf. E.g.,
>
>   (when (cl-some #'invalid-value-check olp)
>     (user-error "Invalid outline path in template %S" template))
>
> for some value of #'invalid-value-check.
>
> WDYT?
>

Well, I agree with you that we should not even have this code in here.  It
is a hack to solve an issue I was not able to crack - maybe you can.  Let
me explain:

When I use customize to insert a template definition with the new
file+olp+datetree target, I want the outline path to be optional.

Here is the relevant part of the customize type definition:

 (list :tag "File [ & Outline path ] & Date tree"
(const :format "" file+olp+datetree)
,file-variants
(repeat :tag "Outline path" :inline t
(string :tag "Headline")))

The problem is that customize sets this up assuming at least one string, in
the customize buffer it looks like this

            Target location: Value Menu File [ & Outline path ] & Date tree:
            Filename       : Value Menu Literal:
            Outline path:
            INS DEL Headline:
            INS
            Template       : Value Menu String:

As you can see, without the user clicking INS, there is already a string
there. So the user would have to click DEL to make the old empty.  I
figured people would forget all the time, therefore the hack to remove
empty headlines.  It is critical that the outline path to be empty, because
that is used to decide if the date trree will be on top level or under a
heading.

Do you or anyone know how to tweak customize to start out with an empty OLP
for this application?  The we can remove that part entirely.  Otherwise, I
am happy to make it a oneliner.


>
> >  When it is nil, the buffer will be widened to make sure an existing date
> > -tree can be found."
> > +tree can be found.  If it is the sympol `subtree-at-point', then the
> tree
> > +will be built under the headline at point."
> >    (setq-local org-datetree-base-level 1)
> > -  (or keep-restriction (widen))
> >    (save-restriction
> > -    (let ((prop (org-find-property "DATE_TREE")))
> > -      (when prop
> > -     (goto-char prop)
> > -     (setq-local org-datetree-base-level
> > -                 (org-get-valid-level (org-current-level) 1))
> > -     (org-narrow-to-subtree)))
> > +    (if (eq keep-restriction 'subtree-at-point)
> > +     (progn
> > +       (or (org-at-heading-p) (error "Not at heading"))
>
> Nitpick:
>
>   (unless (org-at-heading-p) (error "Not at heading"))
>

OK


>
> > +       (widen)
> > +       (org-narrow-to-subtree)
> > +       (setq-local org-datetree-base-level
> > +                   (org-get-valid-level (org-current-level) 1)))
> > +      (or keep-restriction (widen))
>
>   (unless keep-restriction (widen))
>

OK


>
> > +      ;; support the old way of tree placement, using a property
>
>   ;; Support ... using a property.
>

OK


>
> > +      (let ((prop (org-find-property "DATE_TREE")))
> > +     (when prop
> > +       (goto-char prop)
> > +       (setq-local org-datetree-base-level
> > +                   (org-get-valid-level (org-current-level) 1))
> > +       (org-narrow-to-subtree))))
> >      (goto-char (point-min))
> >      (let ((year (calendar-extract-year d))
> >         (month (calendar-extract-month d))
> > @@ -84,18 +93,26 @@ tree can be found."
> >    "Find or create an ISO week entry for date D.
> >  Compared to `org-datetree-find-date-create' this function creates
> >  entries ordered by week instead of months.
> > -If KEEP-RESTRICTION is non-nil, do not widen the buffer.  When it
> > -is nil, the buffer will be widened to make sure an existing date
> > -tree can be found."
> > +When it is nil, the buffer will be widened to make sure an existing date
> > +tree can be found.  If it is the sympol `subtree-at-point', then the
> tree
> > +will be built under the headline at point."
> >    (setq-local org-datetree-base-level 1)
> > -  (or keep-restriction (widen))
> >    (save-restriction
> > -    (let ((prop (org-find-property "WEEK_TREE")))
> > -      (when prop
> > -     (goto-char prop)
> > -     (setq-local org-datetree-base-level
> > -                 (org-get-valid-level (org-current-level) 1))
> > -     (org-narrow-to-subtree)))
> > +    (if (eq keep-restriction 'subtree-at-point)
> > +     (progn
> > +       (or (org-at-heading-p) (error "Not at heading"))
> > +       (widen)
> > +       (org-narrow-to-subtree)
> > +       (setq-local org-datetree-base-level
> > +                   (org-get-valid-level (org-current-level) 1)))
> > +      (or keep-restriction (widen))
> > +      ;; support the old way of tree placement, using a property
> > +      (let ((prop (org-find-property "WEEK_TREE")))
> > +     (when prop
> > +       (goto-char prop)
> > +       (setq-local org-datetree-base-level
> > +                   (org-get-valid-level (org-current-level) 1))
> > +       (org-narrow-to-subtree))))
>
> See above, since the same suggestions apply. Also, it looks like there
> is some code duplication involved here. Would it make sense to factor
> the common part out of them?
>

I'll take a look if it does make sense and do it if it is easy.  I see it
as a separate issue since the week tree was implemented using a copy of the
date tree function.  But I can merge this change into the patch I am making.


>
> >      (goto-char (point-min))
> >      (require 'cal-iso)
> >      (let* ((year (calendar-extract-year d))
> >       Modified lisp/org.el
> > diff --git a/lisp/org.el b/lisp/org.el
> > index 38fce70..ac56d71 100644
> > --- a/lisp/org.el
> > +++ b/lisp/org.el
> > @@ -16615,6 +16615,8 @@ only headings."
> >        end found flevel)
> >      (unless buffer (error "File not found :%s" file))
> >      (with-current-buffer buffer
> > +      (unless (derived-mode-p 'org-mode)
> > +     (error "Buffer %s needs to be in org-mode" buffer))
>
> Nitpick: "Buffer %s needs to be in Org mode"
>

OK


>
> Ideally, a bunch of tests in test-org-capture.el would be nice.
>

Will do so after we have converged.

Regards

Carsten


>
>
> Regards,
>
> --
> Nicolas Goaziou
>

[-- Attachment #2: Type: text/html, Size: 24793 bytes --]

Re: Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik <dominik@uva.nl> writes:

> We do disagree here a bit.  This little bit of extra work just keeps the
> existing templates working.  We do not introduce a really different
> structure of the org-capture-templates.  Rather, the code introduces a new
> target type, and it makes some older target types be implemented as special
> versions of the new ones.  The old targets are no longer in the manual, any
> customize user will be switched automatically.  What remains is a small bit
> of code that makes sure that the setup of user who might have been using
> that for a long time continues to work.
>
> In my eyes, this is worth it.  Breaking something in a new version is no
> big deal for people who use Org regularly, but I am sure there are a lot of
> users out there who have not changed their setup for a long time, have not
> followed the discussions here and would be frustrated if their setup breaks
> after getting a new version of Emacs, for example.  So we can shoot a
> warning, but I would vote for just keeping this piece of code
> indefinitely.

Our sole disagreement is above duration. "forever" is a bit long and
unrealistic to me, particularly when it is about supporting undocumented
features.

I'm not suggesting to remove the conversion right now. It could be in
Org 10.0 or even Org 11.0 (As of Org 9.0.4 we still support variables
and functions deprecated since Org 8.0). But at least, at some point we
know that some compatibility layer can be removed instead of letting it
pile up indefinitely.

As a user, I expect changes to happens any time I update a software
I use to some higher major version. What I suggest is simply to send
a warning whenever a template using old methods is used, in addition to
doing the conversion. After a couple of years of seeing this message,
I'm pretty sure almost all users will have switched to the new pattern.

Anyway, I'm not going to oppose this patch just for that point. At least
you have my opinion.

> OK, no objections to a different implementation.  I am not familiar with
> pcase, looks general and useful, should learn about it.

Pattern matching is a very interesting feature, indeed. There are other
possible implementations of the same functin that do not rely on
`pcase', however. My point is more about reducing our use of `setq' at
a minimum, particularly since we moved to lexical scoping.

> Hmm, maybe I misunderstand pcase.  I was under the impression that when
> pcase does the match, it will bind the path to outline-path locally (with
> let or something similar), so that I can, in the scope of the current
> match, use setq to change the variable.
>
> Is my understanding incorrect here?

Your understanding is correct, and it is syntactically right to use
`setq' here.

However, as I pointed out above, it is better to use a let-binding over
a `setq' whenever possible since:

  1. it is faster,
  2. it makes the scope of the variable explicit.

> Well, I agree with you that we should not even have this code in here.  It
> is a hack to solve an issue I was not able to crack - maybe you can.  Let
> me explain:
>
> When I use customize to insert a template definition with the new
> file+olp+datetree target, I want the outline path to be optional.
>
> Here is the relevant part of the customize type definition:
>
>  (list :tag "File [ & Outline path ] & Date tree"
> (const :format "" file+olp+datetree)
> ,file-variants
> (repeat :tag "Outline path" :inline t
> (string :tag "Headline")))
>
> The problem is that customize sets this up assuming at least one string, in
> the customize buffer it looks like this
>
>             Target location: Value Menu File [ & Outline path ] & Date tree:
>             Filename       : Value Menu Literal:
>             Outline path:
>             INS DEL Headline:
>             INS
>             Template       : Value Menu String:
>
> As you can see, without the user clicking INS, there is already a string
> there. So the user would have to click DEL to make the old empty.  I
> figured people would forget all the time, therefore the hack to remove
> empty headlines.  It is critical that the outline path to be empty, because
> that is used to decide if the date trree will be on top level or under a
> heading.
>
> Do you or anyone know how to tweak customize to start out with an empty OLP
> for this application?  The we can remove that part entirely.  Otherwise, I
> am happy to make it a oneliner.

Wouldn't something like

         (choice
          (const :tag "Top level" :inline t nil)
          (repeat :tag "Outline path" :inline t
                  (string :tag "Headline")))

do the job?

> I'll take a look if it does make sense and do it if it is easy.  I see it
> as a separate issue since the week tree was implemented using a copy of the
> date tree function.  But I can merge this change into the patch I am making.

Of course, this is not a blocker. This came to mind when I was reading
your patch.

>> Ideally, a bunch of tests in test-org-capture.el would be nice.
>>
>
> Will do so after we have converged.

Great. Thank you.

Regards,

-- 
Nicolas Goaziou