{"thread":{"id":"23832","subject":"PATCH: Improved support for ISO 8601 timezones","startedAt":"2010-05-17T19:07:09Z","lastAt":"2010-05-19T17:21:51Z","messageCount":6,"participants":["Marcus Comstedt","Jay Soffian","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"141834","messageId":"1274123231-18482-1-git-send-email-marcus@mc.pp.se","threadId":"23832","inReplyTo":null,"subject":"PATCH: Improved support for ISO 8601 timezones","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-05-17T19:07:09Z","receivedAt":"2010-05-17T19:07:09Z","isPatch":false,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"Hi.\n\nI discovered that git's date parser does not understand \"Z\" to mean\nthe \"UTC\" timezone. This is unfortunate, because the use of \"Z\" is\nprescribed by ISO 8601.\n\nI made a small patch to add \"Z\" as an alias for \"UTC\", which enables\nstandard ISO 8601 timestamps to be parsed correctly. Also, it fixes\na bug that at least three characters of the timezone name had to match,\nwhich is of course impossible when the name of the timezone is shorter\nthan three characters. There was already such a timezone before (\"NT\")\nwhich could not be selected due to the bug.\n\nThe second patch, which is perhaps less essential, adds support for\nthe remaining numerical timezone indicators defined by ISO 8601 not\nalready supported by git (only +-hhmm was supported, but ISO 8601\nalso specifies that +-hh:mm and +-hh are ok as well).\n\nThanks\n\n\n // Marcus\n"},{"id":"141835","messageId":"1274123231-18482-2-git-send-email-marcus@mc.pp.se","threadId":"23832","inReplyTo":"1274123231-18482-1-git-send-email-marcus@mc.pp.se","subject":"[PATCH 1/2] Added \"Z\" as an alias for the timezone \"UTC\"","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-05-17T19:07:10Z","receivedAt":"2010-05-17T19:07:10Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"The name \"Z\" for the UTC timezone is required to properly parse\nISO 8601 times. Added it to the list of recignozed timezones.\n\nAlso, fixed the bug that timezone names shorter than 3 characters\ncan never be matched by match_alpha(). Prior to the introduction\nof the \"Z\" zone, this affected the timezone \"NT\" (Nome).\n\nSigned-off-by: Marcus Comstedt \n---\n:100644 100644 002aa3c... 6bae49c... M\tdate.c\n date.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 002aa3c..6bae49c 100644\n--- a/date.c\n+++ b/date.c\n@@ -229,6 +229,7 @@ static const struct {\n \n \t{ \"GMT\", 0, 0, },\t/* Greenwich Mean */\n \t{ \"UTC\", 0, 0, },\t/* Universal (Coordinated) */\n+\t{ \"Z\", 0, 0, }, /* Zulu, alias for UTC */\n \n \t{ \"WET\", 0, 0, },\t/* Western European */\n \t{ \"BST\", 0, 1, },\t/* British Summer */\n@@ -305,7 +306,7 @@ static int match_alpha(const char *date, struct tm *tm, int *offset)\n \n \tfor (i = 0; i < ARRAY_SIZE(timezone_names); i++) {\n \t\tint match = match_string(date, timezone_names[i].name);\n-\t\tif (match >= 3) {\n+\t\tif (match >= 3 || match == strlen(timezone_names[i].name)) {\n \t\t\tint off = timezone_names[i].offset;\n \n \t\t\t/* This is bogus, but we like summer */\n-- \n1.7.0.4\n"},{"id":"141836","messageId":"1274123231-18482-3-git-send-email-marcus@mc.pp.se","threadId":"23832","inReplyTo":"1274123231-18482-1-git-send-email-marcus@mc.pp.se","subject":"[PATCH 2/2] Accept the timezone specifiers [+-]hh:mm and [+-]hh in addition to [+-]hhmm","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-05-17T19:07:11Z","receivedAt":"2010-05-17T19:07:11Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"ISO 8601 specifies three syntaxes for timezones other than \"Z\".\ngit already supports the +-hhmm syntax. This patch adds support\nfor the other two: +-hh:mm and +-hh.\n\nSigned-off-by: Marcus Comstedt \n---\n:100644 100644 6bae49c... f83e46e... M\tdate.c\n date.c | 23 +++++++++++++++++++++++\n 1 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex 6bae49c..f83e46e 100644\n--- a/date.c\n+++ b/date.c\n@@ -555,6 +555,18 @@ static int match_tz(const char *date, int *offp)\n \tint min, hour;\n \tint n = end - date - 1;\n \n+\t/* Check for HH:MM format, allowed by ISO 8601 */\n+\tif (n == 2 && date[3] == ':') {\n+\t\tchar *end2;\n+\t\tmin = strtoul(date+4, &end2, 10);\n+\t\t/* If we have two digits after the colon too, assume HH:MM */\n+\t\tif (end2 == date+6) {\n+\t\t\toffset = offset*100 + min;\n+\t\t\tend = end2;\n+\t\t\tn = end - date - 1;\n+\t\t}\n+\t}\n+\n \tmin = offset % 100;\n \thour = offset / 100;\n \n@@ -570,6 +582,17 @@ static int match_tz(const char *date, int *offp)\n \n \t\t*offp = offset;\n \t}\n+\t/*\n+\t * Also accept just the hour, allowed by ISO 8601\n+\t */\n+\telse if (n == 2 && hour == 0 && min < 24) {\n+\t\toffset = min*60;\n+\t\tif (*date == '-')\n+\t\t\toffset = -offset;\n+\n+\t\t*offp = offset;\n+\t}\n+\n \treturn end - date;\n }\n \n-- \n1.7.0.4\n"},{"id":"141839","messageId":"AANLkTiljyxwlwGK4i-9QKGSW4T_-v2HtFkcu16a7Ndam@mail.gmail.com","threadId":"23832","inReplyTo":"1274123231-18482-2-git-send-email-marcus@mc.pp.se","subject":"Re: [PATCH 1/2] Added \"Z\" as an alias for the timezone \"UTC\"","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-05-17T20:32:57Z","receivedAt":"2010-05-17T20:32:57Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Mon, May 17, 2010 at 3:07 PM, Marcus Comstedt wrote:\n> The name \"Z\" for the UTC timezone is required to properly parse\n> ISO 8601 times.  Added it to the list of recignozed timezones.\n\ns/Added/Add/; s/recignozed/recognized/\n\n> Also, fixed the bug that timezone names shorter than 3 characters\n\ns/fixed the/fix a/\n\nj.\n"},{"id":"141907","messageId":"7v632karpe.fsf@alter.siamese.dyndns.org","threadId":"23832","inReplyTo":"1274123231-18482-3-git-send-email-marcus@mc.pp.se","subject":"Re: [PATCH 2/2] Accept the timezone specifiers [+-]hh:mm and [+-]hh in addition to [+-]hhmm","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-05-19T14:31:25Z","receivedAt":"2010-05-19T14:31:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Marcus Comstedt writes:\n\n> ISO 8601 specifies three syntaxes for timezones other than \"Z\".\n> git already supports the +-hhmm syntax. This patch adds support\n> for the other two: +-hh:mm and +-hh.\n>\n> Signed-off-by: Marcus Comstedt \n> ---\n> :100644 100644 6bae49c... f83e46e... M\tdate.c\n> date.c | 23 +++++++++++++++++++++++\n> 1 files changed, 23 insertions(+), 0 deletions(-)\n>\n> diff --git a/date.c b/date.c\n> index 6bae49c..f83e46e 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -555,6 +555,18 @@ static int match_tz(const char *date, int *offp)\n> \tint min, hour;\n> \tint n = end - date - 1;\n> \n> +\t/* Check for HH:MM format, allowed by ISO 8601 */\n> +\tif (n == 2 && date[3] == ':') {\n> +\t\tchar *end2;\n> +\t\tmin = strtoul(date+4, &end2, 10);\n> +\t\t/* If we have two digits after the colon too, assume HH:MM */\n> +\t\tif (end2 == date+6) {\n> +\t\t\toffset = offset*100 + min;\n> +\t\t\tend = end2;\n> +\t\t\tn = end - date - 1;\n> +\t\t}\n> +\t}\n> +\n> \tmin = offset % 100;\n> \thour = offset / 100;\n> \n> @@ -570,6 +582,17 @@ static int match_tz(const char *date, int *offp)\n> \n> \t\t*offp = offset;\n> \t}\n> +\t/*\n> +\t * Also accept just the hour, allowed by ISO 8601\n> +\t */\n> +\telse if (n == 2 && hour == 0 && min < 24) {\n> +\t\toffset = min*60;\n> +\t\tif (*date == '-')\n> +\t\t\toffset = -offset;\n> +\n> +\t\t*offp = offset;\n> +\t}\n> +\n\nI don't recall seeing in ISO 8601 that +hh or -hh without minute\nresolution was allowed, but I don't have my copy of ISO 8601 with me (they\nare packed and are still in transit with my household goods) so I'll take\nyour word for it for now [*1*].\n\nBut the placement of this second hunk is somewhat curious. Why doesn't the\nupdated function look like this?\n\n int offset = strtoul(date + 1, &end, 10);\n int min, hour;\n int n = end - date - 1;\n\n if (n == 2 && offset <= 14) {\n /* +HH:MM (ISO 8601) or +HH (ISO 8601 abbreviated) */\n hour = offset;\n if (n == 2 && date[3] == ':') {\n min = strtoul(date + 4, &end, 10);\n if (end != date + 6)\n return 0; /* Bad CRAP */\n } else {\n min = 0;\n }\n } else if (n < 3) {\n return 0; /* we want at least 3 digits */\n } else {\n min = offset % 100;\n hour = offset / 100;\n }\n\n if (60 <= min)\n return 0; /* invalid minute */\n\n offset = hour * 60 + min;\n if (*date == '-')\n offset = -offset;\n *offp = offset;\n return end - date;\n\n\n[Footnote]\n\n*1* Appendix A of RFC3339 seems to agree with you.\n"},{"id":"141915","messageId":"yf9ocgbkdsg.fsf@chiyo.mc.pp.se","threadId":"23832","inReplyTo":"7v632karpe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Accept the timezone specifiers [+-]hh:mm and [+-]hh in addition to [+-]hhmm","fromName":"Marcus Comstedt","fromEmail":"marcus@mc.pp.se","sentAt":"2010-05-19T17:21:51Z","receivedAt":"2010-05-19T17:21:51Z","isPatch":true,"sender":{"key":"marcus@mc.pp.se","avatar":"https://avatars.githubusercontent.com/u/411296?v=4"},"body":"\nHi Junio.\n\nThanks for reviewing this patch.\n\n\nJunio C Hamano writes:\n\n> I don't recall seeing in ISO 8601 that +hh or -hh without minute\n> resolution was allowed, but I don't have my copy of ISO 8601 with me (they\n> are packed and are still in transit with my household goods) so I'll take\n> your word for it for now [*1*].\n\nIn the final draft of 8601:2000 (which is the only version I have),\nsection 5.3.4.1 states that \"[...] the representation of the difference\ncan be expressed in hours and minutes, or hours only.\" Examples of\nthis then follow in that section and the next one. Maybe they changed\nit in the final version (or it differs from another release of the\nstandard)? I wish you could \"git log -S\" ISO standards... :-)\nWikipedia also agrees that it is allowed by the standard though.\n\n\n> But the placement of this second hunk is somewhat curious. Why doesn't the\n> updated function look like this?\n[...]\n\nI was perhaps treading a bit over-cautiously. The placement allowed\nme to leave the existing code both syntactically and semantically\nunaltered. After all, there was nothing wrong with the old code per\nse, I was just adding new functionality. I also wanted the two\nchanges independent, in case you wanted one but not the other.\n\nI can concede that your variant leaves a more appealing end result\nthough. (Except for the fact that \"n == 2\" is needlessly tested in\nthe inner if. ;)\n\nOne thing though: Shouldn't 1 be returned for bad crap rather than 0?\nSeems to me parse_date will get stuck otherwise, because the sign will\nnever be consumed. In fact, the old code would consume both the sign\nand the initial sequence of digits in the crap case. Consuming just\nthe sign would leave the digits to be handled by match_digit, which\nmay or may not regard it as non-crap. Good or bad, I don't know. But\nit might cause regressions.\n\nI'll play around a little with the code and perform some new unit\ntests, and then resubmit a new patch with the suggested structure.\n\n\n // Marcus\n"}]}