unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
@ 2016-12-02  3:46 Hong Xu
  2016-12-02  3:55 ` Hong Xu
  0 siblings, 1 reply; 13+ messages in thread
From: Hong Xu @ 2016-12-02  3:46 UTC (permalink / raw)
  To: 25086

	* parse-time.el (parse-iso8601-time-string): Fix its timezone
        parsing.
        * parse-time.el: Add doc for parse-iso8601-time-string and
        make it autoload.
        * editfns.c (Fdecode-time): Minor doc improvement for decode-time.
---
 doc/misc/emacs-mime.texi    |  3 +++
 lisp/calendar/parse-time.el | 12 ++++++++----
 src/editfns.c               |  5 ++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/doc/misc/emacs-mime.texi b/doc/misc/emacs-mime.texi
index 4d68246bba4b..8d6536265416 100644
--- a/doc/misc/emacs-mime.texi
+++ b/doc/misc/emacs-mime.texi
@@ -1536,6 +1536,9 @@ time-date
 (date-to-time "Sat Sep 12 12:21:54 1998 +0200")
 @result{} (13818 19266)
 
+(parse-iso8601-time-string "1998-09-12T12:21:54+0200")
+@result{} (13818 19266)
+
 (float-time '(13818 19266))
 @result{} 905595714.0
 
diff --git a/lisp/calendar/parse-time.el b/lisp/calendar/parse-time.el
index 6ba26a4a00d0..bc1efbb074e3 100644
--- a/lisp/calendar/parse-time.el
+++ b/lisp/calendar/parse-time.el
@@ -195,7 +195,7 @@ parse-time-iso8601-regexp
 	 (time-minute 2digit)
 	 (time-second 2digit)
 	 (time-secfrac "\\(\\.[0-9]+\\)?")
-	 (time-numoffset (concat "[-+]\\(" time-hour "\\):" time-minute))
+	 (time-numoffset (concat "\\([-+]" time-hour "\\):?" time-minute))
 	 (time-offset (concat "Z" time-numoffset))
 	 (partial-time (concat time-hour colon time-minute colon time-second
 			       time-secfrac))
@@ -204,13 +204,17 @@ parse-time-iso8601-regexp
 	 (date-time (concat full-date "T" full-time)))
     (list (concat "^" full-date)
 	  (concat "T" partial-time)
-	  (concat "Z" time-numoffset)))
+	  (concat "Z?" time-numoffset)))
   "List of regular expressions matching ISO 8601 dates.
 1st regular expression matches the date.
 2nd regular expression matches the time.
 3rd regular expression matches the (optional) timezone specification.")
 
+;;;###autoload
 (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'."
   (let* ((date-re (nth 0 parse-time-iso8601-regexp))
 	 (time-re (nth 1 parse-time-iso8601-regexp))
 	 (tz-re (nth 2 parse-time-iso8601-regexp))
@@ -235,10 +239,10 @@ parse-iso8601-time-string
                                                     "0"))
 	      re-start (match-end 0))
 	(when (string-match tz-re date-string re-start)
-	  (setq tz (match-string 1 date-string)))
+	  (setq tz (* 3600 (string-to-number (match-string 1 date-string)))))
 	(setq time (list seconds minute hour day month year day-of-week dst tz))))
 
-    ;; Fall back to having Gnus do fancy things for us.
+    ;; Fall back to having `parse-time-string' do fancy things for us.
     (when (not time)
       (setq time (parse-time-string date-string)))
 
