Subject: Re: bug#55635: `make-decoded-time' incorrectly sets DST to nil, it should be -1 (guess)

From: Paul Eggert <[email-masked]>
Date: Sun, 29 May 2022 15:04:38 -0700

On 5/29/22 06:10, Lars Ingebrigtsen wrote:
> Paul Eggert <[email-masked]> writes:
> 
>> With that in mind, here are three suggestions.
>>
>> 1. decoded-time-dst should return -1, instead of nil, when given a
>> 6-element list, since nil means standard time and -1 means DST is
>> unknown.
> 
> I think that sounds correct.

I looked into that, and since decoded-time-dst is defined by 
cl-defstruct it's not easy to change how it works. For now, I just 
documented the glitch; see the 3rd attached patch.

I also installed the 1st attached patch which mentions that Emacs's 
decoded dst flag differs from that of Common Lisp, and the 2nd attached 
patch which makes iso8601 parsing compatible with what we've discussed 
here by having it return -1 for unknown DST flags.

>> > 2. make-decoded-time should generate a six-element list unless given a
>> > DST or ZONE arg.
> 
> I don't think we should do this.

OK, I left that alone.

From 01e814b6eee70d7ba63b4bc2b183e43b900ed187 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sun, 29 May 2022 08:52:13 -0700
Subject: [PATCH 1/3] Doc fix for dst flag

* doc/lispref/os.texi (Time Conversion): Note Common Lisp dst differs.
---
 doc/lispref/os.texi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index f4dd2e7072..11a0d02338 100644
--- a/doc/lispref/os.texi
+++ b/doc/lispref/os.texi
@@ -1649,8 +1649,8 @@ Time Conversion
 a particular form should specify @var{form}.
 
 @strong{Common Lisp Note:} Common Lisp has different meanings for
-@var{dow} and @var{utcoff}, and its @var{second} is an integer between
-0 and 59 inclusive.
+@var{dow}, @code{dst} and @var{utcoff}, and its @var{second} is an
+integer between 0 and 59 inclusive.
 
 To access (or alter) the elements in the calendrical information, the
 @code{decoded-time-second}, @code{decoded-time-minute},
-- 
2.34.1

From 477302d490adf3e7c4c1d772aaba7dbdd454b60a Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sun, 29 May 2022 13:07:50 -0700
Subject: [PATCH 2/3] =?UTF-8?q?ISO=208601=20strings=20sans=20"Z"=20don?=
 =?UTF-8?q?=E2=80=99t=20specify=20DST=20flag?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/calendar/iso8601.el (iso8601--zone-dst): New function.
(iso8601-parse, iso8601-parse-time): Use it.
(iso8601--decoded-time): Default dst to -1, not nil.
* test/lisp/calendar/iso8601-tests.el (test-iso8601-combined)
(standard-test-time-of-day-zone): Adjust to new behavior.
---
 lisp/calendar/iso8601.el            | 12 +++++++++---
 test/lisp/calendar/iso8601-tests.el | 20 ++++++++++----------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/lisp/calendar/iso8601.el b/lisp/calendar/iso8601.el
index e31120f52f..6827a957a6 100644
--- a/lisp/calendar/iso8601.el
+++ b/lisp/calendar/iso8601.el
@@ -114,6 +114,11 @@ iso8601--duration-match
          iso8601--duration-week-match
          iso8601--duration-combined-match)))
 
+;; "Z" dnd "z" are standard time; nil and [-+][0-9][0-9]... are local time
+;; with unknown DST.
+(defun iso8601--zone-dst (zone)
+  (if (= (length zone) 1) nil -1))
+
 (defun iso8601-parse (string &optional form)
   "Parse an ISO 8601 date/time string and return a `decode-time' structure.
 
@@ -140,7 +145,7 @@ iso8601-parse
         (setf (decoded-time-zone date)
               ;; The time zone in decoded times are in seconds.
 	      (* (iso8601-parse-zone zone-string) 60))
-	(setf (decoded-time-dst date) nil))
+	(setf (decoded-time-dst date) (iso8601--zone-dst zone-string)))
       date)))
 
 (defun iso8601-parse-date (string)
@@ -256,6 +261,7 @@ iso8601-parse-time
           (iso8601--decoded-time :hour hour
                                  :minute (or minute 0)
                                  :second (or second 0)
+				 :dst (iso8601--zone-dst zone)
                                  :zone (and zone
                                             (* 60 (iso8601-parse-zone
                                                    zone)))))))))
