unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#72570: 31.0.50; Regression in date-to-time
@ 2024-08-11  8:57 Ulrich Mueller
  2024-08-12  7:27 ` Ulrich Mueller
  2024-08-12 11:42 ` Eli Zaretskii
  0 siblings, 2 replies; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-11  8:57 UTC (permalink / raw)
  To: 72570

Function documentation of 'date-to-time' says:
"If DATE lacks timezone information, GMT is assumed."

Also, the Lispref Manual:
"This function assumes Universal Time if STRING lacks explicit time zone
information, ..."

  (format-time-string
   "%Y-%m-%d %H:%M:%S" (date-to-time "2024-01-01 00:00:00") t)

returns "2023-12-31 23:00:00" as its result, while I would expect
"2024-01-01 00:00:00" with both conversions being in the same (UTC)
timezone. Apparently, date-to-time incorrectly uses the local timezone
(Europe/Berlin) instead of UTC.

In Emacs 23.4 the function worked as expected.

Git bisecting points to this commit:

  commit b069e5a697f37a06704136a8d5376b4d088658c8 (HEAD)
  Author: Gnus developers <ding@gnus.org>
  Date:   Thu Sep 23 00:30:37 2010 +0000

      Merge Changes made in Gnus trunk.

      [...]
      time-date.el (date-to-time): Speed up date-to-time.
      [...]


In GNU Emacs 31.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.18.0) of 2024-08-11 built on localhost
Repository revision: ca56dc2e71660cf501f417ab683590ba2e333661
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: Gentoo Linux

Configured using:
 'configure --prefix=/usr --build=x86_64-pc-linux-gnu
 --host=x86_64-pc-linux-gnu --mandir=/usr/share/man
 --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc
 --localstatedir=/var/lib --datarootdir=/usr/share
 --disable-silent-rules --docdir=/usr/share/doc/emacs-31.0.9999
 --htmldir=/usr/share/doc/emacs-31.0.9999/html --libdir=/usr/lib64
 --program-suffix=-emacs-31-vcs --includedir=/usr/include/emacs-31-vcs
 --infodir=/usr/share/info/emacs-31-vcs --localstatedir=/var
 --enable-locallisppath=/etc/emacs:/usr/share/emacs/site-lisp
 --without-compress-install --without-hesiod --without-pop
 --with-file-notification=inotify --with-pdumper --enable-acl
 --enable-xattr --with-dbus --with-modules --with-gameuser=:gamestat
 --with-libgmp --with-gpm --without-native-compilation
 --without-kerberos --without-kerberos5 --with-lcms2 --with-xml2
 --without-mailutils --without-selinux --without-sqlite3 --with-gnutls
 --without-libsystemd --with-threads --without-tree-sitter
 --without-wide-int --with-sound=alsa --with-zlib --with-x
 --without-pgtk --without-ns --without-gconf --with-gsettings
 --without-toolkit-scroll-bars --with-xpm --with-xft --with-cairo
 --with-harfbuzz --with-libotf --with-m17n-flt --with-x-toolkit=lucid
 --with-xaw3d --with-gif --with-jpeg --with-png --with-rsvg --with-tiff
 --without-webp --with-imagemagick --with-dumping=pdumper
 'CFLAGS=-march=native -ggdb -O2 -pipe -fno-fast-math -ffp-contract=off'
 CPPFLAGS= 'LDFLAGS=-Wl,-O1 -Wl,--as-needed
 -Wl,-z,pack-relative-relocs''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ
IMAGEMAGICK JPEG LCMS2 LIBOTF LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF X11 XAW3D XDBE XIM XINPUT2
XPM LUCID ZLIB

Important settings:
  value of $LC_CTYPE: en_GB.UTF-8
  value of $LC_TIME: en_GB.UTF-8
  value of $LANG: POSIX
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils cl-extra help-mode
parse-time iso8601 time-date subr-x cl-loaddefs cl-lib rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/x-win x-win term/common-win x-dnd
touch-screen tool-bar dnd fontset image regexp-opt fringe tabulated-list
replace newcomment text-mode lisp-mode prog-mode register page tab-bar
menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse
jit-lock font-lock syntax font-core term/tty-colors frame minibuffer
nadvice seq simple cl-generic indonesian philippine cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop
case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo x-toolkit xinput2 x multi-tty move-toolbar
make-network-process emacs)

Memory information:
((conses 16 42539 9548) (symbols 48 5561 0) (strings 32 14122 3238)
 (string-bytes 1 338179) (vectors 16 9658)
 (vector-slots 8 113557 9685) (floats 8 28 24) (intervals 56 407 8)
 (buffers 984 13))





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-11  8:57 bug#72570: 31.0.50; Regression in date-to-time Ulrich Mueller
@ 2024-08-12  7:27 ` Ulrich Mueller
  2024-08-12  8:18   ` Ulrich Mueller
  2024-08-12 11:42 ` Eli Zaretskii
  1 sibling, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-12  7:27 UTC (permalink / raw)
  To: 72570

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

Attached patch will make date-to-time work as documented.

Alternatively, maybe it would be more consistent with other time
functions in Emacs if date-to-time would use local time as the default
(i.e. what is assumed by the current unit test in time-date-tests.el).

However, the fallback code uses Universal Time, and I don't see an easy
way to modify it for local time. The problem is that
timezone-make-date-arpa-standard would need an explicit timezone as its
second argument, which we don't know. We cannot use (current-time-zone)
there, because it's the timezone of the current time, not the one of the
to-be-converted timestamp. (For example, in the Europe/Berlin timezone,
(current-time-zone) returns (7200 "CEST"), while for conversion of
"2024-01-01 00:00:00" one would need (3600 "CET")).

(BTW, the code in timezone.el is very brittle. For example,
(timezone-make-date-arpa-standard "2024-01-01") signals a
wrong-type-argument error from 'capitalize'.)


[-- Attachment #2: 0001-Make-date-to-time-without-timezone-work-as-documente.patch --]
[-- Type: text/plain, Size: 2301 bytes --]

From 6c55891b5850c923196187b21b73f52d474c325a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Mon, 12 Aug 2024 07:30:01 +0200
Subject: [PATCH] Make date-to-time without timezone work as documented

* lisp/calendar/time-date.el (date-to-time): Assume Universal Time
rather than local time when no timezone is specified.  (Bug#72570)
* test/lisp/calendar/time-date-tests.el (test-date-to-time):
Update test.
---
 lisp/calendar/time-date.el            | 5 +++--
 test/lisp/calendar/time-date-tests.el | 8 ++++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/lisp/calendar/time-date.el b/lisp/calendar/time-date.el
index eca80f1e8b6..2f3ef0d8b4c 100644
--- a/lisp/calendar/time-date.el
+++ b/lisp/calendar/time-date.el
@@ -156,7 +156,7 @@ date-to-time
   (condition-case err
       (let ((parsed (parse-time-string date)))
 	(when (decoded-time-year parsed)
-	  (decoded-time-set-defaults parsed))
+	  (decoded-time-set-defaults parsed t))
 	(encode-time parsed))
     (error
      (let ((overflow-error '(error "Specified time is not representable")))
@@ -164,7 +164,8 @@ date-to-time
 	   (signal (car err) (cdr err))
 	 (condition-case err
 	     (encode-time (parse-time-string
-			   (timezone-make-date-arpa-standard date)))
+			   (timezone-make-date-arpa-standard
+			    date nil '(0 "UTC"))))
 	   (error
 	    (if (equal err overflow-error)
 		(signal (car err) (cdr err))
diff --git a/test/lisp/calendar/time-date-tests.el b/test/lisp/calendar/time-date-tests.el
index 6512dd0bd07..d7d39f050b7 100644
--- a/test/lisp/calendar/time-date-tests.el
+++ b/test/lisp/calendar/time-date-tests.el
@@ -42,8 +42,12 @@ test-obsolete-encode-time-value
                  '(1 2 3 4))))
 
 (ert-deftest test-date-to-time ()
-  (should (equal (format-time-string "%F %T" (date-to-time "2021-12-04"))
-                 "2021-12-04 00:00:00")))
+  (should (equal (format-time-string
+                  "%F %T" (date-to-time "2021-12-04") t)
+                 "2021-12-04 00:00:00"))
+  (should (equal (format-time-string
+                  "%F %T" (date-to-time "2021-12-04 00:00:00 +0100") t)
+                 "2021-12-03 23:00:00")))
 
 (ert-deftest test-days-between ()
   (should (equal (days-between "2021-10-22" "2020-09-29") 388)))
-- 
2.45.2


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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12  7:27 ` Ulrich Mueller
@ 2024-08-12  8:18   ` Ulrich Mueller
  0 siblings, 0 replies; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-12  8:18 UTC (permalink / raw)
  To: 72570

>>>>> On Mon, 12 Aug 2024, Ulrich Mueller wrote:

> Alternatively, maybe it would be more consistent with other time
> functions in Emacs if date-to-time would use local time as the default

Thinking about it, parsing a time string with local time is ill-defined.
For example, in the Europe/Berlin timezone, the timestamp "2024-03-31
02:00:00" didn't exist (because 01:59:59 was followed by 03:00:00), and
"2024-10-27 02:00:00" has two solutions.

So I guess Universal Time is more reasonable as the default.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-11  8:57 bug#72570: 31.0.50; Regression in date-to-time Ulrich Mueller
  2024-08-12  7:27 ` Ulrich Mueller
