Re: [PATCH] capture: Fix handling of time range for :time-prompt

2021-02-02 Thread Richard Lawrence
Kyle Meyer  writes:

> Pushed (3e64d3475).

Thank you!

>> As far as I can tell, that is not fully possible today, even with this
>> patch. The reason is that time *range* information entered at the prompt
>> generated by :time-prompt gets thrown away. The reason for *that* is
>> that org-read-date is called with t as its to-time argument (the second
>> argument), so that the date is returned in Emacs' internal time
>> representation, which doesn't represent ranges.
>
> Hmm.  Is it really about the TO-TIME argument?  If org-read-date is
> called with TO-TIME as nil, doesn't it still throw away the end of the
> range?
>
>   (let ((org-time-was-given nil)
> (org-end-time-was-given nil))
> (org-read-date nil nil nil "Date for tree entry:"))
>   ;; enter "3pm-4pm" => "2021-02-01 15:00"
>
> Or, actually, it stores the end in org-end-time-was-given, but it does
> that regardless of the TO-TIME argument.

Ah, that's interesting, thanks for pointing it out. I thought I had
checked to see if org-end-time-was-given had a value after org-read-date
gets called for a capture :time-prompt, but now I see that of course
that wouldn't work without the surrounding let binding...   

OK, so with your patch, the way is clear to stick this value in
org-capture-plist and make use of it when filling %T and friends in
templates. I'll see if I can come up with a patch to do that, and
report back.

-- 
Best,
Richard



Re: [PATCH] capture: Fix handling of time range for :time-prompt

2021-02-01 Thread Kyle Meyer
Richard Lawrence writes:

> Kyle Meyer  writes:
>
>> Testing that against the cases in your initial report, I believe it
>> leads to the same results as your patch, so here's a cleaned-up version.
>
> I confirm that this cleaned up version works for me and gets the same
> results for my test cases. I think it should be applied (modulo one
> nitpick below). Are you willing to apply it, Kyle? I don't have commit
> rights myself.

Sure.  Thank you for testing it.