diff --git a/src/editfns.c b/src/editfns.c
index 5cc4a67ab19b..2f1f412d9e4e 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2101,9 +2101,8 @@ format_time_string (char const *format, ptrdiff_t formatlen,
 
 DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 2, 0,
        doc: /* Decode a time value as (SEC MINUTE HOUR DAY MONTH YEAR DOW DST UTCOFF).
-The optional SPECIFIED-TIME should be a list of (HIGH LOW . IGNORED),
-as from `current-time' and `file-attributes', or nil to use the
-current time.
+The optional TIME should be a list of (HIGH LOW . IGNORED), as from
+`current-time' and `file-attributes', or nil to use the current time.
 It can also be a single integer number of seconds since the epoch.
 The obsolete form (HIGH . LOW) is also still accepted.
 The optional ZONE is omitted or nil for Emacs local time, t for
-- 
2.1.4







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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-02  3:46 bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string Hong Xu
@ 2016-12-02  3:55 ` Hong Xu
  2016-12-02  4:04   ` Hong Xu
  0 siblings, 1 reply; 13+ messages in thread
From: Hong Xu @ 2016-12-02  3:55 UTC (permalink / raw)
  To: 25086

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


On 2016-12-01 Thu 19:46 GMT-0800, Hong Xu <hong@topbug.net> wrote:

> 	* parse-time.el (parse-iso8601-time-string): Fix its timezone
>         parsing.
>         * parse-time.el: Add doc for parse-iso8601-time-string and
>         make it autoload.
>         * editfns.c (Fdecode-time): Minor doc improvement for decode-time.

I should have noted that the current version basically cannot parse the
timezone at all. To reproduce, try the following:

(format-time-string "%H %M %S"
                    (parse-iso8601-time-string "1998-09-12T12:21:54-0200") t)


Also missed one line:

* emacs-mime.texi (time-date): Add an example for parse-iso8601-time-string.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-02  3:55 ` Hong Xu
@ 2016-12-02  4:04   ` Hong Xu
       [not found]     ` <qf5fum6ubn4.fsf@marmstrong-macbookpro.roam.corp.google.com>
  2016-12-17 14:58     ` Eli Zaretskii
  0 siblings, 2 replies; 13+ messages in thread
From: Hong Xu @ 2016-12-02  4:04 UTC (permalink / raw)
  To: 25086


[-- Attachment #1.1: Type: text/plain, Size: 826 bytes --]


On 2016-12-01 Thu 19:55 GMT-0800, Hong Xu <hong@topbug.net> wrote:

> On 2016-12-01 Thu 19:46 GMT-0800, Hong Xu <hong@topbug.net> wrote:
>
>> 	* parse-time.el (parse-iso8601-time-string): Fix its timezone
>>         parsing.
>>         * parse-time.el: Add doc for parse-iso8601-time-string and
>>         make it autoload.
>>         * editfns.c (Fdecode-time): Minor doc improvement for decode-time.
>
> I should have noted that the current version basically cannot parse the
> timezone at all. To reproduce, try the following:
>
> (format-time-string "%H %M %S"
>                     (parse-iso8601-time-string "1998-09-12T12:21:54-0200") t)
>
>
> Also missed one line:
>
> * emacs-mime.texi (time-date): Add an example for parse-iso8601-time-string.

I've now updated the patch and put everything together, as attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Fix-the-timezone-detection-of-parse-iso8601-time-str.patch --]
[-- Type: text/x-diff, Size: 3929 bytes --]

[PATCH] Fix the timezone detection of parse-iso8601-time-string.

        * parse-time.el (parse-iso8601-time-string): Fix its timezone
        parsing.
        * parse-time.el: Add doc for parse-iso8601-time-string and
        make it autoload.
        * editfns.c (Fdecode-time): Minor doc improvement for decode-time.
        * emacs-mime.texi (time-date): Add an example for
        parse-iso8601-time-string.
---
 doc/misc/emacs-mime.texi    |  3 +++
 lisp/calendar/parse-time.el | 12 ++++++++----
 src/editfns.c               |  5 ++---
 3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/doc/misc/emacs-mime.texi b/doc/misc/emacs-mime.texi
index 4d68246bba4b..8d6536265416 100644
--- a/doc/misc/emacs-mime.texi
+++ b/doc/misc/emacs-mime.texi
@@ -1536,6 +1536,9 @@ time-date
 (date-to-time "Sat Sep 12 12:21:54 1998 +0200")
 @result{} (13818 19266)
 
+(parse-iso8601-time-string "1998-09-12T12:21:54+0200")
+@result{} (13818 19266)
+
 (float-time '(13818 19266))
 @result{} 905595714.0
 
diff --git a/lisp/calendar/parse-time.el b/lisp/calendar/parse-time.el
index 6ba26a4a00d0..e9dac0149b10 100644
--- a/lisp/calendar/parse-time.el
+++ b/lisp/calendar/parse-time.el
@@ -195,7 +195,7 @@ parse-time-iso8601-regexp
 	 (time-minute 2digit)
 	 (time-second 2digit)
 	 (time-secfrac "\\(\\.[0-9]+\\)?")
-	 (time-numoffset (concat "[-+]\\(" time-hour "\\):" time-minute))
+	 (time-numoffset (concat "\\([-+]" time-hour "\\):?" time-minute "?"))
 	 (time-offset (concat "Z" time-numoffset))
 	 (partial-time (concat time-hour colon time-minute colon time-second
 			       time-secfrac))
@@ -204,13 +204,17 @@ parse-time-iso8601-regexp
 	 (date-time (concat full-date "T" full-time)))
     (list (concat "^" full-date)
 	  (concat "T" partial-time)
-	  (concat "Z" time-numoffset)))
+	  (concat "Z?" time-numoffset)))
   "List of regular expressions matching ISO 8601 dates.
 1st regular expression matches the date.
 2nd regular expression matches the time.
 3rd regular expression matches the (optional) timezone specification.")
 
+;;;###autoload
 (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'."
   (let* ((date-re (nth 0 parse-time-iso8601-regexp))
 	 (time-re (nth 1 parse-time-iso8601-regexp))
 	 (tz-re (nth 2 parse-time-iso8601-regexp))
@@ -235,10 +239,10 @@ parse-iso8601-time-string
                                                     "0"))
 	      re-start (match-end 0))
 	(when (string-match tz-re date-string re-start)
-	  (setq tz (match-string 1 date-string)))
+	  (setq tz (* 3600 (string-to-number (match-string 1 date-string)))))
 	(setq time (list seconds minute hour day month year day-of-week dst tz))))
 
-    ;; Fall back to having Gnus do fancy things for us.
+    ;; Fall back to having `parse-time-string' do fancy things for us.
     (when (not time)
       (setq time (parse-time-string date-string)))
 
diff --git a/src/editfns.c b/src/editfns.c
index 5cc4a67ab19b..2f1f412d9e4e 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2101,9 +2101,8 @@ format_time_string (char const *format, ptrdiff_t formatlen,
 
 DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 2, 0,
        doc: /* Decode a time value as (SEC MINUTE HOUR DAY MONTH YEAR DOW DST UTCOFF).
-The optional SPECIFIED-TIME should be a list of (HIGH LOW . IGNORED),
-as from `current-time' and `file-attributes', or nil to use the
-current time.
+The optional TIME should be a list of (HIGH LOW . IGNORED), as from
+`current-time' and `file-attributes', or nil to use the current time.
 It can also be a single integer number of seconds since the epoch.
 The obsolete form (HIGH . LOW) is also still accepted.
 The optional ZONE is omitted or nil for Emacs local time, t for
-- 
2.1.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
       [not found]     ` <qf5fum6ubn4.fsf@marmstrong-macbookpro.roam.corp.google.com>
@ 2016-12-02  6:13       ` Hong Xu
  2016-12-12  0:37         ` Hong Xu
  0 siblings, 1 reply; 13+ messages in thread
From: Hong Xu @ 2016-12-02  6:13 UTC (permalink / raw)
  To: Matt Armstrong; +Cc: 25086


[-- Attachment #1.1: Type: text/plain, Size: 696 bytes --]


On 2016-12-01 Thu 21:45 GMT-0800, Matt Armstrong <marmstrong@google.com> wrote:

> Hong Xu <hong@topbug.net> writes:
>
>>  doc/misc/emacs-mime.texi    |  3 +++
>>  lisp/calendar/parse-time.el | 12 ++++++++----
>>  src/editfns.c               |  5 ++---
>>  3 files changed, 13 insertions(+), 7 deletions(-)
>
> Hi Hong, have you seen test/lisp/calendar/parse-time-tests.el?  It might
> be nice to add test coverage for parse-iso8601-time-string there.


Thanks for the reminder. A patch to add tests is attached.

Note that I've kept the two patches separate since the one without tests
should be applied to the stable branch (since it has no test), but this
patch should be applied to master.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: parse-test.patch --]
[-- Type: text/x-diff, Size: 1044 bytes --]

diff --git a/test/lisp/calendar/parse-time-tests.el b/test/lisp/calendar/parse-time-tests.el
index 9bcf2b4a53c7..6b2b7af0694b 100644
--- a/test/lisp/calendar/parse-time-tests.el
+++ b/test/lisp/calendar/parse-time-tests.el
@@ -42,7 +42,15 @@
   (should (equal (parse-time-string "Monday, 22 february 2016 19:35:42 +0100")
                  '(42 35 19 22 2 2016 1 nil 3600)))
   (should (equal (parse-time-string "Monday, 22 february 2016 19:35:42 PDT")
-                 '(42 35 19 22 2 2016 1 t -25200))))
+                 '(42 35 19 22 2 2016 1 t -25200)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-0200")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02:00")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54+02")
+                 '(13818 19266))))
 
 (provide 'parse-time-tests)
 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-02  6:13       ` Hong Xu
@ 2016-12-12  0:37         ` Hong Xu
  2016-12-12 17:22           ` Matt Armstrong
  0 siblings, 1 reply; 13+ messages in thread
From: Hong Xu @ 2016-12-12  0:37 UTC (permalink / raw)
  To: Matt Armstrong; +Cc: 25086

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


On 2016-12-01 Thu 22:13 GMT-0800, Hong Xu <hong@topbug.net> wrote:

> On 2016-12-01 Thu 21:45 GMT-0800, Matt Armstrong <marmstrong@google.com> wrote:
>
>> Hong Xu <hong@topbug.net> writes:
>>
>>>  doc/misc/emacs-mime.texi    |  3 +++
>>>  lisp/calendar/parse-time.el | 12 ++++++++----
>>>  src/editfns.c               |  5 ++---
>>>  3 files changed, 13 insertions(+), 7 deletions(-)
>>
>> Hi Hong, have you seen test/lisp/calendar/parse-time-tests.el?  It might
>> be nice to add test coverage for parse-iso8601-time-string there.
>
>
> Thanks for the reminder. A patch to add tests is attached.
>
> Note that I've kept the two patches separate since the one without tests
> should be applied to the stable branch (since it has no test), but this
> patch should be applied to master.

It's been some time. Do you still consider applying this patch?

Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-12  0:37         ` Hong Xu
@ 2016-12-12 17:22           ` Matt Armstrong
  2016-12-12 17:58             ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Matt Armstrong @ 2016-12-12 17:22 UTC (permalink / raw)
  To: Hong Xu; +Cc: 25086

Hong Xu <hong@topbug.net> writes:

> It's been some time. Do you still consider applying this patch?

Hi Hong, just so you know, I do not have commit access so I cannot apply
a patch myself.  A similar bug I filed took a month or two before a
volunteer applied the patch.  Perhaps emailing emacs-devel would
generate some interest.





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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-12 17:22           ` Matt Armstrong
@ 2016-12-12 17:58             ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2016-12-12 17:58 UTC (permalink / raw)
  To: Matt Armstrong; +Cc: 25086, hong

> From: Matt Armstrong <marmstrong@google.com>
> Date: Mon, 12 Dec 2016 09:22:53 -0800
> Cc: 25086@debbugs.gnu.org
> 
> Hong Xu <hong@topbug.net> writes:
> 
> > It's been some time. Do you still consider applying this patch?
> 
> Hi Hong, just so you know, I do not have commit access so I cannot apply
> a patch myself.  A similar bug I filed took a month or two before a
> volunteer applied the patch.  Perhaps emailing emacs-devel would
> generate some interest.

It's on my list, if no one beats me to it.





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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-02  4:04   ` Hong Xu
       [not found]     ` <qf5fum6ubn4.fsf@marmstrong-macbookpro.roam.corp.google.com>
@ 2016-12-17 14:58     ` Eli Zaretskii
  2016-12-17 20:42       ` Hong Xu
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2016-12-17 14:58 UTC (permalink / raw)
  To: Hong Xu; +Cc: 25086

> From: Hong Xu <hong@topbug.net>
> Date: Thu, 01 Dec 2016 20:04:00 -0800
> 
> +;;;###autoload
>  (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'."

Why did you want to autoload this?  The log message doesn't mention
this change at all.

> +                 '(42 35 19 22 2 2016 1 t -25200)))
> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-0200")
> +                 '(13818 33666)))
> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02:00")
> +                 '(13818 33666)))
> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02")
> +                 '(13818 33666)))
> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54+02")
> +                 '(13818 19266))))

Does this also test this part of your changes:

> @@ -204,13 +204,17 @@ parse-time-iso8601-regexp
>  	 (date-time (concat full-date "T" full-time)))
>      (list (concat "^" full-date)
>  	  (concat "T" partial-time)
> -	  (concat "Z" time-numoffset)))
> +	  (concat "Z?" time-numoffset)))

If not, could you please add tests for that?

Thanks.





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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-17 14:58     ` Eli Zaretskii
@ 2016-12-17 20:42       ` Hong Xu
  2016-12-18  3:11         ` Hong Xu
  0 siblings, 1 reply; 13+ messages in thread
From: Hong Xu @ 2016-12-17 20:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 25086


[-- Attachment #1.1: Type: text/plain, Size: 1795 bytes --]


On 2016-12-17 Sat 06:58 GMT-0800, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Hong Xu <hong@topbug.net>
>> Date: Thu, 01 Dec 2016 20:04:00 -0800
>>
>> +;;;###autoload
>>  (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'."
>
> Why did you want to autoload this?  The log message doesn't mention
> this change at all.

The attached new version now fixes this issue in the log message. It
autoloads because parse-time-string autoloads -- If parse-time-string is
useful for autoloading, there is no reason that
parse-iso8601-time-string is not useful for that.

>
>> +                 '(42 35 19 22 2 2016 1 t -25200)))
>> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-0200")
>> +                 '(13818 33666)))
>> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02:00")
>> +                 '(13818 33666)))
>> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02")
>> +                 '(13818 33666)))
>> +  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54+02")
>> +                 '(13818 19266))))
>
> Does this also test this part of your changes:
>
>> @@ -204,13 +204,17 @@ parse-time-iso8601-regexp
>>  	 (date-time (concat full-date "T" full-time)))
>>      (list (concat "^" full-date)
>>  	  (concat "T" partial-time)
>> -	  (concat "Z" time-numoffset)))
>> +	  (concat "Z?" time-numoffset)))
>
> If not, could you please add tests for that?
>

I've added the tests now. And indeed, it exposes a bug in the original patch.

Again, I decomposed the patch to two different parts, one for the stable
branch and the other (with only test files) for the master branch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Fix-the-timezone-detection-of-parse-iso8601-time-str.patch --]
[-- Type: text/x-diff, Size: 4446 bytes --]

From 39e4d94da9b8806871cb2c9145cd1f14233526f5 Mon Sep 17 00:00:00 2001
From: Hong Xu <hong@topbug.net>
Date: Sat, 17 Dec 2016 12:07:45 -0800
Subject: [PATCH] Fix the timezone detection of parse-iso8601-time-string.

        * parse-time.el (parse-iso8601-time-string): Fix its timezone
        parsing and make it autoload.
        * parse-time.el: Add doc for parse-iso8601-time-string and
        make it autoload.
        * editfns.c (Fdecode-time): Minor doc improvement for decode-time.
        * emacs-mime.texi (time-date): Add an example for
        parse-iso8601-time-string.
---
 doc/misc/emacs-mime.texi    |  3 +++
 lisp/calendar/parse-time.el | 19 ++++++++++++-------
 src/editfns.c               |  5 ++---
 3 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/doc/misc/emacs-mime.texi b/doc/misc/emacs-mime.texi
index 4d68246bba4b..8d6536265416 100644
--- a/doc/misc/emacs-mime.texi
+++ b/doc/misc/emacs-mime.texi
@@ -1536,6 +1536,9 @@ time-date
 (date-to-time "Sat Sep 12 12:21:54 1998 +0200")
 @result{} (13818 19266)
 
+(parse-iso8601-time-string "1998-09-12T12:21:54+0200")
+@result{} (13818 19266)
+
 (float-time '(13818 19266))
 @result{} 905595714.0
 
diff --git a/lisp/calendar/parse-time.el b/lisp/calendar/parse-time.el
index 6ba26a4a00d0..6bd748cbe0d9 100644
--- a/lisp/calendar/parse-time.el
+++ b/lisp/calendar/parse-time.el
@@ -195,7 +195,7 @@ parse-time-iso8601-regexp
 	 (time-minute 2digit)
 	 (time-second 2digit)
 	 (time-secfrac "\\(\\.[0-9]+\\)?")
-	 (time-numoffset (concat "[-+]\\(" time-hour "\\):" time-minute))
+	 (time-numoffset (concat "\\([-+]" time-hour "\\):?" time-minute "?"))
 	 (time-offset (concat "Z" time-numoffset))
 	 (partial-time (concat time-hour colon time-minute colon time-second
 			       time-secfrac))
@@ -204,19 +204,24 @@ parse-time-iso8601-regexp
 	 (date-time (concat full-date "T" full-time)))
     (list (concat "^" full-date)
 	  (concat "T" partial-time)
-	  (concat "Z" time-numoffset)))
+	  (concat "Z?" time-numoffset)))
   "List of regular expressions matching ISO 8601 dates.
 1st regular expression matches the date.
 2nd regular expression matches the time.
 3rd regular expression matches the (optional) timezone specification.")
 
+;;;###autoload
 (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'."
   (let* ((date-re (nth 0 parse-time-iso8601-regexp))
 	 (time-re (nth 1 parse-time-iso8601-regexp))
 	 (tz-re (nth 2 parse-time-iso8601-regexp))
-	 re-start
-	 time seconds minute hour fractional-seconds
-	 day month year day-of-week dst tz)
+         (tz 0)
+         re-start
+         time seconds minute hour fractional-seconds
+         day month year day-of-week dst)
     ;; We need to populate 'time' with
     ;; (SEC MIN HOUR DAY MON YEAR DOW DST TZ)
 
@@ -235,10 +240,10 @@ parse-iso8601-time-string
                                                     "0"))
 	      re-start (match-end 0))
 	(when (string-match tz-re date-string re-start)
-	  (setq tz (match-string 1 date-string)))
+          (setq tz (* 3600 (string-to-number (match-string 1 date-string)))))
 	(setq time (list seconds minute hour day month year day-of-week dst tz))))
 
-    ;; Fall back to having Gnus do fancy things for us.
+    ;; Fall back to having `parse-time-string' do fancy things for us.
     (when (not time)
       (setq time (parse-time-string date-string)))
 
diff --git a/src/editfns.c b/src/editfns.c
index 5cc4a67ab19b..2f1f412d9e4e 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2101,9 +2101,8 @@ format_time_string (char const *format, ptrdiff_t formatlen,
 
 DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 2, 0,
        doc: /* Decode a time value as (SEC MINUTE HOUR DAY MONTH YEAR DOW DST UTCOFF).
-The optional SPECIFIED-TIME should be a list of (HIGH LOW . IGNORED),
-as from `current-time' and `file-attributes', or nil to use the
-current time.
+The optional TIME should be a list of (HIGH LOW . IGNORED), as from
+`current-time' and `file-attributes', or nil to use the current time.
 It can also be a single integer number of seconds since the epoch.
 The obsolete form (HIGH . LOW) is also still accepted.
 The optional ZONE is omitted or nil for Emacs local time, t for
-- 
2.1.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0001-Add-tests-for-parse-iso8601-time-string.patch --]
[-- Type: text/x-diff, Size: 1679 bytes --]

From 9f06f66c4d5970c4cf692956544456ab4458f3b1 Mon Sep 17 00:00:00 2001
From: Hong Xu <hong@topbug.net>
Date: Sat, 17 Dec 2016 12:38:26 -0800
Subject: [PATCH] Add tests for parse-iso8601-time-string.

	* parse-time-tests.el (parse-time-tests): Add tests for
	parse-iso8601-time-string.
---
 test/lisp/calendar/parse-time-tests.el | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/test/lisp/calendar/parse-time-tests.el b/test/lisp/calendar/parse-time-tests.el
index 9bcf2b4a53c7..7a087d0f0bee 100644
--- a/test/lisp/calendar/parse-time-tests.el
+++ b/test/lisp/calendar/parse-time-tests.el
@@ -42,7 +42,19 @@
   (should (equal (parse-time-string "Monday, 22 february 2016 19:35:42 +0100")
                  '(42 35 19 22 2 2016 1 nil 3600)))
   (should (equal (parse-time-string "Monday, 22 february 2016 19:35:42 PDT")
-                 '(42 35 19 22 2 2016 1 t -25200))))
+                 '(42 35 19 22 2 2016 1 t -25200)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-0200")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02:00")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54+02")
+                 '(13818 19266)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54Z")
+                 '(13818 26466)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54")
+                 '(13818 26466))))
 
 (provide 'parse-time-tests)
 
-- 
2.1.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-17 20:42       ` Hong Xu
@ 2016-12-18  3:11         ` Hong Xu
  2016-12-24 12:41           ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Hong Xu @ 2016-12-18  3:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 25086


[-- Attachment #1.1: Type: text/plain, Size: 362 bytes --]


On 2016-12-17 Sat 12:42 GMT-0800, Hong Xu <hong@topbug.net> wrote:
>
> I've added the tests now. And indeed, it exposes a bug in the original patch.
>
> Again, I decomposed the patch to two different parts, one for the stable
> branch and the other (with only test files) for the master branch.
>

Attached is a newer version which has fixed a few edge cases.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Fix-the-timezone-detection-of-parse-iso8601-time-str.patch --]
[-- Type: text/x-diff, Size: 4837 bytes --]

From a7050f2785a74c6669dae556fa9a1deae84081a8 Mon Sep 17 00:00:00 2001
From: Hong Xu <hong@topbug.net>
Date: Sat, 17 Dec 2016 12:07:45 -0800
Subject: [PATCH] Fix the timezone detection of parse-iso8601-time-string.

        * parse-time.el (parse-iso8601-time-string): Fix its timezone
        parsing and make it autoload.
        * parse-time.el: Add doc for parse-iso8601-time-string.
        * editfns.c (Fdecode-time): Minor doc improvement for decode-time.
        * emacs-mime.texi (time-date): Add an example for
        parse-iso8601-time-string.
---
 doc/misc/emacs-mime.texi    |  3 +++
 lisp/calendar/parse-time.el | 27 ++++++++++++++++++++-------
 src/editfns.c               |  5 ++---
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/doc/misc/emacs-mime.texi b/doc/misc/emacs-mime.texi
index 4d68246bba4b..8d6536265416 100644
--- a/doc/misc/emacs-mime.texi
+++ b/doc/misc/emacs-mime.texi
@@ -1536,6 +1536,9 @@ time-date
 (date-to-time "Sat Sep 12 12:21:54 1998 +0200")
 @result{} (13818 19266)
 
+(parse-iso8601-time-string "1998-09-12T12:21:54+0200")
+@result{} (13818 19266)
+
 (float-time '(13818 19266))
 @result{} 905595714.0
 
diff --git a/lisp/calendar/parse-time.el b/lisp/calendar/parse-time.el
index 6ba26a4a00d0..be1107ee3d29 100644
--- a/lisp/calendar/parse-time.el
+++ b/lisp/calendar/parse-time.el
@@ -195,7 +195,7 @@ parse-time-iso8601-regexp
 	 (time-minute 2digit)
 	 (time-second 2digit)
 	 (time-secfrac "\\(\\.[0-9]+\\)?")
-	 (time-numoffset (concat "[-+]\\(" time-hour "\\):" time-minute))
+	 (time-numoffset (concat "\\([-+]\\)" time-hour ":?" time-minute "?"))
 	 (time-offset (concat "Z" time-numoffset))
 	 (partial-time (concat time-hour colon time-minute colon time-second
 			       time-secfrac))
@@ -204,19 +204,23 @@ parse-time-iso8601-regexp
 	 (date-time (concat full-date "T" full-time)))
     (list (concat "^" full-date)
 	  (concat "T" partial-time)
-	  (concat "Z" time-numoffset)))
+	  (concat "\\(Z\\|" time-numoffset "\\)")))
   "List of regular expressions matching ISO 8601 dates.
 1st regular expression matches the date.
 2nd regular expression matches the time.
 3rd regular expression matches the (optional) timezone specification.")
 
+;;;###autoload
 (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'."
   (let* ((date-re (nth 0 parse-time-iso8601-regexp))
 	 (time-re (nth 1 parse-time-iso8601-regexp))
 	 (tz-re (nth 2 parse-time-iso8601-regexp))
-	 re-start
-	 time seconds minute hour fractional-seconds
-	 day month year day-of-week dst tz)
+         re-start
+         time seconds minute hour fractional-seconds
+         day month year day-of-week dst tz)
     ;; We need to populate 'time' with
     ;; (SEC MIN HOUR DAY MON YEAR DOW DST TZ)
 
@@ -235,10 +239,19 @@ parse-iso8601-time-string
                                                     "0"))
 	      re-start (match-end 0))
 	(when (string-match tz-re date-string re-start)
-	  (setq tz (match-string 1 date-string)))
+          (if (string= "Z" (match-string 1 date-string))
+              (setq tz 0)  ;; UTC timezone indicated by Z
+            (setq tz (+
+                      (* 3600
+                         (string-to-number (match-string 3 date-string)))
+                      (* 60
+                         (string-to-number
+                          (or (match-string 4 date-string) "0")))))
+            (when (string= "-" (match-string 2 date-string))
+              (setq tz (- tz)))))
 	(setq time (list seconds minute hour day month year day-of-week dst tz))))
 
-    ;; Fall back to having Gnus do fancy things for us.
+    ;; Fall back to having `parse-time-string' do fancy things for us.
     (when (not time)
       (setq time (parse-time-string date-string)))
 
diff --git a/src/editfns.c b/src/editfns.c
index 5cc4a67ab19b..2f1f412d9e4e 100644
--- a/src/editfns.c
+++ b/src/editfns.c
@@ -2101,9 +2101,8 @@ format_time_string (char const *format, ptrdiff_t formatlen,
 
 DEFUN ("decode-time", Fdecode_time, Sdecode_time, 0, 2, 0,
        doc: /* Decode a time value as (SEC MINUTE HOUR DAY MONTH YEAR DOW DST UTCOFF).
-The optional SPECIFIED-TIME should be a list of (HIGH LOW . IGNORED),
-as from `current-time' and `file-attributes', or nil to use the
-current time.
+The optional TIME should be a list of (HIGH LOW . IGNORED), as from
+`current-time' and `file-attributes', or nil to use the current time.
 It can also be a single integer number of seconds since the epoch.
 The obsolete form (HIGH . LOW) is also still accepted.
 The optional ZONE is omitted or nil for Emacs local time, t for
-- 
2.1.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0001-Add-tests-for-parse-iso8601-time-string.patch --]
[-- Type: text/x-diff, Size: 1921 bytes --]

From f53bed4fa277af83c5194854c092687a1002d709 Mon Sep 17 00:00:00 2001
From: Hong Xu <hong@topbug.net>
Date: Sat, 17 Dec 2016 12:38:26 -0800
Subject: [PATCH] Add tests for parse-iso8601-time-string.

	* parse-time-tests.el (parse-time-tests): Add tests for
	parse-iso8601-time-string.
---
 test/lisp/calendar/parse-time-tests.el | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/test/lisp/calendar/parse-time-tests.el b/test/lisp/calendar/parse-time-tests.el
index 9bcf2b4a53c7..6dc23372f249 100644
--- a/test/lisp/calendar/parse-time-tests.el
+++ b/test/lisp/calendar/parse-time-tests.el
@@ -42,7 +42,23 @@
   (should (equal (parse-time-string "Monday, 22 february 2016 19:35:42 +0100")
                  '(42 35 19 22 2 2016 1 nil 3600)))
   (should (equal (parse-time-string "Monday, 22 february 2016 19:35:42 PDT")
-                 '(42 35 19 22 2 2016 1 t -25200))))
+                 '(42 35 19 22 2 2016 1 t -25200)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-0200")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-0230")
+                 '(13818 35466)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02:00")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54-02")
+                 '(13818 33666)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54+0230")
+                 '(13818 17466)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54+02")
+                 '(13818 19266)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54Z")
+                 '(13818 26466)))
+  (should (equal (parse-iso8601-time-string "1998-09-12T12:21:54")
+                 (encode-time 54 21 12 12 9 1998))))
 
 (provide 'parse-time-tests)
 
-- 
2.1.4


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-18  3:11         ` Hong Xu
@ 2016-12-24 12:41           ` Eli Zaretskii
  2016-12-24 21:04             ` Hong Xu
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2016-12-24 12:41 UTC (permalink / raw)
  To: Hong Xu; +Cc: 25086-done

> From: Hong Xu <hong@topbug.net>
> Cc: 25086@debbugs.gnu.org
> Date: Sat, 17 Dec 2016 19:11:58 -0800
> 
> On 2016-12-17 Sat 12:42 GMT-0800, Hong Xu <hong@topbug.net> wrote:
> >
> > I've added the tests now. And indeed, it exposes a bug in the original patch.
> >
> > Again, I decomposed the patch to two different parts, one for the stable
> > branch and the other (with only test files) for the master branch.
> 
> Attached is a newer version which has fixed a few edge cases.

Thanks, pushed to master.

I didn't make parse-iso8601-time-string autoloaded.  I don't think
it's needed; if you think it is, please tell why.

Also, a minor not about log messages:

>         * parse-time.el (parse-iso8601-time-string): Fix its timezone
>         parsing and make it autoload.
>         * parse-time.el: Add doc for parse-iso8601-time-string.

These are changes in the same function and the same source file.  So
they should be written together, like this:

	* parse-time.el (parse-iso8601-time-string): Fix timezone
	parsing.  Add a doc string.

Thank you for your contribution and for your patience.





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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-24 12:41           ` Eli Zaretskii
@ 2016-12-24 21:04             ` Hong Xu
  2016-12-25  3:29               ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Hong Xu @ 2016-12-24 21:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 25086-done

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


On 2016-12-24 Sat 04:41 GMT-0800, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Hong Xu <hong@topbug.net>
>> Cc: 25086@debbugs.gnu.org
>> Date: Sat, 17 Dec 2016 19:11:58 -0800
>>
>> On 2016-12-17 Sat 12:42 GMT-0800, Hong Xu <hong@topbug.net> wrote:
>> >
>> > I've added the tests now. And indeed, it exposes a bug in the original patch.
>> >
>> > Again, I decomposed the patch to two different parts, one for the stable
>> > branch and the other (with only test files) for the master branch.
>>
>> Attached is a newer version which has fixed a few edge cases.
>
> Thanks, pushed to master.
>

Thank you. Could you backport it to the stable branch? It is indeed a
bug fix in my point of view -- no new feature is added.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

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

* bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string.
  2016-12-24 21:04             ` Hong Xu
@ 2016-12-25  3:29               ` Eli Zaretskii
  0 siblings, 0 replies; 13+ messages in thread
From: Eli Zaretskii @ 2016-12-25  3:29 UTC (permalink / raw)
  To: Hong Xu; +Cc: 25086

> From: Hong Xu <hong@topbug.net>
> Cc: 25086-done@debbugs.gnu.org
> Date: Sat, 24 Dec 2016 13:04:05 -0800
> 
> > Thanks, pushed to master.
> >
> 
> Thank you. Could you backport it to the stable branch? It is indeed a
> bug fix in my point of view -- no new feature is added.

Sorry, it's too late for the release branch.  It only gets bugfixes
for issues introduced by the last two releases of Emacs, and this one
wasn't, AFAIU.





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

end of thread, other threads:[~2016-12-25  3:29 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-02  3:46 bug#25086: [PATCH] Fix the timezone detection of parse-iso8601-time-string Hong Xu
2016-12-02  3:55 ` Hong Xu
2016-12-02  4:04   ` Hong Xu
     [not found]     ` <qf5fum6ubn4.fsf@marmstrong-macbookpro.roam.corp.google.com>
2016-12-02  6:13       ` Hong Xu
2016-12-12  0:37         ` Hong Xu
2016-12-12 17:22           ` Matt Armstrong
2016-12-12 17:58             ` Eli Zaretskii
2016-12-17 14:58     ` Eli Zaretskii
2016-12-17 20:42       ` Hong Xu
2016-12-18  3:11         ` Hong Xu
2016-12-24 12:41           ` Eli Zaretskii
2016-12-24 21:04             ` Hong Xu
2016-12-25  3:29               ` 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).