bug#39001: parse-time-string fails to parse Info node's example

bug#39001: parse-time-string fails to parse Info node's example

[mark skilbeck]

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

The info node (elisp) Time Parsing suggests that the string
"1998-09-12T12:21:54-0200" should be of a format that `parse-time-string`
can successfully parse. However, trying to do so produces no useful info
from parsing (it returns a list of NILs).

GNU Emacs 26.3 (build 1, x86_64-apple-darwin18.5.0, Carbon Version 158
AppKit 1671.4) of 2019-08-29

[-- Attachment #2: Type: text/html, Size: 417 bytes --]

bug#39001: parse-time-string fails to parse Info node's example

[Paul Eggert]

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

Thanks for the bug report. I fixed the code so that it matches the 
documentation by installing the attached patches into Emacs master. The 
first patch is the fix, the second is merely refactoring.

[-- Attachment #2: 0001-parse-time-string-now-parses-ISO-8601-format-strings.patch --]
[-- Type: text/x-patch, Size: 3307 bytes --]

From 33d9ca387824e050650b49a60083e4e4cb4e3d11 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 15 Jan 2020 17:36:59 -0800
Subject: [PATCH 1/2] parse-time-string now parses ISO 8601 format strings

* lisp/calendar/parse-time.el (parse-time-string):
Parse strings in ISO 8601 format too (Bug#39001).
(parse-time--rfc-822ish): New internal function,
containing most of the old parse-time-string implementation.
(parse-iso8601-time-string): Simplify, now that
parse-time-string groks ISO 8601.
---
 etc/NEWS                    |  3 +++
 lisp/calendar/parse-time.el | 25 ++++++++++++++-----------
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 0e43c321d8..3d5915a377 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -70,6 +70,9 @@ called when the function object is garbage-collected.  Use
 'set_function_finalizer' to set the finalizer and
 'get_function_finalizer' to retrieve it.
 
+** 'parse-time-string' can now parse ISO 8601 format strings,
+such as "2020-01-15T16:12:21-08:00".
+
 \f
 * Changes in Emacs 28.1 on Non-Free Operating Systems
 
diff --git a/lisp/calendar/parse-time.el b/lisp/calendar/parse-time.el
index 7110a81f0d..4d4f88efff 100644
--- a/lisp/calendar/parse-time.el
+++ b/lisp/calendar/parse-time.el
@@ -149,13 +149,20 @@ parse-time-rules
 ;;;###autoload
 (defun parse-time-string (string)
   "Parse the time in STRING into (SEC MIN HOUR DAY MON YEAR DOW DST TZ).
-STRING should be something resembling an RFC 822 (or later) date-time, e.g.,
-\"Fri, 25 Mar 2016 16:24:56 +0100\", but this function is
+STRING should be an ISO 8601 time string, e.g., \"2020-01-15T16:12:21-08:00\",
+or something resembling an RFC 822 (or later) date-time, e.g.,
+\"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."
+  (condition-case ()
+      (decoded-time-set-defaults (iso8601-parse string))
+    (wrong-type-argument
+     (parse-time--rfc-822ish string))))
+
+(defun parse-time--rfc-822ish (string)
   (let ((time (list nil nil nil nil nil nil nil -1 nil))
 	(temp (parse-time-tokenize (downcase string))))
     (while temp
@@ -196,15 +203,11 @@ parse-time-string
     time))
 
 (defun parse-iso8601-time-string (date-string)
-  "Parse an ISO 8601 time string, such as 2016-12-01T23:35:06-05:00.
-If DATE-STRING cannot be parsed, it falls back to
-`parse-time-string'."
-  (when-let ((time
-              (if (iso8601-valid-p date-string)
-                  (decoded-time-set-defaults (iso8601-parse date-string))
-                ;; Fall back to having `parse-time-string' do fancy
-                ;; things for us.
-                (parse-time-string date-string))))
+  "Parse an ISO 8601 time string, such as \"2020-01-15T16:12:21-08:00\".
+Fall back on parsing something resembling an RFC 822 (or later) date-time.
+This function is like `parse-time-string' except that it returns
+a Lisp timestamp when successful."
+  (when-let ((time (parse-time-string date-string)))
     (encode-time time)))
 
 (provide 'parse-time)
-- 
2.24.1


[-- Attachment #3: 0002-Refactor-parse-time-string.patch --]
[-- Type: text/x-patch, Size: 3427 bytes --]

From d3db908ffbfa17ff74080455ac0520c663f920ee Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 15 Jan 2020 17:41:42 -0800
Subject: [PATCH 2/2] Refactor parse-time-string

* lisp/calendar/parse-time.el (parse-time--rfc-822ish):
Remove, and fold its body into its only caller.
---
 lisp/calendar/parse-time.el | 79 ++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 41 deletions(-)

diff --git a/lisp/calendar/parse-time.el b/lisp/calendar/parse-time.el
index 4d4f88efff..6a4612297c 100644
--- a/lisp/calendar/parse-time.el
+++ b/lisp/calendar/parse-time.el
@@ -160,47 +160,44 @@ parse-time-string
   (condition-case ()
       (decoded-time-set-defaults (iso8601-parse string))
     (wrong-type-argument
-     (parse-time--rfc-822ish string))))
-
-(defun parse-time--rfc-822ish (string)
-  (let ((time (list nil nil nil nil nil nil nil -1 nil))
-	(temp (parse-time-tokenize (downcase string))))
-    (while temp
-      (let ((parse-time-elt (pop temp))
-	    (rules parse-time-rules)
-	    (exit nil))
-	(while (and rules (not exit))
-	  (let* ((rule (pop rules))
-		 (slots (pop rule))
-		 (predicate (pop rule))
-		 (parse-time-val))
-	    (when (and (not (nth (car slots) time)) ;not already set
-		       (setq parse-time-val
-			     (cond ((and (consp predicate)
-					 (not (functionp predicate)))
-				    (and (numberp parse-time-elt)
-					 (<= (car predicate) parse-time-elt)
-					 (or (not (cdr predicate))
-					     (<= parse-time-elt
-						 (cadr predicate)))
-					 parse-time-elt))
-				   ((symbolp predicate)
-				    (cdr (assoc parse-time-elt
-						(symbol-value predicate))))
-				   ((funcall predicate)))))
-	      (setq exit t)
-	      (while slots
-		(let ((new-val (if rule
-				   (let ((this (pop rule)))
-				     (if (vectorp this)
-					 (cl-parse-integer
-					  parse-time-elt
-					  :start (aref this 0)
-					  :end (aref this 1))
-				       (funcall this)))
-				 parse-time-val)))
-		  (setf (nth (pop slots) time) new-val))))))))
-    time))
+     (let ((time (list nil nil nil nil nil nil nil -1 nil))
+	   (temp (parse-time-tokenize (downcase string))))
+       (while temp
+	 (let ((parse-time-elt (pop temp))
+	       (rules parse-time-rules)
+	       (exit nil))
+	   (while (and rules (not exit))
+	     (let* ((rule (pop rules))
+		    (slots (pop rule))
+		    (predicate (pop rule))
+		    (parse-time-val))
+	       (when (and (not (nth (car slots) time)) ;not already set
+			  (setq parse-time-val
+				(cond ((and (consp predicate)
+					    (not (functionp predicate)))
+				       (and (numberp parse-time-elt)
+					    (<= (car predicate) parse-time-elt)
+					    (or (not (cdr predicate))
+						(<= parse-time-elt
+						    (cadr predicate)))
+					    parse-time-elt))
+				      ((symbolp predicate)
+				       (cdr (assoc parse-time-elt
+						   (symbol-value predicate))))
+				      ((funcall predicate)))))
+		 (setq exit t)
+		 (while slots
+		   (let ((new-val (if rule
+				      (let ((this (pop rule)))
+					(if (vectorp this)
+					    (cl-parse-integer
+					     parse-time-elt
+					     :start (aref this 0)
+					     :end (aref this 1))
+					  (funcall this)))
+				    parse-time-val)))
+		     (setf (nth (pop slots) time) new-val))))))))
+       time))))
 
 (defun parse-iso8601-time-string (date-string)
   "Parse an ISO 8601 time string, such as \"2020-01-15T16:12:21-08:00\".
-- 
2.24.1

bug#39001: closed (Re: parse-time-string fails to parse Info node's example)

[Stefan Kangas]

Paul Eggert <eggert@cs.ucla.edu> writes:

> Thanks for the bug report. I fixed the code so that it matches the documentation
> by installing the attached patches into Emacs master. The first patch is the
> fix, the second is merely refactoring.

There's a test suite in
test/lisp/calendar/parse-time-tests.el

Perhaps we should add a test or two there?

Best regards,
Stefan Kangas

bug#39001: parse-time-string fails to parse Info node's example

[Paul Eggert]

On 1/15/20 6:14 PM, Stefan Kangas wrote:
> Perhaps we should add a test or two there?

Yes, please go ahead.