>> -  (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
[...]
>> +  (if (and (or (not org-time-was-given))
>
> Nitpick: (or (not org-time-was-given)) is equivalent to (not 
> org-time-was-given)

Eh, thanks for spotting my sloppy conversion.  Fixed, though I ended up
rearranging things a bit more to avoid unnecessary `not's that were in
the original code.

Pushed (3e64d3475).

> As for this:
>
>> The original change came in b61ff117b (org-capture.el:
>> Set a correct time value with file+datetree+prompt, 2012-09-24), and I
>> believe the related thread is
>> .
>
> Thanks for the reference to this thread. I would like to be able to do
> exactly what Gregor mentioned there:
>
> - be prompted when capturing for the date and time / time range of the
>   event/appointment.
> - record the event/appointment in a date tree under the date entered at
>   the prompt
> - have a timestamp with the full time information entered at the prompt
>   appear in the resulting heading
>
> In short: enter the full date and time information *once*, and use this
> both to place the entry in the datetree and to give it a timestamp.

This isn't functionality I use myself, so I'm not sure I have things
straight in my head, but, yeah, sounds reasonable.
>
> As far as I can tell, that is not fully possible today, even with this
> patch. The reason is that time *range* information entered at the prompt
> generated by :time-prompt gets thrown away. The reason for *that* is
> that org-read-date is called with t as its to-time argument (the second
> argument), so that the date is returned in Emacs' internal time
> representation, which doesn't represent ranges.

Hmm.  Is it really about the TO-TIME argument?  If org-read-date is
called with TO-TIME as nil, doesn't it still throw away the end of the
range?

  (let ((org-time-was-given nil)
(org-end-time-was-given nil))
(org-read-date nil nil nil "Date for tree entry:"))
  ;; enter "3pm-4pm" => "2021-02-01 15:00"

Or, actually, it stores the end in org-end-time-was-given, but it does
that regardless of the TO-TIME argument.

  ;; TO-TIME nil
  (let ((org-time-was-given nil)
(org-end-time-was-given nil))
(org-read-date nil nil nil "Date for tree entry:")
org-end-time-was-given)
  ;; enter "3pm-4pm" => "16:00"

  ;; TO-TIME t
  (let ((org-time-was-given nil)
(org-end-time-was-given nil))
(org-read-date nil t nil "Date for tree entry:")
org-end-time-was-given)
  ;; enter "3pm-4pm" => "16:00"

And the org-time-stamp command uses a non-nil TO-TIME, formatting the
time stamp with something like this:

  ;; org-time-stamp
  (let* ((org-time-was-given nil)
 (org-end-time-was-given nil))
(org-insert-time-stamp
 (org-read-date nil t nil "Date for tree entry:")
 org-time-was-given
 nil nil nil
 (list org-end-time-was-given)))
  ;; enter "3pm-4pm" => "16:00" => <2021-02-01 Mon 15:00-16:00>

> Bastien's recommended solution back then was to use %^t and %^T in the
> capture template instead of %t and %T. The problem with that is that it
> requires entering the date twice: once at the prompt generated by
> :time-prompt, and once at the prompt to replace the %^T in the template.
>
> Could we instead save, say, :start-time and :end-time values in
> org-capture-plist after the :time-prompt, and use them to populate %T,
> %U, etc. in the template? This seems like the right solution to me, but
> it might require modifying org-read-date, which is a hairy piece of
> code. What do others think about this? Should I come up with a patch for
> this, or is there a different solution that the community and
> maintainers would prefer?

I don't have a good grasp of all the details here yet, but something in
that direction sounds worth considering to me.  And perhaps by carrying
along org-end-time-was-given, you won't need to modify org-read-date.

Thanks.



Re: [PATCH] capture: Fix handling of time range for :time-prompt

2021-01-31 Thread Richard Lawrence
Dear Kyle and all,

Thanks for following up! Sorry it's taken me so long to reply.

Kyle Meyer  writes:

> Testing that against the cases in your initial report, I believe it
> leads to the same results as your patch, so here's a cleaned-up version.

I confirm that this cleaned up version works for me and gets the same
results for my test cases. I think it should be applied (modulo one
nitpick below). Are you willing to apply it, Kyle? I don't have commit
rights myself.

> -- >8 --
> Subject: [PATCH] capture: Fix handling of time range for :time-prompt
>
> * lisp/org-capture.el (org-capture-set-target-location): Bind
> org-end-time-was-given around the org-read-date call to get a return
> value that uses the start time rather than doing custom adjustment of
> the return value.
>
> If org-capture-set-target-location detects that the answer to
> org-read-date has a time range, it strips the end time from the answer
> and calls org-read-date-analyze again.  (org-read-date already calls
> it underneath.)  The regexp it uses, however, can easily match a date,
> leading to a bogus result.
>
> org-read-date-analyze is already capable of processing the time range
> in a way that matches org-capture-set-target-location's intent: when
> org-end-time-was-given is bound, org-read-date-analyze splits off the
> end value of the range and stores it in org-end-time-was-given.  Drop
> the custom logic and let org-read-date-analyze handle the range.
>
> Reported-by: Richard Lawrence 
> Ref: https://orgmode.org/list/87h7obh4ct.fsf@aquinas
> ---
>  lisp/org-capture.el | 35 +++
>  1 file changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/lisp/org-capture.el b/lisp/org-capture.el
> index f40f2b335..d7b69f228 100644
> --- a/lisp/org-capture.el
> +++ b/lisp/org-capture.el
> @@ -1025,28 +1025,23 @@ (defun org-capture-set-target-location ( 
> target)
>  (time-to-days org-overriding-default-time))
> ((or (org-capture-get :time-prompt)
>  (equal current-prefix-arg 1))
> -;; Prompt for date.
> -(let ((prompt-time (org-read-date
> -nil t nil "Date for tree entry:")))
> +   ;; Prompt for date.  Bind `org-end-time-was-given' so
> +   ;; that `org-read-date-analyze' handles the time range
> +   ;; case and returns `prompt-time' with the start value.
> +   (let* ((org-time-was-given nil)
> +  (org-end-time-was-given nil)
> +  (prompt-time (org-read-date
> + nil t nil "Date for tree entry:")))
>(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 0 0
> - org-extend-today-until
> - (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)))
> +  (if (and (or (not org-time-was-given))

Nitpick: (or (not org-time-was-given)) is equivalent to (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 0 0
> + org-extend-today-until
> + (cl-cdddr (decode-time prompt-time)))
> +prompt-time))
>(time-to-days prompt-time)))
> (t
>  ;; Current date, possibly corrected for late night
>
> base-commit: 9e8215f4a5df7d03ac787da78d28f69a4c18e7d3

As for this:

> The original change came in b61ff117b (org-capture.el:
> Set a correct time value with file+datetree+prompt, 2012-09-24), and I
> believe the related thread is
> .

Thanks for the reference to this thread. I would like to be able to do
exactly what Gregor mentioned there:

- be prompted when capturing for the date and time / time range of the
  event/appointment.
- record the