@ 2024-08-12 11:42 ` Eli Zaretskii
  2024-08-12 12:51   ` Ulrich Mueller
  2024-08-12 22:03   ` Paul Eggert
  1 sibling, 2 replies; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-12 11:42 UTC (permalink / raw)
  To: Ulrich Mueller, Paul Eggert; +Cc: 72570

> From: Ulrich Mueller <ulm@gentoo.org>
> Date: Sun, 11 Aug 2024 10:57:57 +0200
> 
> Function documentation of 'date-to-time' says:
> "If DATE lacks timezone information, GMT is assumed."
> 
> Also, the Lispref Manual:
> "This function assumes Universal Time if STRING lacks explicit time zone
> information, ..."
> 
>   (format-time-string
>    "%Y-%m-%d %H:%M:%S" (date-to-time "2024-01-01 00:00:00") t)
> 
> returns "2023-12-31 23:00:00" as its result, while I would expect
> "2024-01-01 00:00:00" with both conversions being in the same (UTC)
> timezone. Apparently, date-to-time incorrectly uses the local timezone
> (Europe/Berlin) instead of UTC.
> 
> In Emacs 23.4 the function worked as expected.
> 
> Git bisecting points to this commit:
> 
>   commit b069e5a697f37a06704136a8d5376b4d088658c8 (HEAD)
>   Author: Gnus developers <ding@gnus.org>
>   Date:   Thu Sep 23 00:30:37 2010 +0000
> 
>       Merge Changes made in Gnus trunk.
> 
>       [...]
>       time-date.el (date-to-time): Speed up date-to-time.
>       [...]

That commit made this change in date-to-time:

  -      (apply 'encode-time
  -            (parse-time-string
  -             ;; `parse-time-string' isn't sufficiently general or
  -             ;; robust.  It fails to grok some of the formats that
  -             ;; timezone does (e.g. dodgy post-2000 stuff from some
  -             ;; Elms) and either fails or returns bogus values.  Lars
  -             ;; reverted this change, but that loses non-trivially
  -             ;; often for me.  -- fx
  -             (timezone-make-date-arpa-standard date)))
  -    (error (error "Invalid date: %s" date))))
  +      (apply 'encode-time (parse-time-string date))
  +    (error (condition-case ()
  +              (apply 'encode-time
  +                     (parse-time-string
  +                      (timezone-make-date-arpa-standard date)))
  +            (error (error "Invalid date: %s" date))))))

IOW, it applied parse-time-string to the argument DATE instead of
first running it through timezone-make-date-arpa-standard.

But that is not necessarily incorrect, because several time-conversion
functions, including those involved in this issue, don't document
their assumptions about time-zone when it is omitted.  This includes
parse-time-string and iso8601-parse.  Since these functions don't
document this aspect, and the code involved in the above snippet is
quite convoluted, it is hard for me to decide which part of what
function needs to be fixed.

Paul, any suggestions?  And can we please document the missing
time-zone assumptions?





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12 11:42 ` Eli Zaretskii
@ 2024-08-12 12:51   ` Ulrich Mueller
  2024-08-12 13:26     ` Eli Zaretskii
  2024-08-12 22:03   ` Paul Eggert
  1 sibling, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-12 12:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 72570, Paul Eggert

>>>>> On Mon, 12 Aug 2024, Eli Zaretskii wrote:

> That commit made this change in date-to-time:

>   -      (apply 'encode-time
>   -            (parse-time-string
>   -             ;; `parse-time-string' isn't sufficiently general or
>   -             ;; robust.  It fails to grok some of the formats that
>   -             ;; timezone does (e.g. dodgy post-2000 stuff from some
>   -             ;; Elms) and either fails or returns bogus values.  Lars
>   -             ;; reverted this change, but that loses non-trivially
>   -             ;; often for me.  -- fx
>   -             (timezone-make-date-arpa-standard date)))
>   -    (error (error "Invalid date: %s" date))))
>   +      (apply 'encode-time (parse-time-string date))
>   +    (error (condition-case ()
>   +              (apply 'encode-time
>   +                     (parse-time-string
>   +                      (timezone-make-date-arpa-standard date)))
>   +            (error (error "Invalid date: %s" date))))))

> IOW, it applied parse-time-string to the argument DATE instead of
> first running it through timezone-make-date-arpa-standard.

Which is still present as a fallback, if parse-time-string either errors
out, or doesn't return a useful date.

One problem is that the initial parse-time-string code defaults to
local time (contrary to what is documented), while the
timezone-make-date-arpa-standard fallback code defaults to UTC.

> But that is not necessarily incorrect, because several time-conversion
> functions, including those involved in this issue, don't document
> their assumptions about time-zone when it is omitted.  This includes
> parse-time-string and iso8601-parse.  Since these functions don't
> document this aspect, and the code involved in the above snippet is
> quite convoluted, it is hard for me to decide which part of what
> function needs to be fixed.

Well, parse-time-string doesn't make any assumptions about the timezone.
It will return a list with nil in its TZ element, if it cannot extract
any timezone info from the string.

Same for iso8601-parse, the returned TZ is nil if it is not present in
the argument string. Examples:

(iso8601-parse "2024-08-12T00:00:00")      -> (0 0 0 12 8 2024 nil -1 nil)
(iso8601-parse "2024-08-12T00:00:00Z")     -> (0 0 0 12 8 2024 nil nil 0)
(iso8601-parse "2024-08-12T00:00:00+0200") -> (0 0 0 12 8 2024 nil -1 7200)





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12 12:51   ` Ulrich Mueller
@ 2024-08-12 13:26     ` Eli Zaretskii
  2024-08-12 13:48       ` Ulrich Mueller
  0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-12 13:26 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: 72570, eggert

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  72570@debbugs.gnu.org
> Date: Mon, 12 Aug 2024 14:51:02 +0200
> 
> > But that is not necessarily incorrect, because several time-conversion
> > functions, including those involved in this issue, don't document
> > their assumptions about time-zone when it is omitted.  This includes
> > parse-time-string and iso8601-parse.  Since these functions don't
> > document this aspect, and the code involved in the above snippet is
> > quite convoluted, it is hard for me to decide which part of what
> > function needs to be fixed.
> 
> Well, parse-time-string doesn't make any assumptions about the timezone.
> It will return a list with nil in its TZ element, if it cannot extract
> any timezone info from the string.
> 
> Same for iso8601-parse, the returned TZ is nil if it is not present in
> the argument string.

Yes, but what does nil as timezone mean, when you later interpret the
time values?  Is it UTC or is it local time?





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12 13:26     ` Eli Zaretskii
@ 2024-08-12 13:48       ` Ulrich Mueller
  2024-08-12 14:30         ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-12 13:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 72570, eggert

>>>>> On Mon, 12 Aug 2024, Eli Zaretskii wrote:

>> Well, parse-time-string doesn't make any assumptions about the timezone.
>> It will return a list with nil in its TZ element, if it cannot extract
>> any timezone info from the string.
>> 
>> Same for iso8601-parse, the returned TZ is nil if it is not present in
>> the argument string.

> Yes, but what does nil as timezone mean, when you later interpret the
> time values?  Is it UTC or is it local time?

AFAICS it has its usual meaning (same as in ZONE of encode-time), i.e.
nil for local time. However, this could be easily overridden by calling
(decoded-time-set-defaults parsed t) in the following line. See the
patch attached to my earlier message.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12 13:48       ` Ulrich Mueller
@ 2024-08-12 14:30         ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-12 14:30 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: 72570, eggert

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: eggert@cs.ucla.edu,  72570@debbugs.gnu.org
> Date: Mon, 12 Aug 2024 15:48:04 +0200
> 
> >>>>> On Mon, 12 Aug 2024, Eli Zaretskii wrote:
> 
> >> Well, parse-time-string doesn't make any assumptions about the timezone.
> >> It will return a list with nil in its TZ element, if it cannot extract
> >> any timezone info from the string.
> >> 
> >> Same for iso8601-parse, the returned TZ is nil if it is not present in
> >> the argument string.
> 
> > Yes, but what does nil as timezone mean, when you later interpret the
> > time values?  Is it UTC or is it local time?
> 
> AFAICS it has its usual meaning (same as in ZONE of encode-time), i.e.
> nil for local time.

If this is true in all use cases (and I'm not sure, as we just
discovered in this case), it should be documented, and we should have
tests in our test suite to make sure we never regress in this aspect.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12 11:42 ` Eli Zaretskii
  2024-08-12 12:51   ` Ulrich Mueller
@ 2024-08-12 22:03   ` Paul Eggert
  2024-08-13  5:55     ` Ulrich Mueller
  2024-08-13 11:15     ` Eli Zaretskii
  1 sibling, 2 replies; 26+ messages in thread
From: Paul Eggert @ 2024-08-12 22:03 UTC (permalink / raw)
  To: Eli Zaretskii, Ulrich Mueller; +Cc: 72570

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

On 2024-08-12 04:42, Eli Zaretskii wrote:
> Paul, any suggestions?  And can we please document the missing
> time-zone assumptions?

Not sure what documentation is missing, but I went through the doc for 
parse-time-string and iso8601-parse and found some room for improvement 
and so installed the attached.

As for the original issue, I tend to agree with Ulrich that date-to-time 
should default to local time. This would need changes to both 
documentation and to code. Ulrich mentioned the following potential 
issue with this idea:

> The problem is that
> timezone-make-date-arpa-standard would need an explicit timezone as its
> second argument, which we don't know.

The code could use timezone-time-zone-from-absolute to infer a timezone. 
timezone-fix-time already does this sort of thing. Admittedly getting 
the details right could be a bit tricky, and there is no perfect 
solution in this area (certainly timezone-fix-time is flawed). It might 
be good enough, though.

[-- Attachment #2: 0001-Document-time-parsing-functions-a-bit-better.patch --]
[-- Type: text/x-patch, Size: 4160 bytes --]

From 7a828c938ca9daf37baa02a50bb6463e2b7c0b85 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 12 Aug 2024 14:31:19 -0700
Subject: [PATCH] Document time-parsing functions a bit better

See <https://bugs.gnu.org/72570#14>
---
 doc/lispref/os.texi         | 26 +++++++++++++++++++-------
 lisp/calendar/iso8601.el    | 10 +++++-----
 lisp/calendar/parse-time.el |  5 ++---
 3 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index 3ba3da459bf..5839de4a650 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -1800,19 +1800,31 @@ Time Parsing
 resemble an RFC 822 (or later) or ISO 8601 string, like ``Fri, 25 Mar
 2016 16:24:56 +0100'' or ``1998-09-12T12:21:54-0200'', but this
 function will attempt to parse less well-formed time strings as well.
+It parses an incomplete string like ``2024-08-13'' to an incomplete
+structure like @samp{(nil nil nil 13 8 2024 nil -1 nil)}, in which
+an unknown DST value is @minus{}1 and other unknown values are @code{nil}.
 @end defun
 
 @vindex ISO 8601 date/time strings
 @defun iso8601-parse string
-For a more strict function (that will error out upon invalid input),
-this function can be used instead.  It can parse all variants of
+This function acts like @code{parse-time-string} except it is stricter
+and errors out upon invalid input.  It can parse all variants of
 the ISO 8601 standard, so in addition to the formats mentioned above,
 it also parses things like ``1998W45-3'' (week number) and
-``1998-245'' (ordinal day number).  To parse durations, there's
-@code{iso8601-parse-duration}, and to parse intervals, there's
-@code{iso8601-parse-interval}.  All these functions return decoded
-time structures, except the final one, which returns three of them
-(the start, the end, and the duration).
+``1998-245'' (ordinal day number).
+@end defun
+
+@defun iso8601-parse-duration string
+This function parses an ISO 8601 time duration @var{string}
+and returns a decoded time structure.
+@c FIXME: example? behavior on incomplete input?
+@end defun
+
+@defun iso8601-parse-interval string
+This function parses an ISO 8601 time interval @var{string}
+and returns three decoded time structures
+representing the start, the end, and the duration.
+@c FIXME: example? behavior on incomplete input?
 @end defun
 
 @defun format-time-string format-string &optional time zone
diff --git a/lisp/calendar/iso8601.el b/lisp/calendar/iso8601.el
index a32b52564c9..a31b60eaec2 100644
--- a/lisp/calendar/iso8601.el
+++ b/lisp/calendar/iso8601.el
@@ -121,11 +121,11 @@ iso8601--zone-dst
 
 (defun iso8601-parse (string &optional form)
   "Parse an ISO 8601 date/time string and return a `decode-time' structure.
-
-The ISO 8601 date/time strings look like \"2008-03-02T13:47:30\",
-but shorter, incomplete strings like \"2008-03-02\" are valid, as
-well as variants like \"2008W32\" (week number) and
-\"2008-234\" (ordinal day number).
+ISO 8601 date/time strings look like \"2008-03-02T13:47:30+05:30\",
+or like shorter, incomplete strings like date \"2008-03-02\",
+week number \"2008W32\", and ordinal day number \"2008-234\".
+Values returned are identical to those of `decode-time', except
+that an unknown DST value is -1 and other unknown values are nil.
 
 See `decode-time' for the meaning of FORM."
   (if (not (iso8601-valid-p string))
diff --git a/lisp/calendar/parse-time.el b/lisp/calendar/parse-time.el
index c34329a4002..9538ea92ee5 100644
--- a/lisp/calendar/parse-time.el
+++ b/lisp/calendar/parse-time.el
@@ -154,9 +154,8 @@ parse-time-string
 \"Wed, 15 Jan 2020 16:12:21 -0800\".  This function is
 somewhat liberal in what format it accepts, and will attempt to
 return a \"likely\" value even for somewhat malformed strings.
-The values returned are identical to those of `decode-time', but
-any unknown values other than DST are returned as nil, and an
-unknown DST value is returned as -1.
+Values returned are identical to those of `decode-time', except
+that an unknown DST value is -1 and other unknown values are nil.
 
 See `decode-time' for the meaning of FORM."
   (condition-case ()
-- 
2.43.0


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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12 22:03   ` Paul Eggert
@ 2024-08-13  5:55     ` Ulrich Mueller
  2024-08-13 19:39       ` Paul Eggert
  2024-08-13 11:15     ` Eli Zaretskii
  1 sibling, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-13  5:55 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Eli Zaretskii, 72570

>>>>> On Tue, 13 Aug 2024, Paul Eggert wrote:

> As for the original issue, I tend to agree with Ulrich that
> date-to-time should default to local time. This would need changes to
> both documentation and to code. Ulrich mentioned the following
> potential issue with this idea:

>> The problem is that timezone-make-date-arpa-standard would need an
>> explicit timezone as its second argument, which we don't know.

> The code could use timezone-time-zone-from-absolute to infer a
> timezone. timezone-fix-time already does this sort of thing.
> Admittedly getting the details right could be a bit tricky, and there
> is no perfect solution in this area (certainly timezone-fix-time is
> flawed). It might be good enough, though.

IIUC timezone-time-zone-from-absolute expects the SECONDS argument
to be seconds since midnight in UTC (again, not documented). In the
Europe/Berlin timezone (DST starts at 02:00 local time), I get this:

  (timezone-time-zone-from-absolute
   (timezone-absolute-from-gregorian 3 31 2024)
   (* 1 60 60))

  (7200 "CEST")

So don't we have a chicken-and-egg problem? We'd need the UTC timestamp
to infer the timezone, but the timezone to calculate the timestamp?





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-12 22:03   ` Paul Eggert
  2024-08-13  5:55     ` Ulrich Mueller
@ 2024-08-13 11:15     ` Eli Zaretskii
  2024-08-13 15:09       ` Ulrich Mueller
  2024-08-13 19:59       ` Paul Eggert
  1 sibling, 2 replies; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-13 11:15 UTC (permalink / raw)
  To: Paul Eggert; +Cc: ulm, 72570

> Date: Mon, 12 Aug 2024 15:03:46 -0700
> Cc: 72570@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> On 2024-08-12 04:42, Eli Zaretskii wrote:
> > Paul, any suggestions?  And can we please document the missing
> > time-zone assumptions?
> 
> Not sure what documentation is missing, but I went through the doc for 
> parse-time-string and iso8601-parse and found some room for improvement 
> and so installed the attached.

Thanks, but that doesn't document what I found missing: the
interpretation of the missing time-zone information.  Maybe you
consider it unnecessary to mention these defaults in the doc strings
of parse-time-string and iso8601-parse, but it is something I kept
asking myself when I tried to understand what does date-to-time do
when called without time-zone argument.  At the very least we should
say that without TZ information, these functions return nil as the
time zone/UTC offset, and that the interpretation of that nil is up to
the APIs that the program will call with these results.  Also, it
would be good to document what will these functions do when the TZ
information _is_ supplied: will they still just parse the string into
numbers, or will they convert the date/time to UTC?

> As for the original issue, I tend to agree with Ulrich that date-to-time 
> should default to local time. This would need changes to both 
> documentation and to code. Ulrich mentioned the following potential 
> issue with this idea:
> 
> > The problem is that
> > timezone-make-date-arpa-standard would need an explicit timezone as its
> > second argument, which we don't know.
> 
> The code could use timezone-time-zone-from-absolute to infer a timezone. 
> timezone-fix-time already does this sort of thing. Admittedly getting 
> the details right could be a bit tricky, and there is no perfect 
> solution in this area (certainly timezone-fix-time is flawed). It might 
> be good enough, though.

The change which affected behavior that triggered this discussion was
done because evidently timezone-time-zone-from-absolute was deemed to
be too slow, and therefore the authors of the change wanted to avoid
it.  I'm okay with making the result of that change the de-facto
default, but then (a) we should document this change in NEWS including
the (trivial) way of getting back old behavior, and (b) make sure that
the new behavior happens even if the body of this condition-case in
date-to-time:

  (condition-case err
      (let ((parsed (parse-time-string date)))
	(when (decoded-time-year parsed)
	  (decoded-time-set-defaults parsed))
	(encode-time parsed))

does not signal an error.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 11:15     ` Eli Zaretskii
@ 2024-08-13 15:09       ` Ulrich Mueller
  2024-08-13 15:36         ` Eli Zaretskii
  2024-08-13 19:59       ` Paul Eggert
  1 sibling, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-13 15:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 72570, Paul Eggert

>>>>> On Tue, 13 Aug 2024, Eli Zaretskii wrote:

> The change which affected behavior that triggered this discussion was
> done because evidently timezone-time-zone-from-absolute was deemed to
> be too slow, and therefore the authors of the change wanted to avoid
> it.  I'm okay with making the result of that change the de-facto
> default, but then (a) we should document this change in NEWS including
> the (trivial) way of getting back old behavior,

But what is the old behaviour? The one that is documented, or the actual
behaviour of the code?

  (format-time-string "%F %T" (date-to-time "2024-08-13 00:00:00"))
  "2024-08-13 00:00:00"

  (format-time-string "%F %T" (date-to-time "2024-8-13 00:00:00"))
  "2024-08-13 02:00:00"

In the first case, parse-time-string (inside the first condition-case)
succeeds and it uses local time. In the second case with a slightly
malformed date, it takes the detour via timezone-make-date-arpa-standard
and uses UTC.

Also:

  (format-time-string "%F %T" (date-to-time "2024/08/13 00:00:00"))
  "2024-01-08 00:00:00"

The GIGO principle at its finest. :) However, I'd much prefer if the
function would check validity of the input and error out, rather than
returning a nonsensical result. But maybe that's another issue and
outside the scope of this bug report.

> and (b) make sure that
> the new behavior happens even if the body of this condition-case in
> date-to-time:

>   (condition-case err
>       (let ((parsed (parse-time-string date)))
> 	(when (decoded-time-year parsed)
> 	  (decoded-time-set-defaults parsed))
> 	(encode-time parsed))

> does not signal an error.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 15:09       ` Ulrich Mueller
@ 2024-08-13 15:36         ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-13 15:36 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: 72570, eggert

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  72570@debbugs.gnu.org
> Date: Tue, 13 Aug 2024 17:09:14 +0200
> 
> >>>>> On Tue, 13 Aug 2024, Eli Zaretskii wrote:
> 
> > The change which affected behavior that triggered this discussion was
> > done because evidently timezone-time-zone-from-absolute was deemed to
> > be too slow, and therefore the authors of the change wanted to avoid
> > it.  I'm okay with making the result of that change the de-facto
> > default, but then (a) we should document this change in NEWS including
> > the (trivial) way of getting back old behavior,
> 
> But what is the old behaviour? The one that is documented, or the actual
> behaviour of the code?

The former, IMO.  It was also the actual behavior until that commit
you found.

>   (format-time-string "%F %T" (date-to-time "2024-08-13 00:00:00"))
>   "2024-08-13 00:00:00"
> 
>   (format-time-string "%F %T" (date-to-time "2024-8-13 00:00:00"))
>   "2024-08-13 02:00:00"
> 
> In the first case, parse-time-string (inside the first condition-case)
> succeeds and it uses local time. In the second case with a slightly
> malformed date, it takes the detour via timezone-make-date-arpa-standard
> and uses UTC.

That's my point (b), I believe:

> > and (b) make sure that
> > the new behavior happens even if the body of this condition-case in
> > date-to-time:
> 
> >   (condition-case err
> >       (let ((parsed (parse-time-string date)))
> > 	(when (decoded-time-year parsed)
> > 	  (decoded-time-set-defaults parsed))
> > 	(encode-time parsed))
> 
> > does not signal an error.

> Also:
> 
>   (format-time-string "%F %T" (date-to-time "2024/08/13 00:00:00"))
>   "2024-01-08 00:00:00"
> 
> The GIGO principle at its finest. :) However, I'd much prefer if the
> function would check validity of the input and error out, rather than
> returning a nonsensical result. But maybe that's another issue and
> outside the scope of this bug report.

I indeed think it's a separate issue.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13  5:55     ` Ulrich Mueller
@ 2024-08-13 19:39       ` Paul Eggert
  2024-08-13 21:11         ` Ulrich Mueller
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Eggert @ 2024-08-13 19:39 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: Eli Zaretskii, 72570

On 2024-08-12 22:55, Ulrich Mueller wrote:

> don't we have a chicken-and-egg problem? We'd need the UTC timestamp
> to infer the timezone, but the timezone to calculate the timestamp?

You're quite right that we have a chicken and egg problem, and that no 
solution will be perfect here. My point was that using a heuristic would 
be better than nothing, if the goal is to infer timezone from incomplete 
input. That's why timezone-fix-time uses this heuristic.

I continue to think that your idea of changing date-to-time to default 
to the local time zone is the best way to move forward. How about this 
idea for doing it?

* Change the doc to say this. This is a simple change, and it reflects 
existing behavior better.

* Change date-to-time so that if it falls back on 
timezone-make-date-arpa standard, it insists that the string denotes a 
time zone; if the string lacks a time zone it errors out with "Invalid 
date", which it already does with invalid dates.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 11:15     ` Eli Zaretskii
  2024-08-13 15:09       ` Ulrich Mueller
@ 2024-08-13 19:59       ` Paul Eggert
  2024-08-14  8:37         ` Eli Zaretskii
  1 sibling, 1 reply; 26+ messages in thread
From: Paul Eggert @ 2024-08-13 19:59 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: ulm, 72570

On 2024-08-13 04:15, Eli Zaretskii wrote:
 > I'm okay with making the result of that change the de-facto
 > default, but then (a) we should document this change in NEWS including
 > the (trivial) way of getting back old behavior, and (b) make sure that
 > the new behavior happens even if the body of this condition-case in
 > date-to-time:
 >
 >    (condition-case err
 >        (let ((parsed (parse-time-string date)))
 >          (when (decoded-time-year parsed)
 >            (decoded-time-set-defaults parsed))
 >          (encode-time parsed))
 >
 > does not signal an error.

That all sounds good, and I suggested something similar to Ulrich in the 
email I sent a few minutes ago.


> Thanks, but that doesn't document what I found missing: the
> interpretation of the missing time-zone information.  Maybe you
> consider it unnecessary to mention these defaults in the doc strings
> of parse-time-string and iso8601-parse, but it is something I kept
> asking myself when I tried to understand what does date-to-time do
> when called without time-zone argument.

It's not parse-time-string's job to interpret missing time zone info, so 
I'm not sure what its doc string should say about this, other than "nil 
represents missing info" (something the doc already says).

What the abovementioned date-to-time code says is "Parse DATE. If you 
find a year, use reasonable defaults for other components. Then try to 
encode the result. This attempt to encode will fail if the year is 
missing, because encode-time fails when crucial time components are 
unknown. This heuristic defaults to local time when the string lacks 
time zone information because encode-time treats a nil time zone as 
local time."

Would it help to add a comment to that effect, to the date-to-time code?


> At the very least we should
> say that without TZ information, these functions return nil as the
> time zone/UTC offset

The doc already says that ...


> and that the interpretation of that nil is up to
> the APIs that the program will call with these results.

... and feel free to add words to that effect, if you think it's still 
needed given the above comments.

This issue isn't just about time zones, though: it's about every 
component. For example, the interpretation of a nil year is up to the 
caller (as shown in the code you quoted).


> Also, it
> would be good to document what will these functions do when the TZ
> information _is_ supplied: will they still just parse the string into
> numbers, or will they convert the date/time to UTC?

They don't convert. They just parse. Feel free to add more doc along 
those lines.






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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 19:39       ` Paul Eggert
@ 2024-08-13 21:11         ` Ulrich Mueller
  2024-08-13 21:19           ` Paul Eggert
  0 siblings, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-13 21:11 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Eli Zaretskii, 72570

>>>>> On Tue, 13 Aug 2024, Paul Eggert wrote:

> You're quite right that we have a chicken and egg problem, and that no
> solution will be perfect here. My point was that using a heuristic
> would be better than nothing, if the goal is to infer timezone from
> incomplete input. That's why timezone-fix-time uses this heuristic.

> I continue to think that your idea of changing date-to-time to default
> to the local time zone is the best way to move forward. How about this
> idea for doing it?

> * Change the doc to say this. This is a simple change, and it reflects
>   existing behavior better.

> * Change date-to-time so that if it falls back on
>   timezone-make-date-arpa standard, it insists that the string denotes
>   a time zone; if the string lacks a time zone it errors out with
>   "Invalid date", which it already does with invalid dates.

Sounds good.

(Assuming that the fallback code is still needed today.
From commit history in the Gnus repository, it looks like the call to
timezone-make-date-arpa-standard was first added on 2000-01-06 in
https://github.com/dabrahams/gnus/commit/f1c5b21410f6e574913bbcf97ffd5e98d9ab89ac#diff-c8e4386c2f71d2573b4b5e6549e54c51cdf1fa649c6578ebd377f84e27576c93L32 .
The rationale for the change was to make the function "... more robust,
e.g. against the crop of year 100 dates in Jan 2000".)





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 21:11         ` Ulrich Mueller
@ 2024-08-13 21:19           ` Paul Eggert
  2024-08-14  8:12             ` Ulrich Mueller
  2024-08-14 14:09             ` Ulrich Mueller
  0 siblings, 2 replies; 26+ messages in thread
From: Paul Eggert @ 2024-08-13 21:19 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: Eli Zaretskii, 72570

On 2024-08-13 14:11, Ulrich Mueller wrote:
> (Assuming that the fallback code is still needed today.
>  From commit history in the Gnus repository, it looks like the call to
> timezone-make-date-arpa-standard was first added on 2000-01-06 in
> https://github.com/dabrahams/gnus/commit/ 
> f1c5b21410f6e574913bbcf97ffd5e98d9ab89ac#diff- 
> c8e4386c2f71d2573b4b5e6549e54c51cdf1fa649c6578ebd377f84e27576c93L32 .
> The rationale for the change was to make the function "... more robust,
> e.g. against the crop of year 100 dates in Jan 2000".)

If that's the only reason it's there, we can simply remove the call to 
timezone-make-date-arpa-standard. That would be better to coating the 
pig with even more lipstick.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 21:19           ` Paul Eggert
@ 2024-08-14  8:12             ` Ulrich Mueller
  2024-08-14 14:09             ` Ulrich Mueller
  1 sibling, 0 replies; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-14  8:12 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Eli Zaretskii, 72570

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

>>>>> On Tue, 13 Aug 2024, Paul Eggert wrote:

> On 2024-08-13 14:11, Ulrich Mueller wrote:
>> (Assuming that the fallback code is still needed today.
>> From commit history in the Gnus repository, it looks like the call to
>> timezone-make-date-arpa-standard was first added on 2000-01-06 in
>> https://github.com/dabrahams/gnus/commit/f1c5b21410f6e574913bbcf97ffd5e98d9ab89ac#diff-c8e4386c2f71d2573b4b5e6549e54c51cdf1fa649c6578ebd377f84e27576c93L32 .
>> The rationale for the change was to make the function "... more robust,
>> e.g. against the crop of year 100 dates in Jan 2000".)

> If that's the only reason it's there, we can simply remove the call to
> timezone-make-date-arpa-standard. That would be better to coating the
> pig with even more lipstick.

IIUC timezone-parse-date is the function that is used for parsing a date
in the fallback code. Its function documentation says:

| Understands the following styles:
|  (1) 14 Apr 89 03:20[:12] [GMT]
|  (2) Fri, 17 Mar 89 4:01[:33] [GMT]
|  (3) Mon Jan 16 16:12[:37] [GMT] 1989
|  (4) 6 May 1992 1641-JST (Wednesday)
|  (5) 22-AUG-1993 10:59:12.82
|  (6) Thu, 11 Apr 16:17:12 91 [MET]
|  (7) Mon, 6  Jul 16:47:20 T 1992 [MET]
|  (8) 1996-06-24 21:13:12 [GMT]
|  (9) 1996-06-24 21:13-ZONE
|  (10) 19960624T211312

I've used these to make some test cases for ERT, see the attached file.
The current date-to-time correctly handles styles 1, 2, 3, 6 (without
timezone), 7 (without timezone), 8, and 10. It returns an incorrect
result for styles 4, 5, 6 (with timezone), 7 (with timezone), and 9.

If I redefine date-to-time with the fallback code removed, the test
results are identical (somewhat surprisingly).

So I'd conclude that we don't gain much from the fallback code.


[-- Attachment #2: test-date-to-time.el --]
[-- Type: text/plain, Size: 2331 bytes --]

(ert-deftest test-date-to-time ()
  ;; Test cases are the formats mentioned in the docstring
  ;; of timezone-parse-date
  (let ((date-list
	 '(("14 Apr 89 03:20"              (00 20 03 14 04 1989 nil -1 nil))
	   ("14 Apr 89 03:20:12"           (12 20 03 14 04 1989 nil -1 nil))
	   ("14 Apr 89 03:20 GMT"          (00 20 03 14 04 1989 nil nil 0))
	   ("14 Apr 89 03:20:12 GMT"       (12 20 03 14 04 1989 nil nil 0))
	   ("Fri, 17 Mar 89 4:01"          (00 01 04 17 03 1989 nil -1 nil))
	   ("Fri, 17 Mar 89 4:01:33"       (33 01 04 17 03 1989 nil -1 nil))
	   ("Fri, 17 Mar 89 4:01 GMT"      (00 01 04 17 03 1989 nil nil 0))
	   ("Fri, 17 Mar 89 4:01:33 GMT"   (33 01 04 17 03 1989 nil nil 0))
	   ("Mon Jan 16 16:12 1989"        (00 12 16 16 01 1989 nil -1 nil))
	   ("Mon Jan 16 16:12:37 1989"     (37 12 16 16 01 1989 nil -1 nil))
	   ("Mon Jan 16 16:12 GMT 1989"    (00 12 16 16 01 1989 nil nil 0))
	   ("Mon Jan 16 16:12:37 GMT 1989" (37 12 16 16 01 1989 nil nil 0))
	   ("Thu, 11 Apr 16:17:12 91"      (12 17 16 11 04 1991 nil -1 nil))
	   ("Mon, 6  Jul 16:47:20 T 1992"  (20 47 16 06 07 1992 nil -1 nil))
	   ("1996-06-24 21:13:12"          (12 13 21 24 06 1996 nil -1 nil))
	   ("1996-06-24 21:13:12 GMT"      (12 13 21 24 06 1996 nil nil 0))
	   ("19960624t211312"              (12 13 21 24 06 1996 nil -1 nil))
	   ;; fails to parse time, returns 00:00:00
	   ;("6 May 1992 1641-JST (Wednesday)"
	   ;                                (00 41 16 06 05 1992 nil nil 32400))
	   ;; parses date incorrectly as 1982-01-01
	   ;("22-AUG-1993 10:59:12.82"      (12 59 10 22 08 1993 nil -1 nil))
	   ;; fails to parse timezone
	   ;("Thu, 11 Apr 16:17:12 91 MET"  (12 17 16 11 04 1991 nil nil 3600))
	   ;("Mon, 6  Jul 16:47:20 T 1992 MET"
	   ;                                (20 47 16 06 07 1992 nil nil 3600))
	   ;; fails to parse time, returns 00:00:00
	   ;("1996-06-24 21:13-JST"         (00 13 21 24 06 1996 nil nil 32400))
	   ))
	(process-environment (copy-sequence process-environment))
	(tz (getenv "TZ")))
    (unwind-protect
	(progn
	  (setenv "TZ" "UTC")
	  (dolist (date date-list)
	    (should (equal
		     (date-to-time (car date))
		     (encode-time (cadr date))))))
      ;; This is needed because setenv handles TZ specially.
      ;; So, restoring the environment is not enough.
      (setenv "TZ" tz))))

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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 19:59       ` Paul Eggert
@ 2024-08-14  8:37         ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-14  8:37 UTC (permalink / raw)
  To: Paul Eggert; +Cc: ulm, 72570

> Date: Tue, 13 Aug 2024 12:59:00 -0700
> Cc: ulm@gentoo.org, 72570@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> 
> > Thanks, but that doesn't document what I found missing: the
> > interpretation of the missing time-zone information.  Maybe you
> > consider it unnecessary to mention these defaults in the doc strings
> > of parse-time-string and iso8601-parse, but it is something I kept
> > asking myself when I tried to understand what does date-to-time do
> > when called without time-zone argument.
> 
> It's not parse-time-string's job to interpret missing time zone info, so 
> I'm not sure what its doc string should say about this, other than "nil 
> represents missing info" (something the doc already says).
> 
> What the abovementioned date-to-time code says is "Parse DATE. If you 
> find a year, use reasonable defaults for other components. Then try to 
> encode the result. This attempt to encode will fail if the year is 
> missing, because encode-time fails when crucial time components are 
> unknown. This heuristic defaults to local time when the string lacks 
> time zone information because encode-time treats a nil time zone as 
> local time."
> 
> Would it help to add a comment to that effect, to the date-to-time code?

Yes, I think so.

> > At the very least we should
> > say that without TZ information, these functions return nil as the
> > time zone/UTC offset
> 
> The doc already says that ...
> 
> 
> > and that the interpretation of that nil is up to
> > the APIs that the program will call with these results.
> 
> ... and feel free to add words to that effect, if you think it's still 
> needed given the above comments.
> 
> This issue isn't just about time zones, though: it's about every 
> component. For example, the interpretation of a nil year is up to the 
> caller (as shown in the code you quoted).
> 
> 
> > Also, it
> > would be good to document what will these functions do when the TZ
> > information _is_ supplied: will they still just parse the string into
> > numbers, or will they convert the date/time to UTC?
> 
> They don't convert. They just parse. Feel free to add more doc along 
> those lines.

Done.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-13 21:19           ` Paul Eggert
  2024-08-14  8:12             ` Ulrich Mueller
@ 2024-08-14 14:09             ` Ulrich Mueller
  2024-08-15  3:27               ` Paul Eggert
  1 sibling, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-14 14:09 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Eli Zaretskii, 72570

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

>>>>> On Tue, 13 Aug 2024, Paul Eggert wrote:

> If that's the only reason it's there, we can simply remove the call to
> timezone-make-date-arpa-standard. That would be better to coating the
> pig with even more lipstick.

The attached patch does the former, and also adds some test cases.


[-- Attachment #2: 0001-Drop-fallback-code-in-date-to-time-update-documentat.patch --]
[-- Type: text/plain, Size: 5113 bytes --]

From ccb2d7282cb792ae993b94821ae7bf1d79f4d348 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ulrich=20M=C3=BCller?= <ulm@gentoo.org>
Date: Wed, 14 Aug 2024 13:57:16 +0200
Subject: [PATCH] Drop fallback code in date-to-time, update documentation

* lisp/calendar/time-date.el (date-to-time): Drop fallback code.
Document that the default timezone is local time, rather than GMT.
* test/lisp/calendar/time-date-tests.el (test-date-to-time):
Add more test cases.
* doc/lispref/os.texi (Time Parsing): Document that 'date-to-time'
defaults to local time.  (Bug#72570)
---
 doc/lispref/os.texi                   |  4 ++--
 lisp/calendar/time-date.el            | 19 ++++---------------
 test/lisp/calendar/time-date-tests.el | 24 ++++++++++++++++++++++--
 3 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index 5839de4a650..ddaa1de221c 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -1788,8 +1788,8 @@ Time Parsing
 This function parses the time-string @var{string} and returns the
 corresponding Lisp timestamp.  The argument @var{string} should represent
 a date-time, and should be in one of the forms recognized by
-@code{parse-time-string} (see below).  This function assumes Universal
-Time if @var{string} lacks explicit time zone information,
+@code{parse-time-string} (see below).  This function assumes local time
+if @var{string} lacks explicit time zone information,
 and assumes earliest values if @var{string} lacks month, day, or time.
 The operating system limits the range of time and zone values.
 @end defun
diff --git a/lisp/calendar/time-date.el b/lisp/calendar/time-date.el
index eca80f1e8b6..717cb4a5c5b 100644
--- a/lisp/calendar/time-date.el
+++ b/lisp/calendar/time-date.el
@@ -145,30 +145,19 @@ encode-time-value
 (autoload 'timezone-make-date-arpa-standard "timezone")
 
 ;;;###autoload
-;; `parse-time-string' isn't sufficiently general or robust.  It fails
-;; to grok some of the formats that timezone does (e.g. dodgy
-;; post-2000 stuff from some Elms) and either fails or returns bogus
-;; values.  timezone-make-date-arpa-standard should help.
 (defun date-to-time (date)
   "Parse a string DATE that represents a date-time and return a time value.
 DATE should be in one of the forms recognized by `parse-time-string'.
-If DATE lacks timezone information, GMT is assumed."
+If DATE lacks timezone information, local time is assumed."
   (condition-case err
       (let ((parsed (parse-time-string date)))
 	(when (decoded-time-year parsed)
 	  (decoded-time-set-defaults parsed))
 	(encode-time parsed))
     (error
-     (let ((overflow-error '(error "Specified time is not representable")))
-       (if (equal err overflow-error)
-	   (signal (car err) (cdr err))
-	 (condition-case err
-	     (encode-time (parse-time-string
-			   (timezone-make-date-arpa-standard date)))
-	   (error
-	    (if (equal err overflow-error)
-		(signal (car err) (cdr err))
-	      (error "Invalid date: %s" date)))))))))
+     (if (equal err '(error "Specified time is not representable"))
+	 (signal (car err) (cdr err))
+       (error "Invalid date: %s" date)))))
 
 ;;;###autoload
 (defalias 'time-to-seconds #'float-time)
diff --git a/test/lisp/calendar/time-date-tests.el b/test/lisp/calendar/time-date-tests.el
index 6512dd0bd07..f8e434e17b1 100644
--- a/test/lisp/calendar/time-date-tests.el
+++ b/test/lisp/calendar/time-date-tests.el
@@ -42,8 +42,28 @@ test-obsolete-encode-time-value
                  '(1 2 3 4))))
 
 (ert-deftest test-date-to-time ()
-  (should (equal (format-time-string "%F %T" (date-to-time "2021-12-04"))
-                 "2021-12-04 00:00:00")))
+  (let ((date-list
+         '(("2021-12-04"                   (00 00 00 04 12 2021 nil -1 nil))
+           ("2006-05-04T03:02:01Z"         (01 02 03 04 05 2006 nil nil 0))
+           ;; Test cases from timezone-parse-date docstring
+           ("14 Apr 89 03:20"              (00 20 03 14 04 1989 nil -1 nil))
+           ("14 Apr 89 03:20:12 GMT"       (12 20 03 14 04 1989 nil nil 0))
+           ("Fri, 17 Mar 89 4:01"          (00 01 04 17 03 1989 nil -1 nil))
+           ("Fri, 17 Mar 89 4:01:33 GMT"   (33 01 04 17 03 1989 nil nil 0))
+           ("Mon Jan 16 16:12 1989"        (00 12 16 16 01 1989 nil -1 nil))
+           ("Mon Jan 16 16:12:37 GMT 1989" (37 12 16 16 01 1989 nil nil 0))
+           ("Thu, 11 Apr 16:17:12 91"      (12 17 16 11 04 1991 nil -1 nil))
+           ("Mon, 6  Jul 16:47:20 T 1992"  (20 47 16 06 07 1992 nil -1 nil))
+           ("1996-06-24 21:13:12"          (12 13 21 24 06 1996 nil -1 nil))
+           ("19960624t211312"              (12 13 21 24 06 1996 nil -1 nil))
+           ;; These are parsed incorrectly:
+           ;; "6 May 1992 1641-JST (Wednesday)"
+           ;; "22-AUG-1993 10:59:12.82"
+           ;; "1996-06-24 21:13-ZONE"
+           )))
+    (dolist (date date-list)
+      (should (equal (date-to-time (car date))
+                     (encode-time (cadr date)))))))
 
 (ert-deftest test-days-between ()
   (should (equal (days-between "2021-10-22" "2020-09-29") 388)))
-- 
2.45.2


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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-14 14:09             ` Ulrich Mueller
@ 2024-08-15  3:27               ` Paul Eggert
  2024-08-15  4:35                 ` Ulrich Mueller
  0 siblings, 1 reply; 26+ messages in thread
From: Paul Eggert @ 2024-08-15  3:27 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: Eli Zaretskii, 72570

On 2024-08-14 07:09, Ulrich Mueller wrote:
> The attached patch does the former, and also adds some test cases.

Thanks, looks good except it needs an entry in etc/NEWS.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-15  3:27               ` Paul Eggert
@ 2024-08-15  4:35                 ` Ulrich Mueller
  2024-08-15  6:26                   ` Paul Eggert
  2024-08-15  6:45                   ` Eli Zaretskii
  0 siblings, 2 replies; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-15  4:35 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Eli Zaretskii, 72570

>>>>> On Thu, 15 Aug 2024, Paul Eggert wrote:

> Thanks, looks good except it needs an entry in etc/NEWS.

This OK? It would go under "Lisp Changes in Emacs 31.1", I suppose?

+++
** 'date-to-time' now defaults to local time
The function now assumes local time instead of Universal Time when
its argument lacks explicit time zone information.  This has been its
de-facto behavior since Emacs 24 although documentation said otherwise.
Also, the fallback on 'timezone-make-date-arpa-standard' has been
removed because its supported date styles can be handled by
'parse-time-string'.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-15  4:35                 ` Ulrich Mueller
@ 2024-08-15  6:26                   ` Paul Eggert
  2024-08-15  6:45                   ` Eli Zaretskii
  1 sibling, 0 replies; 26+ messages in thread
From: Paul Eggert @ 2024-08-15  6:26 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: Eli Zaretskii, 72570

On 2024-08-14 21:35, Ulrich Mueller wrote:
> This OK? It would go under "Lisp Changes in Emacs 31.1", I suppose?

Yes, looks good to me.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-15  4:35                 ` Ulrich Mueller
  2024-08-15  6:26                   ` Paul Eggert
@ 2024-08-15  6:45                   ` Eli Zaretskii
  2024-08-15  7:18                     ` Ulrich Mueller
  1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-15  6:45 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: 72570, eggert

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  72570@debbugs.gnu.org
> Date: Thu, 15 Aug 2024 06:35:07 +0200
> 
> >>>>> On Thu, 15 Aug 2024, Paul Eggert wrote:
> 
> > Thanks, looks good except it needs an entry in etc/NEWS.
> 
> This OK? It would go under "Lisp Changes in Emacs 31.1", I suppose?
> 
> +++
> ** 'date-to-time' now defaults to local time
> The function now assumes local time instead of Universal Time when
> its argument lacks explicit time zone information.  This has been its
> de-facto behavior since Emacs 24 although documentation said otherwise.
> Also, the fallback on 'timezone-make-date-arpa-standard' has been
> removed because its supported date styles can be handled by
> 'parse-time-string'.

Please say explicitly how to get the UTC interpretation instead of the
local-time interpretation (although it is arguably almost obvious).





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-15  6:45                   ` Eli Zaretskii
@ 2024-08-15  7:18                     ` Ulrich Mueller
  2024-08-15  7:22                       ` Eli Zaretskii
  0 siblings, 1 reply; 26+ messages in thread
From: Ulrich Mueller @ 2024-08-15  7:18 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 72570, eggert

>>>>> On Thu, 15 Aug 2024, Eli Zaretskii wrote:

>> +++
>> ** 'date-to-time' now defaults to local time
>> The function now assumes local time instead of Universal Time when
>> its argument lacks explicit time zone information.  This has been its
>> de-facto behavior since Emacs 24 although documentation said otherwise.
>> Also, the fallback on 'timezone-make-date-arpa-standard' has been
>> removed because its supported date styles can be handled by
>> 'parse-time-string'.

> Please say explicitly how to get the UTC interpretation instead of the
> local-time interpretation (although it is arguably almost obvious).

Like this?

  To restore the previously documented behavior, specify "+0000" or "Z"
  as the time zone in the argument.





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

* bug#72570: 31.0.50; Regression in date-to-time
  2024-08-15  7:18                     ` Ulrich Mueller
@ 2024-08-15  7:22                       ` Eli Zaretskii
  0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2024-08-15  7:22 UTC (permalink / raw)
  To: Ulrich Mueller; +Cc: 72570, eggert

> From: Ulrich Mueller <ulm@gentoo.org>
> Cc: eggert@cs.ucla.edu,  72570@debbugs.gnu.org
> Date: Thu, 15 Aug 2024 09:18:35 +0200
> 
> >>>>> On Thu, 15 Aug 2024, Eli Zaretskii wrote:
> 
> >> +++
> >> ** 'date-to-time' now defaults to local time
> >> The function now assumes local time instead of Universal Time when
> >> its argument lacks explicit time zone information.  This has been its
> >> de-facto behavior since Emacs 24 although documentation said otherwise.
> >> Also, the fallback on 'timezone-make-date-arpa-standard' has been
> >> removed because its supported date styles can be handled by
> >> 'parse-time-string'.
> 
> > Please say explicitly how to get the UTC interpretation instead of the
> > local-time interpretation (although it is arguably almost obvious).
> 
> Like this?
> 
>   To restore the previously documented behavior, specify "+0000" or "Z"
>   as the time zone in the argument.

Yes, thanks.





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

end of thread, other threads:[~2024-08-15  7:22 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-11  8:57 bug#72570: 31.0.50; Regression in date-to-time Ulrich Mueller
2024-08-12  7:27 ` Ulrich Mueller
2024-08-12  8:18   ` Ulrich Mueller
2024-08-12 11:42 ` Eli Zaretskii
2024-08-12 12:51   ` Ulrich Mueller
2024-08-12 13:26     ` Eli Zaretskii
2024-08-12 13:48       ` Ulrich Mueller
2024-08-12 14:30         ` Eli Zaretskii
2024-08-12 22:03   ` Paul Eggert
2024-08-13  5:55     ` Ulrich Mueller
2024-08-13 19:39       ` Paul Eggert
2024-08-13 21:11         ` Ulrich Mueller
2024-08-13 21:19           ` Paul Eggert
2024-08-14  8:12             ` Ulrich Mueller
2024-08-14 14:09             ` Ulrich Mueller
2024-08-15  3:27               ` Paul Eggert
2024-08-15  4:35                 ` Ulrich Mueller
2024-08-15  6:26                   ` Paul Eggert
2024-08-15  6:45                   ` Eli Zaretskii
2024-08-15  7:18                     ` Ulrich Mueller
2024-08-15  7:22                       ` Eli Zaretskii
2024-08-13 11:15     ` Eli Zaretskii
2024-08-13 15:09       ` Ulrich Mueller
2024-08-13 15:36         ` Eli Zaretskii
2024-08-13 19:59       ` Paul Eggert
2024-08-14  8:37         ` Eli Zaretskii

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

	https://git.savannah.gnu.org/cgit/emacs.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).