@@ -364,7 +370,7 @@ iso8601--value
 
 (cl-defun iso8601--decoded-time (&key second minute hour
                                       day month year
-                                      dst zone)
+				      (dst -1) zone)
   (list (iso8601--value second)
         (iso8601--value minute)
         (iso8601--value hour)
@@ -372,7 +378,7 @@ iso8601--decoded-time
         (iso8601--value month)
         (iso8601--value year)
         nil
-	(if (or dst zone) dst -1)
+	dst
         zone))
 
 (defun iso8601--encode-time (time)
diff --git a/test/lisp/calendar/iso8601-tests.el b/test/lisp/calendar/iso8601-tests.el
index 6c9e85ec92..f64c498c02 100644
--- a/test/lisp/calendar/iso8601-tests.el
+++ b/test/lisp/calendar/iso8601-tests.el
@@ -82,9 +82,9 @@ test-iso8601-combined
   (should (equal (iso8601-parse "2008-03-02T13:47:30Z")
                  '(30 47 13 2 3 2008 nil nil 0)))
   (should (equal (iso8601-parse "2008-03-02T13:47:30+01:00")
-                 '(30 47 13 2 3 2008 nil nil 3600)))
+                 '(30 47 13 2 3 2008 nil -1 3600)))
   (should (equal (iso8601-parse "2008-03-02T13:47:30-01")
-                 '(30 47 13 2 3 2008 nil nil -3600))))
+                 '(30 47 13 2 3 2008 nil -1 -3600))))
 
 (ert-deftest test-iso8601-duration ()
   (should (equal (iso8601-parse-duration "P3Y6M4DT12H30M5S")
@@ -221,24 +221,24 @@ standard-test-time-of-day-utc
 
 (ert-deftest standard-test-time-of-day-zone ()
   (should (equal (iso8601-parse-time "152746+0100")
-                 '(46 27 15 nil nil nil nil nil 3600)))
+                 '(46 27 15 nil nil nil nil -1 3600)))
   (should (equal (iso8601-parse-time "15:27:46+0100")
-                 '(46 27 15 nil nil nil nil nil 3600)))
+                 '(46 27 15 nil nil nil nil -1 3600)))
 
   (should (equal (iso8601-parse-time "152746+01")
-                 '(46 27 15 nil nil nil nil nil 3600)))
+                 '(46 27 15 nil nil nil nil -1 3600)))
   (should (equal (iso8601-parse-time "15:27:46+01")
-                 '(46 27 15 nil nil nil nil nil 3600)))
+                 '(46 27 15 nil nil nil nil -1 3600)))
 
   (should (equal (iso8601-parse-time "152746-0500")
-                 '(46 27 15 nil nil nil nil nil -18000)))
+                 '(46 27 15 nil nil nil nil -1 -18000)))
   (should (equal (iso8601-parse-time "15:27:46-0500")
-                 '(46 27 15 nil nil nil nil nil -18000)))
+                 '(46 27 15 nil nil nil nil -1 -18000)))
 
   (should (equal (iso8601-parse-time "152746-05")
-                 '(46 27 15 nil nil nil nil nil -18000)))
+                 '(46 27 15 nil nil nil nil -1 -18000)))
   (should (equal (iso8601-parse-time "15:27:46-05")
-                 '(46 27 15 nil nil nil nil nil -18000))))
+                 '(46 27 15 nil nil nil nil -1 -18000))))

 (defun test-iso8601-format-time-string-zone-round-trip (offset-minutes z-format)
-- 
2.34.1

From 63da939a7afac05775f6c8183d59ab8f78ce0f01 Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sun, 29 May 2022 14:57:48 -0700
Subject: [PATCH 3/3] Document decoded-time-string issue on 6-elt args

* lisp/simple.el: Document problematic use of decoded-time-dst on
6-element args.
---
 lisp/simple.el | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lisp/simple.el b/lisp/simple.el
index a254ee2251..d6b7045432 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -10519,6 +10519,14 @@ capitalize-dwim
 the number of seconds east of Greenwich.")
   )
 
+;; Document that decoded-time-dst is problematic on 6-element lists.
+;; It should return -1 indicating unknown DST, but currently returns
+;; nil indicating standard time.
+(put 'decoded-time-dst 'function-documentation
+     (append (get 'decoded-time-dst 'function-documentation)
+             "As a special case, `decoded-time-dst' returns an unspecified
+value when given a list too short to have a dst element."))
+
 (defun get-scratch-buffer-create ()
   "Return the *scratch* buffer, creating a new one if needed."
   (or (get-buffer "*scratch*")
-- 
2.34.1