Re: [O] Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik  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 

Re: [O] Allowing multiple date trees in a single file

[Carsten Dominik]

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 
wrote:

> Hello,
>
> Carsten Dominik  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 

Re: [O] Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik  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 

Re: [O] Allowing multiple date trees in a single file

[Carsten Dominik]

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 
wrote:

> Hello,
>
> Carsten Dominik  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
>


datetree-patch
Description: Binary data

Re: [O] Allowing multiple date trees in a single file

[Carsten Dominik]

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

Re: [O] 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: [O] Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik  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: [O] Allowing multiple date trees in a single file

[Carsten Dominik]

On Tue, Jan 17, 2017 at 6:50 PM, Nicolas Goaziou 
wrote:

> Hello,
>
> Carsten Dominik  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
>

Re: [O] Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik  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: [O] Allowing multiple date trees in a single file

[Carsten Dominik]

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 
wrote:

> Hello,
>
> Carsten Dominik  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: [O] Allowing multiple date trees in a single file

[Nicolas Goaziou]

Hello,

Carsten Dominik  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

[O] Allowing multiple date trees in a single file

[Carsten Dominik]

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


patch-datetree
Description: Binary data