Re: [O] [PATCH 2/3] org-datetree.el: Add support for ISO week trees.

[Nicolas Goaziou]

Hello,

Rüdiger Sonderfeld  writes:

> I don't think that's necessarily true since both formats don't mix well.  If 
> anyone still wants it then they can simply add both properties.

OK, then please document the property in the manual, and add an entry in
ORG-NEWS.

Otherwise, LGTM.


Regards,

-- 
Nicolas Goaziou

Re: [O] [PATCH 2/3] org-datetree.el: Add support for ISO week trees.

[Rüdiger Sonderfeld]

On Thursday 03 September 2015 07:55:08 Nicolas Goaziou wrote:
> Rüdiger Sonderfeld  writes:
> > On Wednesday 02 September 2015 21:58:17 Nicolas Goaziou wrote:
> >> Rüdiger Sonderfeld  writes:
> >> > +(let ((prop (org-find-property "DATE_WEEK_TREE")))
> >> 
> >> I don't think we need to introduce a new property for that. DATE_TREE is
> >> enough.
> > 
> > Since DATE_TREE and DATE_WEEK_TREE (or WEEK_TREE instead?) are structured
> > differently it might make sense to keep the property separated.
> 
> If you want both a date tree and a week tree in the same, you probably
> want them to start at the same level, don't you?

I don't think that's necessarily true since both formats don't mix well.  If 
anyone still wants it then they can simply add both properties.

I've send updated versions of the patches as a reply to this message.

Regards,

Rüdiger

Re: [O] [PATCH 2/3] org-datetree.el: Add support for ISO week trees.

[Rüdiger Sonderfeld]

On Wednesday 02 September 2015 21:58:17 Nicolas Goaziou wrote:
> Rüdiger Sonderfeld  writes:
> > +(let ((prop (org-find-property "DATE_WEEK_TREE")))
> 
> I don't think we need to introduce a new property for that. DATE_TREE is
> enough.

Since DATE_TREE and DATE_WEEK_TREE (or WEEK_TREE instead?) are structured 
differently it might make sense to keep the property separated.

> > +  ;; ISO 8601 week format is %G-W%V(-%u)
> > +  (org-datetree--find-create "^\\*+[
> > \t]+\\([12][0-9]\\{3\\}\\)\\(\\s-*?\\([
> > \t]:[[:alnum:]:_@#%%]+:\\)?\\s-*$\\)"
> Isn't this line too long?

What's the limit?  Because if it's 80 char then I'd need to do some `concat' 
ugliness because the regex is over 80 char long.

I've fixed the rest and will send updated patches.

Cheers,

Rüdiger

Re: [O] [PATCH 2/3] org-datetree.el: Add support for ISO week trees.

[Nicolas Goaziou]

Rüdiger Sonderfeld  writes:

> On Wednesday 02 September 2015 21:58:17 Nicolas Goaziou wrote:
>> Rüdiger Sonderfeld  writes:
>> > +(let ((prop (org-find-property "DATE_WEEK_TREE")))
>> 
>> I don't think we need to introduce a new property for that. DATE_TREE is
>> enough.
>
> Since DATE_TREE and DATE_WEEK_TREE (or WEEK_TREE instead?) are structured 
> differently it might make sense to keep the property separated.

If you want both a date tree and a week tree in the same, you probably
want them to start at the same level, don't you?

>> > +  ;; ISO 8601 week format is %G-W%V(-%u)
>> > +  (org-datetree--find-create "^\\*+[
>> > \t]+\\([12][0-9]\\{3\\}\\)\\(\\s-*?\\([
>> > \t]:[[:alnum:]:_@#%%]+:\\)?\\s-*$\\)"
>> Isn't this line too long?
>
> What's the limit?  Because if it's 80 char then I'd need to do some `concat' 
> ugliness because the regex is over 80 char long.

You can use continuation markup, i.e., "\\n".


Regards,

Re: [O] [PATCH 2/3] org-datetree.el: Add support for ISO week trees.

[Nicolas Goaziou]

Rüdiger Sonderfeld  writes:

> +(defun org-datetree-find-iso-week-create (date  keep-restriction)
> +  "Find or create an ISO week entry for DATE.
> +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."
> +  (org-set-local 'org-datetree-base-level 1)
> +  (or keep-restriction (widen))
> +  (save-restriction
> +(let ((prop (org-find-property "DATE_WEEK_TREE")))

I don't think we need to introduce a new property for that. DATE_TREE is
enough.

> +  (when prop
> + (goto-char prop)
> + (org-set-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 date))
> +(month (calendar-extract-month date))
> +(day (calendar-extract-day date))
> +(time (encode-time 0 0 0 day month year))
> +(iso-date (calendar-iso-from-absolute
> +   (calendar-absolute-from-gregorian date)))
> +(weekyear (nth 2 iso-date))
> +(week (car iso-date))
> +(weekday (cadr iso-date)))

Nitpick, since you used (nth 2 ...):

  car  -> nth 0
  cadr -> nth 1

> +  ;; ISO 8601 week format is %G-W%V(-%u)
> +  (org-datetree--find-create "^\\*+[ 
> \t]+\\([12][0-9]\\{3\\}\\)\\(\\s-*?\\([ \t]:[[:alnum:]:_@#%%]+:\\)?\\s-*$\\)"

Isn't this line too long?

> +  weekyear nil nil
> +  (format-time-string "%G" time))
> +  (org-datetree--find-create "^\\*+[ \t]+%d-W\\([0-5][0-9]\\)$"
> +  weekyear week nil
> +  (format-time-string "%G-W%V" time))
> +  ;; For the actual day we use the regular date instead of ISO week.
> +  (org-datetree--find-create "^\\*+[ \t]+%d-%02d-\\([0123][0-9]\\) \\w+$"
> +  year month day
> +
> +(defun org-datetree--find-create (regex year  month day insert)
> +  "Find the datetree matched by REGEX for YEAR, MONTH, or DAY.

Here you have expectations about REGEX, since you use

  (format regex year month day)

Could you clarify that it should have three format placeholders in the
docstring?

> +If INSERT is non-nil insert the text if not found."

Insert where? What text? Could you tweak docstring to specify that
INSERT is a string and where it is going to be inserted?

>(let ((re (format regex year month day))
>   match)
>  (goto-char (point-min))
> @@ -86,25 +126,27 @@ (defun org-datetree--find-create (regex year  
> month day)
>   ((not match)
>(goto-char (point-max))
>(unless (bolp) (newline))
> -  (org-datetree-insert-line year month day))
> +  (org-datetree-insert-line year month day insert))
>   ((= (string-to-number (match-string 1)) (or day month year))
>(goto-char (point-at-bol)))

While you're at it:

  (beginning-of-line)


Regards,