emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Richard Lawrence <richard.lawrence@uni-tuebingen.de>
To: Kyle Meyer <kyle@kyleam.com>
Cc: emacs-orgmode@gnu.org
Subject: Re: [PATCH] capture: Fix handling of time range for :time-prompt
Date: Sun, 31 Jan 2021 12:15:10 +0100	[thread overview]
Message-ID: <87a6spz7b5.fsf@aquinas> (raw)
In-Reply-To: <878s8pll2g.fsf@kyleam.com>

Dear Kyle and all,

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

Kyle Meyer <kyle@kyleam.com> 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 <richard.lawrence@berkeley.edu>
> 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 (&optional 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
> <https://orgmode.org/list/20120923194954.GE25237@boo.workgroup/T/#u>.

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.

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.

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?

-- 
Best,
Richard


  reply	other threads:[~2021-01-31 11:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-24 10:42 Bug: incorrect timestamps with :time-prompt and datetrees Richard Lawrence
2020-12-24 12:07 ` Richard Lawrence
2021-01-06 12:16   ` Richard Lawrence
2021-01-12  8:41     ` [PATCH] " Richard Lawrence
2021-01-18  6:35     ` Bug: " Kyle Meyer
2021-01-19  2:25       ` [PATCH] capture: Fix handling of time range for :time-prompt Kyle Meyer
2021-01-31 11:15         ` Richard Lawrence [this message]
2021-02-02  4:42           ` Kyle Meyer
2021-02-02 16:59             ` Richard Lawrence

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87a6spz7b5.fsf@aquinas \
    --to=richard.lawrence@uni-tuebingen.de \
    --cc=emacs-orgmode@gnu.org \
    --cc=kyle@kyleam.com \
    --subject='Re: [PATCH] capture: Fix handling of time range for :time-prompt' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).