unofficial mirror of emacs-orgmode@gnu.org
 help / color / mirror / Atom feed
* Bug: incorrect timestamps with :time-prompt and datetrees
@ 2020-12-24 10:42 Richard Lawrence
  2020-12-24 12:07 ` Richard Lawrence
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Lawrence @ 2020-12-24 10:42 UTC (permalink / raw)
  To: emacs-orgmode

Hi all,

I ran into a subtle bug yesterday. Basically, when using org-capture to
capture

  - an entry into a datetree,
  - on a date other than today (using :time-prompt in org-capture-templates)
  - with a capture template that inserts a timestamp (%T)

then I get incorrect results for either the timestamp or the location in
the datetree, depending on how I enter the date.

Here is a minimal working example:

#+begin_src emacs-lisp
  (setq org-capture-templates
      `(
        ("d" "Diary")
        ("da" "Appointment/Event" entry
         (file+olp+datetree "/tmp/diary.org")
         "**** %^{Description}\n     %T"
         :time-prompt t)))
#+end_src
Notice that the template contains %T, to expand to a timestamp with a
time, and also that the capture target is a datetree and :time-prompt is
true. My understanding is that this should both file the entry to the
entered date in the datetree, and fill the %T template with a timestamp
for the entered date (and time, if any).
  
Here are the results from running a few captures with this setup on
Dec 24 around 11AM. I tried various ways of scheduling an event on Dec 31
at 13:00; what I entered into the prompt of org-read-date is shown in
the headline:

#+begin_example

* 2020

** 2020-12 December

*** 2020-12-24 Thursday
**** Entered "31-12" 
     <2020-12-31 Thu 11:06>

This gets filed to the wrong day in the datetree, but the date in the
timestamp is correct. The time is also correct (it's the current
time, since no time was entered).

**** Entered "31-12 13:00" 
     <2020-12-31 Thu 13:00>

This gets filed to the wrong day in the datetree, but the date and time
in the timestamp are correct.
     
*** 2020-12-31 Thursday
**** Entered "12-31" 
     <2020-12-31 Thu 00:00>

This gets filed to the correct day in the datetree and the date in the
timestamp is correct. The time is 00:00 because no time was entered.
(Why isn't the time 11:06, though, like in the first example?)

**** Entered "12-31 13:00" 
     <2021-01-12 Tue 13:00>

This is the most problematic case: it gets filed to the correct day in
the datetree, but the date in the timestamp is incorrect!

**** Entered "2020-12-31 13:00" 
     <2020-12-31 Thu 13:00>

If I enter the date in full ISO format, the location in the datetree and
the timestamp are both correct.

#+end_example

Possibly relevant here is the value of calendar-date-style, which is
'american for me. I tested briefly with 'european but it did not make a
difference for the "31-12" cases.

This is org 9.4 running from maint (commit ab00524fc). I spent a while
stepping through org-capture and org-read-date but haven't found the
problem yet. I suspect this snippet from a cond form in the middle of
org-capture-set-target-location:

#+begin_src
    ((string-match "\\([^ ]+\\)-[^ ]+[ ]+\\(.*\\)"
                    org-read-date-final-answer)
        ;; Replace any time range by its start.
        (apply #'encode-time
            (org-read-date-analyze
                ;; it looks to me like this is maybe sending the wrong value
                ;; into org-read-date-analyze: 
                (replace-match "\\1 \\2" nil nil  
                               org-read-date-final-answer)

#+end_src

Will report here if I find out more exactly.
Happy holidays, everyone!

-- 
Best,
Richard


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Bug: incorrect timestamps with :time-prompt and datetrees
  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
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Lawrence @ 2020-12-24 12:07 UTC (permalink / raw)
  To: emacs-orgmode

Richard Lawrence <richard.lawrence@uni-tuebingen.de> writes:

> This is org 9.4 running from maint (commit ab00524fc). I spent a while
> stepping through org-capture and org-read-date but haven't found the
> problem yet. I suspect this snippet from a cond form in the middle of
> org-capture-set-target-location:
>
> #+begin_src
>     ((string-match "\\([^ ]+\\)-[^ ]+[ ]+\\(.*\\)"
>                     org-read-date-final-answer)
>         ;; Replace any time range by its start.
>         (apply #'encode-time
>             (org-read-date-analyze
>                 ;; it looks to me like this is maybe sending the wrong value
>                 ;; into org-read-date-analyze: 
>                 (replace-match "\\1 \\2" nil nil  
>                                org-read-date-final-answer)
>
> #+end_src

I can now confirm that this is the problem. It looks like what is happening here
is that the regex is meant to match a time range, but ends up matching
the date: thus a string like "12-31 13:00" gets mangled to "12 13:00"
and sent into org-read-date-analyze.  If I comment out this branch of
the cond form, and enter "12-31 13:00" at the org-read-date-prompt, then
both the timestamp and the location in the datetree are correct.

So I guess the solution is...a better regex is needed to cause this
branch to fire only in the intended case, namely, a time range. Should
it be checking for "am"/"pm" or ":" or similar? Otherwise it seems
impossible to distinguish a date specification like "11-12" from a time
range. 

-- 
Best,
Richard


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Bug: incorrect timestamps with :time-prompt and datetrees
  2020-12-24 12:07 ` Richard Lawrence
@ 2021-01-06 12:16   ` Richard Lawrence
  2021-01-12  8:41     ` [PATCH] " Richard Lawrence
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Lawrence @ 2021-01-06 12:16 UTC (permalink / raw)
  To: emacs-orgmode

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

Hi everyone,

Richard Lawrence <richard.lawrence@uni-tuebingen.de> writes:

> I can now confirm that this is the problem. It looks like what is happening here
> is that the regex is meant to match a time range, but ends up matching
> the date: thus a string like "12-31 13:00" gets mangled to "12 13:00"
> and sent into org-read-date-analyze.

> So I guess the solution is...a better regex is needed to cause this
> branch to fire only in the intended case, namely, a time range. 

Here is a patch for this issue. It uses a narrower regex to match a time
range. This regex requires time ranges to have ":MM" or an AM/PM
specification in the end time, to prevent mangling strings that are
interpreted as dates, like "11-12".

This patch is a minimal change that gets the code working in the way
that seems to have been intended, so it seems worth applying to maint.

However, the way the code is intended to work doesn't seem right to me,
because it simply throws away time range information at the time prompt.
If you enter a time range like "13:00-14:00" at the time prompt, you
will get a timestamp with "13:00" for the time when the %T template is
expanded. (This is because org-capture-set-target-location uses the
beginning of the entered time range to set :default-time, which must be
an encoded time value, and there is no obvious way to set a time range.)
This is a surprising contrast with the behavior of %^T, which preserves
the time range information in the timestamp entered. But fixing this
will be a larger change and possibly requires some discussion.

-- 
Best,
Richard


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-capture-fix-expansion-of-T-when-capturing-to-a-d.patch --]
[-- Type: text/x-diff, Size: 1326 bytes --]

From a41c751f15488a8b0d48d6b1b1744dbbc003f9f0 Mon Sep 17 00:00:00 2001
From: Richard Lawrence <richard.lawrence@berkeley.edu>
Date: Wed, 6 Jan 2021 11:53:42 +0100
Subject: [PATCH] org-capture: fix expansion of %T when capturing to a datetree

* org-capture.el (org-capture-set-target-location): Use a narrower
regular expression to replace a time range by its start time when
setting :default-time, so that dates do not get mangled.

TINYCHANGE
---
 lisp/org-capture.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index f40f2b335..df0eccdbb 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1038,12 +1038,12 @@ Store them in the capture property list."
 			 (apply #'encode-time 0 0
 				org-extend-today-until
 				(cl-cdddr (decode-time prompt-time))))
-			((string-match "\\([^ ]+\\)-[^ ]+[ ]+\\(.*\\)"
+			((string-match "\\(--?\\([012]?[0-9]\\)\\(\\(:[0-5][0-9]\\)\\|\\(am\\|AM\\|pm\\|PM\\)\\>\\)\\)\\(.*\\)"
 				       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
+				 (replace-match "\\6" nil nil
 						org-read-date-final-answer)
 				 prompt-time (decode-time prompt-time))))
 			(t prompt-time)))
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] incorrect timestamps with :time-prompt and datetrees
  2021-01-06 12:16   ` Richard Lawrence
@ 2021-01-12  8:41     ` Richard Lawrence
  0 siblings, 0 replies; 4+ messages in thread
From: Richard Lawrence @ 2021-01-12  8:41 UTC (permalink / raw)
  To: emacs-orgmode

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

Hi everyone,

Bumping this, since I forgot to put "PATCH" in the subject line before.

Richard Lawrence <richard.lawrence@uni-tuebingen.de> writes:

> Here is a patch for this issue. It uses a narrower regex to match a time
> range. This regex requires time ranges to have ":MM" or an AM/PM
> specification in the end time, to prevent mangling strings that are
> interpreted as dates, like "11-12".
>
> This patch is a minimal change that gets the code working in the way
> that seems to have been intended, so it seems worth applying to maint.
>
> However, the way the code is intended to work doesn't seem right to me,
> because it simply throws away time range information at the time prompt.
> If you enter a time range like "13:00-14:00" at the time prompt, you
> will get a timestamp with "13:00" for the time when the %T template is
> expanded. (This is because org-capture-set-target-location uses the
> beginning of the entered time range to set :default-time, which must be
> an encoded time value, and there is no obvious way to set a time range.)
> This is a surprising contrast with the behavior of %^T, which preserves
> the time range information in the timestamp entered. But fixing this
> will be a larger change and possibly requires some discussion.

-- 
Best,
Richard


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-capture-fix-expansion-of-T-when-capturing-to-a-d.patch --]
[-- Type: text/x-diff, Size: 1326 bytes --]

From a6c223664aad6096943e236c9a51c30246e57669 Mon Sep 17 00:00:00 2001
From: Richard Lawrence <richard.lawrence@berkeley.edu>
Date: Wed, 6 Jan 2021 11:53:42 +0100
Subject: [PATCH] org-capture: fix expansion of %T when capturing to a datetree

* org-capture.el (org-capture-set-target-location): Use a narrower
regular expression to replace a time range by its start time when
setting :default-time, so that dates do not get mangled.

TINYCHANGE
---
 lisp/org-capture.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/org-capture.el b/lisp/org-capture.el
index f40f2b335..df0eccdbb 100644
--- a/lisp/org-capture.el
+++ b/lisp/org-capture.el
@@ -1038,12 +1038,12 @@ Store them in the capture property list."
 			 (apply #'encode-time 0 0
 				org-extend-today-until
 				(cl-cdddr (decode-time prompt-time))))
-			((string-match "\\([^ ]+\\)-[^ ]+[ ]+\\(.*\\)"
+			((string-match "\\(--?\\([012]?[0-9]\\)\\(\\(:[0-5][0-9]\\)\\|\\(am\\|AM\\|pm\\|PM\\)\\>\\)\\)\\(.*\\)"
 				       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
+				 (replace-match "\\6" nil nil
 						org-read-date-final-answer)
 				 prompt-time (decode-time prompt-time))))
 			(t prompt-time)))
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-01-12  8:42 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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

unofficial mirror of emacs-orgmode@gnu.org

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/orgmode/0 orgmode/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 orgmode orgmode/ https://yhetil.org/orgmode \
		emacs-orgmode@gnu.org
	public-inbox-index orgmode

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.emacs.orgmode
	nntp://news.gmane.io/gmane.emacs.orgmode


AGPL code for this site: git clone http://ou63pmih66umazou.onion/public-inbox.git