{"thread":{"id":"53221","subject":"Mishandling of fractional seconds in ISO 8601 format","startedAt":"2020-04-14T00:04:08Z","lastAt":"2020-04-24T20:33:52Z","messageCount":49,"participants":["brian m. carlson","Đoàn Trần Công Danh","Jeff King","Danh Doan","Torsten Bögershausen","Junio C Hamano","Philip Oakley"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"395382","messageId":"20200414000324.GA2751707@camp.crustytoothpaste.net","threadId":"53221","inReplyTo":null,"subject":"Mishandling of fractional seconds in ISO 8601 format","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-04-14T00:03:24Z","receivedAt":"2020-04-14T00:04:08Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"git-commit(1) claims we support ISO 8601 format. However, our\napproxidate code misparses some dates with fractional seconds.\n\nReproduction:\n\n git init foo\n cd foo\n echo abc > abc.txt\n git add .\n git commit --date=\"2020-04-03T12:43:55.019-04:00”\n\nThis should produce Fri Apr 3 12:43:55 2020 -0400, but actually produces\nSun Apr 19 12:43:55 2020 +0000 (at least on my system, which uses UTC).\nNote the different date, which is 16 days away from what is expected.\n\nSince we claim to support ISO 8601, we need to either reject fractional\nseconds with an error, or accept and ignore them. If what we really\nsupport is RFC 3339 (which I suspect it is), we need to do the latter,\nsince that profile explicitly permits them in the syntax, as well as\nupdate the documentation accordingly.\n\nThis was originally reported at\nhttps://stackoverflow.com/questions/61193896/how-does-git-parse-date-string/61197722.\nI don't plan to send a patch for this right now, but I wanted to make\nsure it was reported to the list.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"395399","messageId":"cover.1586856398.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"20200414000324.GA2751707@camp.crustytoothpaste.net","subject":"[PATCH 0/2] More ISO-8601 support","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-14T09:31:53Z","receivedAt":"2020-04-14T09:32:06Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-14 00:03:24+0000, \"brian m. carlson\" wrote:\n> git-commit(1) claims we support ISO 8601 format. However, our\n> approxidate code misparses some dates with fractional seconds.\n\nWe have never fully supported ISO-8601, those representation are\nperfectly valid ISO-8601 (for 2020-04-14) but it would require\nmore code to parse correctly, perhap \"tm_wday\" and \"tm_yday\",\nand mktime may help.\n\n\t2020-W16-2\n\t--04-14\n\t2020-105\n\nI don't think not many people interested in those formats.\nIf someone really need them, we can get back to them later.\n\n> Reproduction:\n> \n> git init foo\n> cd foo\n> echo abc > abc.txt\n> git add .\n> git commit --date=\"2020-04-03T12:43:55.019-04:00”\n> \n> This should produce Fri Apr 3 12:43:55 2020 -0400, but actually produces\n> Sun Apr 19 12:43:55 2020 +0000 (at least on my system, which uses UTC).\n> Note the different date, which is 16 days away from what is expected.\n> \n> Since we claim to support ISO 8601, we need to either reject fractional\n> seconds with an error, or accept and ignore them. If what we really\n> support is RFC 3339 (which I suspect it is), we need to do the latter,\n> since that profile explicitly permits them in the syntax, as well as\n> update the documentation accordingly.\n> \n> This was originally reported at\n> https://stackoverflow.com/questions/61193896/how-does-git-parse-date-string/61197722.\n> I don't plan to send a patch for this right now, but I wanted to make\n\nIf we plan on supporting more format from ISO-8601, these 2 patches\ncan be used.\n\nĐoàn Trần Công Danh (2):\n date.c: allow fractional second part of ISO-8601\n date.c: allow compact version of ISO-8601 datetime\n\n date.c | 20 ++++++++++++++++++++\n t/t0006-date.sh | 4 ++++\n 2 files changed, 24 insertions(+)\n\n-- \n2.26.0.485.g518673e55f\n\n"},{"id":"395400","messageId":"45676f4a7d001dbd25bbae531cc2ae5fc53936bb.1586856398.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586856398.git.congdanhqx@gmail.com","subject":"[PATCH 1/2] date.c: allow fractional second part of ISO-8601","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-14T09:31:54Z","receivedAt":"2020-04-14T09:32:09Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"git-commit(1) says ISO-8601 is one of our supported date format.\nHowever, we only support RFC-3339 date format.\n\nWe can either:\n- Update documentation from ISO-8601 to RFC-3339\n- Add full support for ISO-8601\n\nThis series will try to add full support for ISO-8601.\n\nReported-by: Brian M. Carlson \nSigned-off-by: Đoàn Trần Công Danh \n---\n date.c | 2 ++\n t/t0006-date.sh | 1 +\n 2 files changed, 3 insertions(+)\n\ndiff --git a/date.c b/date.c\nindex b0d9a8421d..2f37397beb 100644\n--- a/date.c\n+++ b/date.c\n@@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \tcase ':':\n \t\tif (num3 < 0)\n \t\t\tnum3 = 0;\n+\t\telse if (*end == '.' && isdigit(end[1]))\n+\t\t\tstrtol(end + 1, &end, 10);\n \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n \t\t\ttm->tm_hour = num;\n \t\t\ttm->tm_min = num2;\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex d9fcc829a9..05c914a200 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -81,6 +81,7 @@ check_parse 2008-02 bad\n check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n-- \n2.26.0.485.g518673e55f\n\n"},{"id":"395401","messageId":"06e62c58d5accad7bbebbc51f9fb38fda83a73f6.1586856398.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586856398.git.congdanhqx@gmail.com","subject":"[PATCH 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-14T09:31:55Z","receivedAt":"2020-04-14T09:32:12Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh \n---\n date.c | 18 ++++++++++++++++++\n t/t0006-date.sh | 3 +++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/date.c b/date.c\nindex 2f37397beb..e02d8e191a 100644\n--- a/date.c\n+++ b/date.c\n@@ -666,6 +666,24 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n \t\tn++;\n \t} while (isdigit(date[n]));\n \n+\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n+\tif (n == 8) {\n+\t\ttm->tm_year = num / 10000 - 1900;\n+\t\ttm->tm_mon = (num % 10000) / 100 - 1;\n+\t\ttm->tm_mday = num % 100;\n+\t\treturn n;\n+\t}\n+\n+\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n+\tif (n == 6) {\n+\t\ttm->tm_hour = num / 10000;\n+\t\ttm->tm_min = (num % 10000) / 100;\n+\t\ttm->tm_sec = num % 100;\n+\t\tif (*end == '.' && isdigit(end[1]))\n+\t\t\tstrtoul(end + 1, &end, 10);\n+\t\treturn end - date;\n+\t}\n+\n \t/* Four-digit year or a timezone? */\n \tif (n == 4) {\n \t\tif (num <= 1400 && *offset == -1) {\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 05c914a200..eeb11070a6 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -81,6 +81,9 @@ check_parse 2008-02 bad\n check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n-- \n2.26.0.485.g518673e55f\n\n"},{"id":"395437","messageId":"20200414201606.GA1879688@coredump.intra.peff.net","threadId":"53221","inReplyTo":"45676f4a7d001dbd25bbae531cc2ae5fc53936bb.1586856398.git.congdanhqx@gmail.com","subject":"Re: [PATCH 1/2] date.c: allow fractional second part of ISO-8601","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-14T20:16:06Z","receivedAt":"2020-04-14T20:17:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 14, 2020 at 04:31:54PM +0700, Đoàn Trần Công Danh wrote:\n\n> git-commit(1) says ISO-8601 is one of our supported date format.\n> However, we only support RFC-3339 date format.\n> \n> We can either:\n> - Update documentation from ISO-8601 to RFC-3339\n> - Add full support for ISO-8601\n> \n> This series will try to add full support for ISO-8601.\n\nThat seems more like a cover letter for the full series. The important\nthing in this patch is more like:\n\n ISO-8601 allows timestamps to have a fractional number of seconds. We\n represent time only in terms of whole seconds, so we never bothered\n parsing fractional seconds. However, it's better for us to parse and\n throw away the fractional part than to refuse to parse the timestamp\n at all.\n\n> diff --git a/date.c b/date.c\n> index b0d9a8421d..2f37397beb 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> \tcase ':':\n> \t\tif (num3 < 0)\n> \t\t\tnum3 = 0;\n> +\t\telse if (*end == '.' && isdigit(end[1]))\n> +\t\t\tstrtol(end + 1, &end, 10);\n\nI was a bit worried about this change hurting other cases where people\nmight use dots to separate numbers (but not mean a fraction). But the\ntwo common ones I can think of should be OK:\n\n - 5.seconds.ago would never match because you look for a digit after\n the dot\n\n - a date like 2019.10.12 wouldn't match, because we're only looking\n after a \":\", which heavily implies a time.\n\nIt might be worth covering those cases in tests if we're not already\ndoing so elsewhere.\n\n-Peff\n"},{"id":"395438","messageId":"20200414201733.GB1879688@coredump.intra.peff.net","threadId":"53221","inReplyTo":"45676f4a7d001dbd25bbae531cc2ae5fc53936bb.1586856398.git.congdanhqx@gmail.com","subject":"Re: [PATCH 1/2] date.c: allow fractional second part of ISO-8601","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-14T20:17:33Z","receivedAt":"2020-04-14T20:17:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 14, 2020 at 04:31:54PM +0700, Đoàn Trần Công Danh wrote:\n\n> diff --git a/date.c b/date.c\n> index b0d9a8421d..2f37397beb 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> \tcase ':':\n> \t\tif (num3 < 0)\n> \t\t\tnum3 = 0;\n> +\t\telse if (*end == '.' && isdigit(end[1]))\n> +\t\t\tstrtol(end + 1, &end, 10);\n\nThis just throws away the fractional part. I doubt anybody cares much\neither way, but another option would be to round num3 up or down.\n\n-Peff\n"},{"id":"395440","messageId":"20200414202401.GC1879688@coredump.intra.peff.net","threadId":"53221","inReplyTo":"06e62c58d5accad7bbebbc51f9fb38fda83a73f6.1586856398.git.congdanhqx@gmail.com","subject":"Re: [PATCH 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-14T20:24:01Z","receivedAt":"2020-04-14T20:24:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Apr 14, 2020 at 04:31:55PM +0700, Đoàn Trần Công Danh wrote:\n\n> @@ -666,6 +666,24 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n> \t\tn++;\n> \t} while (isdigit(date[n]));\n> \n> +\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n> +\tif (n == 8) {\n> +\t\ttm->tm_year = num / 10000 - 1900;\n> +\t\ttm->tm_mon = (num % 10000) / 100 - 1;\n> +\t\ttm->tm_mday = num % 100;\n> +\t\treturn n;\n> +\t}\n\nI worry a little this may conflict with other approxidate heuristics.\nThe only one I can think of is an actual unix timestamp, though, and we\nalready require that to have at least 9 digits (plus anybody wanting to\nuse one robustly really should be using @12345678).\n\nAnd it looks like we'd exit early from the function for anything longer\nthan 4 digits anyway, ignoring the value.\n\nWe could probably tighten the heuristics a bit by insisting that the\nmonth and day be sensible. Or even year (see the 1900 to 2100 magic for\nthe 4-digit year guess).\n\n> +\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n> +\tif (n == 6) {\n> +\t\ttm->tm_hour = num / 10000;\n> +\t\ttm->tm_min = (num % 10000) / 100;\n> +\t\ttm->tm_sec = num % 100;\n> +\t\tif (*end == '.' && isdigit(end[1]))\n> +\t\t\tstrtoul(end + 1, &end, 10);\n> +\t\treturn end - date;\n> +\t}\n\nAnd likewise here that the hour, minute, and second be reasonable.\n\n-Peff\n"},{"id":"395454","messageId":"20200414234504.GD2751707@camp.crustytoothpaste.net","threadId":"53221","inReplyTo":"cover.1586856398.git.congdanhqx@gmail.com","subject":"Re: [PATCH 0/2] More ISO-8601 support","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-04-14T23:45:04Z","receivedAt":"2020-04-14T23:46:33Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-04-14 at 09:31:53, Đoàn Trần Công Danh wrote:\n> On 2020-04-14 00:03:24+0000, \"brian m. carlson\" wrote:\n> > git-commit(1) claims we support ISO 8601 format. However, our\n> > approxidate code misparses some dates with fractional seconds.\n> \n> We have never fully supported ISO-8601, those representation are\n> perfectly valid ISO-8601 (for 2020-04-14) but it would require\n> more code to parse correctly, perhap \"tm_wday\" and \"tm_yday\",\n> and mktime may help.\n> \n> \t2020-W16-2\n> \t--04-14\n> \t2020-105\n> \n> I don't think not many people interested in those formats.\n> If someone really need them, we can get back to them later.\n\nYeah, I agree we don't need to support those. I would be happy if we\nsaid \"RFC 3339\", which is a more limited profile, and implemented that.\nThe problem, as you alluded to (and I did as well in my original\nreport), is that ISO 8601 provides a huge number of potential formats,\nmost of which are not interesting for typical software.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"395455","messageId":"20200414234958.GE2751707@camp.crustytoothpaste.net","threadId":"53221","inReplyTo":"20200414201733.GB1879688@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] date.c: allow fractional second part of ISO-8601","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2020-04-14T23:49:58Z","receivedAt":"2020-04-14T23:50:41Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2020-04-14 at 20:17:33, Jeff King wrote:\n> On Tue, Apr 14, 2020 at 04:31:54PM +0700, Đoàn Trần Công Danh wrote:\n> \n> > diff --git a/date.c b/date.c\n> > index b0d9a8421d..2f37397beb 100644\n> > --- a/date.c\n> > +++ b/date.c\n> > @@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> > \tcase ':':\n> > \t\tif (num3 < 0)\n> > \t\t\tnum3 = 0;\n> > +\t\telse if (*end == '.' && isdigit(end[1]))\n> > +\t\t\tstrtol(end + 1, &end, 10);\n> \n> This just throws away the fractional part. I doubt anybody cares much\n> either way, but another option would be to round num3 up or down.\n\nI'm happy with truncation. It's simple and straightforward and will be\nwidely and intuitively understood by programmers, who are probably the\nprimary users of our ISO 8601 handling.\n\nI do appreciate that you pointed this out, because it's a good point.\n\nAnd overall, I agree that this seems like a good fix for the problem.\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"395463","messageId":"20200415021202.GA19025@danh.dev","threadId":"53221","inReplyTo":"20200414202401.GC1879688@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-15T02:12:02Z","receivedAt":"2020-04-15T02:12:11Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-14 16:24:01-0400, Jeff King wrote:\n> On Tue, Apr 14, 2020 at 04:31:55PM +0700, Đoàn Trần Công Danh wrote:\n> \n> > @@ -666,6 +666,24 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n> > \t\tn++;\n> > \t} while (isdigit(date[n]));\n> > \n> > +\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n> > +\tif (n == 8) {\n> > +\t\ttm->tm_year = num / 10000 - 1900;\n> > +\t\ttm->tm_mon = (num % 10000) / 100 - 1;\n> > +\t\ttm->tm_mday = num % 100;\n> > +\t\treturn n;\n> > +\t}\n> \n> I worry a little this may conflict with other approxidate heuristics.\n> The only one I can think of is an actual unix timestamp, though, and we\n> already require that to have at least 9 digits (plus anybody wanting to\n> use one robustly really should be using @12345678).\n> \n> And it looks like we'd exit early from the function for anything longer\n> than 4 digits anyway, ignoring the value.\n> \n> We could probably tighten the heuristics a bit by insisting that the\n> month and day be sensible. Or even year (see the 1900 to 2100 magic for\n> the 4-digit year guess).\n\nYeah, It's make sense to tighten the heuristics.\nWhile 1900 is lower bound for year makes sense to me,\nbut I don't think we should limit upper bound for tm_year.\n\n> > +\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n> > +\tif (n == 6) {\n> > +\t\ttm->tm_hour = num / 10000;\n> > +\t\ttm->tm_min = (num % 10000) / 100;\n> > +\t\ttm->tm_sec = num % 100;\n> > +\t\tif (*end == '.' && isdigit(end[1]))\n> > +\t\t\tstrtoul(end + 1, &end, 10);\n> > +\t\treturn end - date;\n> > +\t}\n> \n> And likewise here that the hour, minute, and second be reasonable.\n> \n> -Peff\n\n-- \nDanh\n"},{"id":"395464","messageId":"20200415021521.GB19025@danh.dev","threadId":"53221","inReplyTo":"20200414201606.GA1879688@coredump.intra.peff.net","subject":"Re: [PATCH 1/2] date.c: allow fractional second part of ISO-8601","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-15T02:15:21Z","receivedAt":"2020-04-15T02:15:34Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-14 16:16:06-0400, Jeff King wrote:\n> On Tue, Apr 14, 2020 at 04:31:54PM +0700, Đoàn Trần Công Danh wrote:\n> \n> > git-commit(1) says ISO-8601 is one of our supported date format.\n> > However, we only support RFC-3339 date format.\n> > \n> > We can either:\n> > - Update documentation from ISO-8601 to RFC-3339\n> > - Add full support for ISO-8601\n> > \n> > This series will try to add full support for ISO-8601.\n> \n> That seems more like a cover letter for the full series. The important\n> thing in this patch is more like:\n> \n> ISO-8601 allows timestamps to have a fractional number of seconds. We\n> represent time only in terms of whole seconds, so we never bothered\n> parsing fractional seconds. However, it's better for us to parse and\n> throw away the fractional part than to refuse to parse the timestamp\n> at all.\n\nYeah, this sounds better. I intended write only this patch.\nI made the second patch when the comment about 8 digits for YYYYMMDD\ncome to my eyes, hence this clumsy.\n\n> > diff --git a/date.c b/date.c\n> > index b0d9a8421d..2f37397beb 100644\n> > --- a/date.c\n> > +++ b/date.c\n> > @@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> > \tcase ':':\n> > \t\tif (num3 < 0)\n> > \t\t\tnum3 = 0;\n> > +\t\telse if (*end == '.' && isdigit(end[1]))\n> > +\t\t\tstrtol(end + 1, &end, 10);\n> \n> I was a bit worried about this change hurting other cases where people\n> might use dots to separate numbers (but not mean a fraction). But the\n> two common ones I can think of should be OK:\n> \n> - 5.seconds.ago would never match because you look for a digit after\n> the dot\n\nThis is covered.\n\n> - a date like 2019.10.12 wouldn't match, because we're only looking\n> after a \":\", which heavily implies a time.\n\nYeah, It's worth to add this test.\n\n\n-- \nDanh\n"},{"id":"395465","messageId":"20200415021732.GC19025@danh.dev","threadId":"53221","inReplyTo":"20200414234958.GE2751707@camp.crustytoothpaste.net","subject":"Re: [PATCH 1/2] date.c: allow fractional second part of ISO-8601","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-15T02:17:32Z","receivedAt":"2020-04-15T02:17:42Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-14 23:49:58+0000, \"brian m. carlson\" wrote:\n> On 2020-04-14 at 20:17:33, Jeff King wrote:\n> > On Tue, Apr 14, 2020 at 04:31:54PM +0700, Đoàn Trần Công Danh wrote:\n> > \n> > > diff --git a/date.c b/date.c\n> > > index b0d9a8421d..2f37397beb 100644\n> > > --- a/date.c\n> > > +++ b/date.c\n> > > @@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> > > \tcase ':':\n> > > \t\tif (num3 < 0)\n> > > \t\t\tnum3 = 0;\n> > > +\t\telse if (*end == '.' && isdigit(end[1]))\n> > > +\t\t\tstrtol(end + 1, &end, 10);\n> > \n> > This just throws away the fractional part. I doubt anybody cares much\n> > either way, but another option would be to round num3 up or down.\n> \n> I'm happy with truncation. It's simple and straightforward and will be\n> widely and intuitively understood by programmers, who are probably the\n> primary users of our ISO 8601 handling.\n> \n> I do appreciate that you pointed this out, because it's a good point.\n> \n> And overall, I agree that this seems like a good fix for the problem.\n\nI'll reword the subject to \"throw away fractional second part of ISO-8601\"\n\n1 second is not that big, and and the logic is simpler.\n\n-- \nDanh\n"},{"id":"395467","messageId":"cover.1586921235.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586856398.git.congdanhqx@gmail.com","subject":"[PATCH v2 0/2] More ISO-8601 support","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-15T03:31:26Z","receivedAt":"2020-04-15T03:32:22Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"This series aims to extend support for ISO-8601 datetime format\nto allow compact version, and fractional part of ISO-8601.\n\nĐoàn Trần Công Danh (2):\n date.c: skip fractional second part of ISO-8601\n date.c: allow compact version of ISO-8601 datetime\n\n Documentation/date-formats.txt | 2 +-\n date.c | 24 ++++++++++++++++++++++++\n t/t0006-date.sh | 5 +++++\n 3 files changed, 30 insertions(+), 1 deletion(-)\n\nRange-diff against v1:\n1: d7d74b03cc ! 1: 03f3e9968b date.c: allow fractional second part of ISO-8601\n @@ Metadata\n Author: Đoàn Trần Công Danh \n \n ## Commit message ##\n - date.c: allow fractional second part of ISO-8601\n + date.c: skip fractional second part of ISO-8601\n \n git-commit(1) says ISO-8601 is one of our supported date format.\n - However, we only support RFC-3339 date format.\n \n - We can either:\n - - Update documentation from ISO-8601 to RFC-3339\n - - Add full support for ISO-8601\n + ISO-8601 allows timestamps to have a fractional number of seconds.\n + We represent time only in terms of whole seconds, so we never bothered\n + parsing fractional seconds. However, it's better for us to parse and\n + throw away the fractional part than to refuse to parse the timestamp\n + at all.\n \n - This series will try to add full support for ISO-8601.\n + And refusing parsing fractional second part may confuse the parse to\n + think fractional and timezone as day and month in this example:\n +\n + 2008-02-14 20:30:45.019-04:00\n \n Reported-by: Brian M. Carlson \n Signed-off-by: Đoàn Trần Công Danh \n \n + ## Documentation/date-formats.txt ##\n +@@ Documentation/date-formats.txt: RFC 2822::\n + ISO 8601::\n + \tTime and date specified by the ISO 8601 standard, for example\n + \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n +-\t`T` character as well.\n ++\t`T` character as well. The fractional part will be ignored.\n + +\n + NOTE: In addition, the date part is accepted in the following formats:\n + `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\n +\n ## date.c ##\n @@ date.c: static int match_multi_number(timestamp_t num, char c, const char *date,\n \tcase ':':\n @@ t/t0006-date.sh: check_parse 2008-02 bad\n check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n ++check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n +check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n2: 48284386c9 < -: ---------- date.c: allow compact version of ISO-8601 datetime\n-: ---------- > 2: 36517af872 date.c: allow compact version of ISO-8601 datetime\n-- \n2.26.0.485.g42af89717d\n\n"},{"id":"395468","messageId":"03f3e9968bc29b758f2dd32434c9e8d19f624fdd.1586921235.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586921235.git.congdanhqx@gmail.com","subject":"[PATCH v2 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-15T03:31:27Z","receivedAt":"2020-04-15T03:32:23Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"git-commit(1) says ISO-8601 is one of our supported date format.\n\nISO-8601 allows timestamps to have a fractional number of seconds.\nWe represent time only in terms of whole seconds, so we never bothered\nparsing fractional seconds. However, it's better for us to parse and\nthrow away the fractional part than to refuse to parse the timestamp\nat all.\n\nAnd refusing parsing fractional second part may confuse the parse to\nthink fractional and timezone as day and month in this example:\n\n\t2008-02-14 20:30:45.019-04:00\n\nReported-by: Brian M. Carlson \nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/date-formats.txt | 2 +-\n date.c | 2 ++\n t/t0006-date.sh | 2 ++\n 3 files changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt\nindex 6926e0a4c8..6f69ba2ddd 100644\n--- a/Documentation/date-formats.txt\n+++ b/Documentation/date-formats.txt\n@@ -20,7 +20,7 @@ RFC 2822::\n ISO 8601::\n \tTime and date specified by the ISO 8601 standard, for example\n \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n-\t`T` character as well.\n+\t`T` character as well. The fractional part will be ignored.\n +\n NOTE: In addition, the date part is accepted in the following formats:\n `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\ndiff --git a/date.c b/date.c\nindex b0d9a8421d..2f37397beb 100644\n--- a/date.c\n+++ b/date.c\n@@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \tcase ':':\n \t\tif (num3 < 0)\n \t\t\tnum3 = 0;\n+\t\telse if (*end == '.' && isdigit(end[1]))\n+\t\t\tstrtol(end + 1, &end, 10);\n \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n \t\t\ttm->tm_hour = num;\n \t\t\ttm->tm_min = num2;\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex d9fcc829a9..d582dea5c5 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -81,6 +81,8 @@ check_parse 2008-02 bad\n check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n-- \n2.26.0.485.g42af89717d\n\n"},{"id":"395469","messageId":"36517af87298ebeb714511534bd180983905c67f.1586921235.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586921235.git.congdanhqx@gmail.com","subject":"[PATCH v2 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-15T03:31:28Z","receivedAt":"2020-04-15T03:32:25Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh \n---\n date.c | 22 ++++++++++++++++++++++\n t/t0006-date.sh | 3 +++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/date.c b/date.c\nindex 2f37397beb..fc42382cfa 100644\n--- a/date.c\n+++ b/date.c\n@@ -666,6 +666,28 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n \t\tn++;\n \t} while (isdigit(date[n]));\n \n+\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n+\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n+\tif (n == 8 || n == 6) {\n+\t\tunsigned int num1 = num / 10000;\n+\t\tunsigned int num2 = (num % 10000) / 100;\n+\t\tunsigned int num3 = num % 100;\n+\t\tif (n == 8 && num1 > 1900 &&\n+\t\t num2 > 0 && num2 <= 12 &&\n+\t\t num3 > 0 && num3 <= 31) {\n+\t\t\ttm->tm_year = num1 - 1900;\n+\t\t\ttm->tm_mon = num2 - 1;\n+\t\t\ttm->tm_mday = num3;\n+\t\t} else if (n == 6 && num1 < 60 && num2 < 60 && num3 <= 60) {\n+\t\t\ttm->tm_hour = num1;\n+\t\t\ttm->tm_min = num2;\n+\t\t\ttm->tm_sec = num3;\n+\t\t\tif (*end == '.' && isdigit(end[1]))\n+\t\t\t\tstrtoul(end + 1, &end, 10);\n+\t\t}\n+\t\treturn end - date;\n+\t}\n+\n \t/* Four-digit year or a timezone? */\n \tif (n == 4) {\n \t\tif (num <= 1400 && *offset == -1) {\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex d582dea5c5..1e169c8d15 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -82,6 +82,9 @@ check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n-- \n2.26.0.485.g42af89717d\n\n"},{"id":"395487","messageId":"20200415101747.ua22fjadmfxzri5n@tb-raspi4","threadId":"53221","inReplyTo":"03f3e9968bc29b758f2dd32434c9e8d19f624fdd.1586921235.git.congdanhqx@gmail.com","subject":"Re: [PATCH v2 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2020-04-15T10:17:48Z","receivedAt":"2020-04-15T10:47:17Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On Wed, Apr 15, 2020 at 10:31:27AM +0700, Đoàn Trần Công Danh wrote:\n> git-commit(1) says ISO-8601 is one of our supported date format.\n>\n> ISO-8601 allows timestamps to have a fractional number of seconds.\n> We represent time only in terms of whole seconds, so we never bothered\n> parsing fractional seconds. However, it's better for us to parse and\n> throw away the fractional part than to refuse to parse the timestamp\n> at all.\n>\n> And refusing parsing fractional second part may confuse the parse to\n> think fractional and timezone as day and month in this example:\n>\n> \t2008-02-14 20:30:45.019-04:00\n>\n> Reported-by: Brian M. Carlson \n> Signed-off-by: Đoàn Trần Công Danh \n> ---\n> Documentation/date-formats.txt | 2 +-\n> date.c | 2 ++\n> t/t0006-date.sh | 2 ++\n> 3 files changed, 5 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt\n> index 6926e0a4c8..6f69ba2ddd 100644\n> --- a/Documentation/date-formats.txt\n> +++ b/Documentation/date-formats.txt\n> @@ -20,7 +20,7 @@ RFC 2822::\n> ISO 8601::\n> \tTime and date specified by the ISO 8601 standard, for example\n> \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n> -\t`T` character as well.\n> +\t`T` character as well. The fractional part will be ignored.\n\nWhen somebody has read the whole patch series, it is clear what\n\"fractional part\" means. Otherwise it is not clear, what fractional part\nmeans. The following may be easier to understand ?\n\n ISO 8601::\n \tTime and date specified by the ISO 8601 standard, for example\n \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n \t`T` character as well.\n \tFractional parts of a second like `2005-04-07T22:13:13.091`\n \twill be ignored and treated as `2005-04-07T22:13:13`\n\n> +\n> NOTE: In addition, the date part is accepted in the following formats:\n> `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\n> diff --git a/date.c b/date.c\n> index b0d9a8421d..2f37397beb 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -556,6 +556,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> \tcase ':':\n> \t\tif (num3 < 0)\n> \t\t\tnum3 = 0;\n> +\t\telse if (*end == '.' && isdigit(end[1]))\n> +\t\t\tstrtol(end + 1, &end, 10);\n> \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n> \t\t\ttm->tm_hour = num;\n> \t\t\ttm->tm_min = num2;\n> diff --git a/t/t0006-date.sh b/t/t0006-date.sh\n> index d9fcc829a9..d582dea5c5 100755\n> --- a/t/t0006-date.sh\n> +++ b/t/t0006-date.sh\n> @@ -81,6 +81,8 @@ check_parse 2008-02 bad\n> check_parse 2008-02-14 bad\n> check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n> check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n> +check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n> +check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n> check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n> check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n> check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n> --\n> 2.26.0.485.g42af89717d\n>\n"},{"id":"395498","messageId":"xmqq5ze094gg.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"20200414202401.GC1879688@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-15T15:03:11Z","receivedAt":"2020-04-15T15:03:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Tue, Apr 14, 2020 at 04:31:55PM +0700, Đoàn Trần Công Danh wrote:\n>\n>> @@ -666,6 +666,24 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n>> \t\tn++;\n>> \t} while (isdigit(date[n]));\n>> \n>> +\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n>> +\tif (n == 8) {\n>> +\t\ttm->tm_year = num / 10000 - 1900;\n>> +\t\ttm->tm_mon = (num % 10000) / 100 - 1;\n>> +\t\ttm->tm_mday = num % 100;\n>> +\t\treturn n;\n>> +\t}\n>\n> I worry a little this may conflict with other approxidate heuristics.\n>\n> The only one I can think of is an actual unix timestamp, though, and we\n> already require that to have at least 9 digits (plus anybody wanting to\n> use one robustly really should be using @12345678).\n\nI am OK with 1/2, but I'd worry a LOT about this one, if we didn't\nrequire 9 digits. 60/100 * 60/100 * 24/100 ~= 8.6% so limiting the\nhour/min/sec to sensible values is not a useful protection against\nambiguity. We'd essentially be declaring that a raw UNIX timestamp\nwithout @ prefix is now hit-and-miss if we take this change, were\nthere not the \"must be at least 9 digits\" requirement.\n\nSomebody was thinking when he wrote the very beginning part of the\nmatch_digit() function ;-)\n\n> We could probably tighten the heuristics a bit by insisting that the\n> month and day be sensible. Or even year (see the 1900 to 2100 magic for\n> the 4-digit year guess).\n\nI do agree that we'd want 0 <= hr <= 24, 0 <= min <= 59, and 0 <=\nsec <= 60 (or should this be 61?), but it is for correctness of the\nnew code. It wouldn't have any value in disambiguation from other\nformats, I would think. So, from that point of view, I would buy\nsomething like 1969 as a lower bound, but I am not sure if we\nnecessarily want an upper bound for the year. What mistake are we\nprotecting us against?\n\nThanks.\n\n>> +\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n>> +\tif (n == 6) {\n>> +\t\ttm->tm_hour = num / 10000;\n>> +\t\ttm->tm_min = (num % 10000) / 100;\n>> +\t\ttm->tm_sec = num % 100;\n>> +\t\tif (*end == '.' && isdigit(end[1]))\n>> +\t\t\tstrtoul(end + 1, &end, 10);\n>> +\t\treturn end - date;\n>> +\t}\n>\n> And likewise here that the hour, minute, and second be reasonable.\n>\n> -Peff\n"},{"id":"395504","messageId":"20200415154157.GC2464307@coredump.intra.peff.net","threadId":"53221","inReplyTo":"xmqq5ze094gg.fsf@gitster.c.googlers.com","subject":"Re: [PATCH 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-04-15T15:41:57Z","receivedAt":"2020-04-15T15:42:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 15, 2020 at 08:03:11AM -0700, Junio C Hamano wrote:\n\n> > I worry a little this may conflict with other approxidate heuristics.\n> >\n> > The only one I can think of is an actual unix timestamp, though, and we\n> > already require that to have at least 9 digits (plus anybody wanting to\n> > use one robustly really should be using @12345678).\n> \n> I am OK with 1/2, but I'd worry a LOT about this one, if we didn't\n> require 9 digits. 60/100 * 60/100 * 24/100 ~= 8.6% so limiting the\n> hour/min/sec to sensible values is not a useful protection against\n> ambiguity. We'd essentially be declaring that a raw UNIX timestamp\n> without @ prefix is now hit-and-miss if we take this change, were\n> there not the \"must be at least 9 digits\" requirement.\n\nYeah, that was exactly why I poked at it.\n\nCuriously, a1a5a6347b (Accept dates before 2000/01/01 when specified as\nseconds since the epoch, 2007-06-06), the commit which introduced the\n9-digit rule, specifically says:\n\n There is now still a limit, 100000000, which is 1973/03/03 09:46:40\n UTC, in order to allow that dates are represented as 8 digits.\n\nbut running \"test-date 20100403\" even back at that commit does not seem\nto work (nor does it work now).\n\nDoubly curious, some nearby code blames to 9f2b6d2936 (date/time: do not\nget confused by fractional seconds, 2008-08-16). So why don't fractional\nseconds work right now?\n\nThey sure don't seem to:\n\n [the 17 becomes a day-of-month]\n $ t/helper/test-tool date approxidate 12:43:50.17\n 12:43:50.17 -> 2020-04-17 16:43:50 +0000\n\nBut I wonder if the new patch breaks the example from that commit\nmessage, which does work now:\n\n [current version; the 17 attaches to \"days ago\"]\n $ t/helper/test-tool date approxidate 12:43:50.17.days.ago\n 12:43:50.17.days.ago -> 2020-03-29 16:43:50 +0000\n\nIt looks like the answer is yes:\n\n [with patch 1/2 applied; now it's a fractional second]\n $ t/helper/test-tool date approxidate 12:43:50.17.days.ago\n 12:43:50.17.days.ago -> 2020-04-15 16:43:50 +0000\n\nTBH I'm not sure how big a deal it is. The input is rather ambiguous for\na strict left-to-right parser, and I'm not sure this case is worth doing\nmore look-ahead parsing. But it's worth making a conscious decision\nabout it.\n\nOne alternative would be to restrict the fractional second handling only\nto the parse_date_basic(), which is quite a bit stricter. It shares\nmatch_digit() with approxidate(), so we'd probably have to pass in an\nextra flag to match_digit() to change the rules. It also means that:\n\n 2020-04-14T12:43:50.17\n\nwould parse a fractional second, but:\n\n yesterday at 12:43:50.17\n\nwouldn't.\n\n> > We could probably tighten the heuristics a bit by insisting that the\n> > month and day be sensible. Or even year (see the 1900 to 2100 magic for\n> > the 4-digit year guess).\n> \n> I do agree that we'd want 0 <= hr <= 24, 0 <= min <= 59, and 0 <=\n> sec <= 60 (or should this be 61?), but it is for correctness of the\n> new code. It wouldn't have any value in disambiguation from other\n> formats, I would think. So, from that point of view, I would buy\n> something like 1969 as a lower bound, but I am not sure if we\n> necessarily want an upper bound for the year. What mistake are we\n> protecting us against?\n\nNo particular mistake. I was just suggesting to keep the heuristics in\nthe new code as tight as possible to leave room for later changes (since\napproxidate by its nature is heuristics and hacks piled atop each\nother).\n\nI do agree that year bounds are a questionable restriction. Right now\nGit's date code can only handle dates between 1970 and 2099, but it\nwould be nice to change that.\n\n-Peff\n"},{"id":"395511","messageId":"xmqqk12g7nci.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"20200415154157.GC2464307@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-15T15:58:05Z","receivedAt":"2020-04-15T15:58:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But I wonder if the new patch breaks the example from that commit\n> message, which does work now:\n>\n> [current version; the 17 attaches to \"days ago\"]\n> $ t/helper/test-tool date approxidate 12:43:50.17.days.ago\n> 12:43:50.17.days.ago -> 2020-03-29 16:43:50 +0000\n>\n> It looks like the answer is yes:\n>\n> [with patch 1/2 applied; now it's a fractional second]\n> $ t/helper/test-tool date approxidate 12:43:50.17.days.ago\n> 12:43:50.17.days.ago -> 2020-04-15 16:43:50 +0000\n>\n> TBH I'm not sure how big a deal it is. The input is rather ambiguous for\n> a strict left-to-right parser, and I'm not sure this case is worth doing\n> more look-ahead parsing. But it's worth making a conscious decision\n> about it.\n\nTrue. I do agree that this would qualify as a regression, but I do\nnot think we are breaking an important use case here.\n\n> One alternative would be to restrict the fractional second handling only\n> to the parse_date_basic(), which is quite a bit stricter. It shares\n> match_digit() with approxidate(), so we'd probably have to pass in an\n> extra flag to match_digit() to change the rules. It also means that:\n>\n> 2020-04-14T12:43:50.17\n>\n> would parse a fractional second, but:\n>\n> yesterday at 12:43:50.17\n>\n> wouldn't.\n\nThat sounds less useful. Somehow \"yesterday at 12.43:50.17\" looks a\nlot more plausible real-user input that we would want to support\nbetter than \"12:43:50.17.day.ago\".\n\n> I do agree that year bounds are a questionable restriction. Right now\n> Git's date code can only handle dates between 1970 and 2099, but it\n> would be nice to change that.\n\nSure. In both directions.\n"},{"id":"395576","messageId":"20200416100426.GA2285@danh.dev","threadId":"53221","inReplyTo":"20200415101747.ua22fjadmfxzri5n@tb-raspi4","subject":"Re: [PATCH v2 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-16T10:04:26Z","receivedAt":"2020-04-16T10:04:37Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-15 12:17:48+0200, Torsten Bögershausen wrote:\n> On Wed, Apr 15, 2020 at 10:31:27AM +0700, Đoàn Trần Công Danh wrote:\n> > git-commit(1) says ISO-8601 is one of our supported date format.\n> >\n> > ISO-8601 allows timestamps to have a fractional number of seconds.\n> > We represent time only in terms of whole seconds, so we never bothered\n> > parsing fractional seconds. However, it's better for us to parse and\n> > throw away the fractional part than to refuse to parse the timestamp\n> > at all.\n> >\n> > And refusing parsing fractional second part may confuse the parse to\n> > think fractional and timezone as day and month in this example:\n> >\n> > \t2008-02-14 20:30:45.019-04:00\n> >\n> > Reported-by: Brian M. Carlson \n> > Signed-off-by: Đoàn Trần Công Danh \n> > ---\n> > Documentation/date-formats.txt | 2 +-\n> > date.c | 2 ++\n> > t/t0006-date.sh | 2 ++\n> > 3 files changed, 5 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt\n> > index 6926e0a4c8..6f69ba2ddd 100644\n> > --- a/Documentation/date-formats.txt\n> > +++ b/Documentation/date-formats.txt\n> > @@ -20,7 +20,7 @@ RFC 2822::\n> > ISO 8601::\n> > \tTime and date specified by the ISO 8601 standard, for example\n> > \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n> > -\t`T` character as well.\n> > +\t`T` character as well. The fractional part will be ignored.\n> \n> When somebody has read the whole patch series, it is clear what\n> \"fractional part\" means. Otherwise it is not clear, what fractional part\n> means. The following may be easier to understand ?\n> \n> ISO 8601::\n> \tTime and date specified by the ISO 8601 standard, for example\n> \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n> \t`T` character as well.\n> \tFractional parts of a second like `2005-04-07T22:13:13.091`\n> \twill be ignored and treated as `2005-04-07T22:13:13`\n\nYes, this read better.\nI think I'll rephase to\n\n\tFractional parts of a second will be ignored, for example\n\t`2005-04-07T22:13:13.019` will be treated as\n\t`2005-04-07T22:13:13`\n\n-- \nDanh\n"},{"id":"395578","messageId":"20200416111608.GB2285@danh.dev","threadId":"53221","inReplyTo":"20200415154157.GC2464307@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-16T11:16:08Z","receivedAt":"2020-04-16T11:28:46Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-15 11:41:57-0400, Jeff King wrote:\n> On Wed, Apr 15, 2020 at 08:03:11AM -0700, Junio C Hamano wrote:\n> \n> > > I worry a little this may conflict with other approxidate heuristics.\n> > >\n> > > The only one I can think of is an actual unix timestamp, though, and we\n> > > already require that to have at least 9 digits (plus anybody wanting to\n> > > use one robustly really should be using @12345678).\n> > \n> > I am OK with 1/2, but I'd worry a LOT about this one, if we didn't\n> > require 9 digits. 60/100 * 60/100 * 24/100 ~= 8.6% so limiting the\n> > hour/min/sec to sensible values is not a useful protection against\n> > ambiguity. We'd essentially be declaring that a raw UNIX timestamp\n> > without @ prefix is now hit-and-miss if we take this change, were\n> > there not the \"must be at least 9 digits\" requirement.\n> \n> Yeah, that was exactly why I poked at it.\n> \n> Curiously, a1a5a6347b (Accept dates before 2000/01/01 when specified as\n> seconds since the epoch, 2007-06-06), the commit which introduced the\n> 9-digit rule, specifically says:\n> \n> There is now still a limit, 100000000, which is 1973/03/03 09:46:40\n> UTC, in order to allow that dates are represented as 8 digits.\n> \n> but running \"test-date 20100403\" even back at that commit does not seem\n> to work (nor does it work now).\n\ncheck-parse 2010-04-03 doesn't work either.\ncheck-approxiate is going through different code path via\napproxidate_digit, but I'm not really sure which action should be take\n\n> Doubly curious, some nearby code blames to 9f2b6d2936 (date/time: do not\n> get confused by fractional seconds, 2008-08-16). So why don't fractional\n> seconds work right now?\n\nIt doesn't work in that time, either:\n\n\t$ git checkout 9f2b6d2936\n\t$ make\n\t$ ./git --version\n\tgit version 1.6.0.3.g9f2b6\n\t$ GIT_AUTHOR_DATE='2019-04-16 00:02:03.19-04:00' ./git-commit --allow-empty -m msg\n\t$ ./git-log --pretty=format:%aD -1\n\tFri, 19 Apr 2019 00:02:03 +0700\n\nThe different is whether we stick the timezone (west of UTC) into the\nfractional or not. If we have both those conditions:\n\n- Specify the timezone west of UTC\n- Stick it with the fractional part\n- The fractional part is parsed to number less than 32\n\nThe the parser will think it's US style of month/day\n\nJudging from the test code, it's likely people that write the code and\nthe test get used to separate time and timezone.\n\nAnd it's unlikely anyone will write fractional parts with only\n1 digit (yield 100% positive with sticking timezone), or 2 digits\n(which yields 32% positive with sticking timezone),\nfor 3 digits, the positive rate is 3.2%\n\n> \n> They sure don't seem to:\n> \n> [the 17 becomes a day-of-month]\n> $ t/helper/test-tool date approxidate 12:43:50.17\n> 12:43:50.17 -> 2020-04-17 16:43:50 +0000\n> \n> But I wonder if the new patch breaks the example from that commit\n> message, which does work now:\n> \n> [current version; the 17 attaches to \"days ago\"]\n> $ t/helper/test-tool date approxidate 12:43:50.17.days.ago\n> 12:43:50.17.days.ago -> 2020-03-29 16:43:50 +0000\n> \n> It looks like the answer is yes:\n> \n> [with patch 1/2 applied; now it's a fractional second]\n> $ t/helper/test-tool date approxidate 12:43:50.17.days.ago\n> 12:43:50.17.days.ago -> 2020-04-15 16:43:50 +0000\n> \n> TBH I'm not sure how big a deal it is. The input is rather ambiguous for\n> a strict left-to-right parser, and I'm not sure this case is worth doing\n> more look-ahead parsing. But it's worth making a conscious decision\n> about it.\n\nHm, I don't think \"12:43:50.17.days.ago\" is a normal input from user\nbut I think \"12:43:50.3.days.ago\" on Monday wouldn't have much less\nuser when compare with \"yesterday at 12:43:50.17\"\n\n> One alternative would be to restrict the fractional second handling only\n> to the parse_date_basic(), which is quite a bit stricter. It shares\n> match_digit() with approxidate(), so we'd probably have to pass in an\n> extra flag to match_digit() to change the rules. It also means that:\n> \n> 2020-04-14T12:43:50.17\n> \n> would parse a fractional second, but:\n> \n> yesterday at 12:43:50.17\n> \n> wouldn't.\n> \n> > > We could probably tighten the heuristics a bit by insisting that the\n> > > month and day be sensible. Or even year (see the 1900 to 2100 magic for\n> > > the 4-digit year guess).\n> > \n> > I do agree that we'd want 0 <= hr <= 24, 0 <= min <= 59, and 0 <=\n> > sec <= 60 (or should this be 61?), but it is for correctness of the\n> > new code. It wouldn't have any value in disambiguation from other\n> > formats, I would think. So, from that point of view, I would buy\n> > something like 1969 as a lower bound, but I am not sure if we\n> > necessarily want an upper bound for the year. What mistake are we\n> > protecting us against?\n> \n> No particular mistake. I was just suggesting to keep the heuristics in\n> the new code as tight as possible to leave room for later changes (since\n> approxidate by its nature is heuristics and hacks piled atop each\n> other).\n> \n> I do agree that year bounds are a questionable restriction. Right now\n> Git's date code can only handle dates between 1970 and 2099, but it\n> would be nice to change that.\n\n2100 is used inside is_date, which is the place we try to guess what\n does those number could mean in multiple format.\n\nI don't think we have ambiguity when parsing ISO-8601 compact date.\n\n-- \nDanh\n"},{"id":"395899","messageId":"cover.1587559135.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586856398.git.congdanhqx@gmail.com","subject":"[PATCH v3 0/2] More ISO-8601 support","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-22T13:15:51Z","receivedAt":"2020-04-22T13:16:07Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"This series aims to extend support for ISO-8601 datetime format\nto allow compact version, and fractional part of ISO-8601.\n\nChanges from v2:\n* Add example for fractional parts of second in documentation\n* Add/Fix regression test on 12:34:56.7.days.ago\n\nĐoàn Trần Công Danh (2):\n date.c: skip fractional second part of ISO-8601\n date.c: allow compact version of ISO-8601 datetime\n\n Documentation/date-formats.txt | 5 ++++-\n date.c | 32 +++++++++++++++++++++++++++++++-\n t/t0006-date.sh | 6 ++++++\n 3 files changed, 41 insertions(+), 2 deletions(-)\n\nRange-diff against v2:\n1: 03f3e9968b ! 1: c6d42785bb date.c: skip fractional second part of ISO-8601\n @@ Documentation/date-formats.txt: RFC 2822::\n \tTime and date specified by the ISO 8601 standard, for example\n \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n -\t`T` character as well.\n -+\t`T` character as well. The fractional part will be ignored.\n ++\t`T` character as well. Fractional parts of a second will be ignored,\n ++\tfor example `2005-04-07T22:13:13.019` will be treated as\n ++\t`2005-04-07T22:13:13`\n ++\n +\n NOTE: In addition, the date part is accepted in the following formats:\n `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\n \n ## date.c ##\n @@ date.c: static int match_multi_number(timestamp_t num, char c, const char *date,\n + \t/* Time? Date? */\n + \tswitch (c) {\n \tcase ':':\n - \t\tif (num3 < 0)\n +-\t\tif (num3 < 0)\n ++\t\tif (num3 < 0) {\n \t\t\tnum3 = 0;\n -+\t\telse if (*end == '.' && isdigit(end[1]))\n ++\t\t} else if (*end == '.' && isdigit(end[1]) &&\n ++\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 &&\n ++\t\t\t tm->tm_mday != -1) {\n ++\t\t\t/* Attempt to guess meaning of in HHMMSS.\n ++\t\t\t * only interpret as fractional when %Y %m %d is known.\n ++\t\t\t */\n +\t\t\tstrtol(end + 1, &end, 10);\n ++\t\t}\n \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n \t\t\ttm->tm_hour = num;\n \t\t\ttm->tm_min = num2;\n @@ t/t0006-date.sh: check_parse 2008-02 bad\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n +@@ t/t0006-date.sh: check_approxidate 5.seconds.ago '2009-08-30 19:19:55'\n + check_approxidate 10.minutes.ago '2009-08-30 19:10:00'\n + check_approxidate yesterday '2009-08-29 19:20:00'\n + check_approxidate 3.days.ago '2009-08-27 19:20:00'\n ++check_approxidate '12:34:56.3.days.ago' '2009-08-27 12:34:56'\n + check_approxidate 3.weeks.ago '2009-08-09 19:20:00'\n + check_approxidate 3.months.ago '2009-05-30 19:20:00'\n + check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'\n2: 36517af872 = 2: 225b6401bd date.c: allow compact version of ISO-8601 datetime\n-- \n2.26.2.303.gf8c07b1a78\n\n"},{"id":"395900","messageId":"c6d42785bb762f691b00c48b57c73a1933fadbc3.1587559135.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587559135.git.congdanhqx@gmail.com","subject":"[PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-22T13:15:52Z","receivedAt":"2020-04-22T13:16:08Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"git-commit(1) says ISO-8601 is one of our supported date format.\n\nISO-8601 allows timestamps to have a fractional number of seconds.\nWe represent time only in terms of whole seconds, so we never bothered\nparsing fractional seconds. However, it's better for us to parse and\nthrow away the fractional part than to refuse to parse the timestamp\nat all.\n\nAnd refusing parsing fractional second part may confuse the parse to\nthink fractional and timezone as day and month in this example:\n\n\t2008-02-14 20:30:45.019-04:00\n\nReported-by: Brian M. Carlson \nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/date-formats.txt | 5 ++++-\n date.c | 10 +++++++++-\n t/t0006-date.sh | 3 +++\n 3 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt\nindex 6926e0a4c8..7e7eaba643 100644\n--- a/Documentation/date-formats.txt\n+++ b/Documentation/date-formats.txt\n@@ -20,7 +20,10 @@ RFC 2822::\n ISO 8601::\n \tTime and date specified by the ISO 8601 standard, for example\n \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n-\t`T` character as well.\n+\t`T` character as well. Fractional parts of a second will be ignored,\n+\tfor example `2005-04-07T22:13:13.019` will be treated as\n+\t`2005-04-07T22:13:13`\n+\n +\n NOTE: In addition, the date part is accepted in the following formats:\n `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\ndiff --git a/date.c b/date.c\nindex b0d9a8421d..62f23b4702 100644\n--- a/date.c\n+++ b/date.c\n@@ -554,8 +554,16 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \t/* Time? Date? */\n \tswitch (c) {\n \tcase ':':\n-\t\tif (num3 < 0)\n+\t\tif (num3 < 0) {\n \t\t\tnum3 = 0;\n+\t\t} else if (*end == '.' && isdigit(end[1]) &&\n+\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 &&\n+\t\t\t tm->tm_mday != -1) {\n+\t\t\t/* Attempt to guess meaning of in HHMMSS.\n+\t\t\t * only interpret as fractional when %Y %m %d is known.\n+\t\t\t */\n+\t\t\tstrtol(end + 1, &end, 10);\n+\t\t}\n \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n \t\t\ttm->tm_hour = num;\n \t\t\ttm->tm_min = num2;\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex d9fcc829a9..80917c81c3 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -81,6 +81,8 @@ check_parse 2008-02 bad\n check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n@@ -103,6 +105,7 @@ check_approxidate 5.seconds.ago '2009-08-30 19:19:55'\n check_approxidate 10.minutes.ago '2009-08-30 19:10:00'\n check_approxidate yesterday '2009-08-29 19:20:00'\n check_approxidate 3.days.ago '2009-08-27 19:20:00'\n+check_approxidate '12:34:56.3.days.ago' '2009-08-27 12:34:56'\n check_approxidate 3.weeks.ago '2009-08-09 19:20:00'\n check_approxidate 3.months.ago '2009-05-30 19:20:00'\n check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'\n-- \n2.26.2.303.gf8c07b1a78\n\n"},{"id":"395901","messageId":"225b6401bd1f7eddc245acfd2c4b37c50c978491.1587559135.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587559135.git.congdanhqx@gmail.com","subject":"[PATCH v3 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-22T13:15:53Z","receivedAt":"2020-04-22T13:16:09Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh \n---\n date.c | 22 ++++++++++++++++++++++\n t/t0006-date.sh | 3 +++\n 2 files changed, 25 insertions(+)\n\ndiff --git a/date.c b/date.c\nindex 62f23b4702..882242c2db 100644\n--- a/date.c\n+++ b/date.c\n@@ -672,6 +672,28 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n \t\tn++;\n \t} while (isdigit(date[n]));\n \n+\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n+\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n+\tif (n == 8 || n == 6) {\n+\t\tunsigned int num1 = num / 10000;\n+\t\tunsigned int num2 = (num % 10000) / 100;\n+\t\tunsigned int num3 = num % 100;\n+\t\tif (n == 8 && num1 > 1900 &&\n+\t\t num2 > 0 && num2 <= 12 &&\n+\t\t num3 > 0 && num3 <= 31) {\n+\t\t\ttm->tm_year = num1 - 1900;\n+\t\t\ttm->tm_mon = num2 - 1;\n+\t\t\ttm->tm_mday = num3;\n+\t\t} else if (n == 6 && num1 < 60 && num2 < 60 && num3 <= 60) {\n+\t\t\ttm->tm_hour = num1;\n+\t\t\ttm->tm_min = num2;\n+\t\t\ttm->tm_sec = num3;\n+\t\t\tif (*end == '.' && isdigit(end[1]))\n+\t\t\t\tstrtoul(end + 1, &end, 10);\n+\t\t}\n+\t\treturn end - date;\n+\t}\n+\n \t/* Four-digit year or a timezone? */\n \tif (n == 4) {\n \t\tif (num <= 1400 && *offset == -1) {\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 80917c81c3..75ee9a96b8 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -82,6 +82,9 @@ check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n-- \n2.26.2.303.gf8c07b1a78\n\n"},{"id":"395914","messageId":"xmqqk127jvrh.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"c6d42785bb762f691b00c48b57c73a1933fadbc3.1587559135.git.congdanhqx@gmail.com","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-22T17:05:54Z","receivedAt":"2020-04-22T17:05:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> \t/* Time? Date? */\n> \tswitch (c) {\n> \tcase ':':\n> -\t\tif (num3 < 0)\n> +\t\tif (num3 < 0) {\n> \t\t\tnum3 = 0;\n> +\t\t} else if (*end == '.' && isdigit(end[1]) &&\n> +\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 &&\n> +\t\t\t tm->tm_mday != -1) {\n> +\t\t\t/* Attempt to guess meaning of in HHMMSS.\n> +\t\t\t * only interpret as fractional when %Y %m %d is known.\n> +\t\t\t */\n> +\t\t\tstrtol(end + 1, &end, 10);\n\nOK, so we saw ':' and parsed after it, and then saw ':' and\n, which we know is a good positive number. We haven't checked\nwhat is at this point, but it has to be 0 or more digits\n(because we wouldn't have parsed for if it weren't terminated\nwith the same c, i.e. ':').\n\n*end points at the byte that stopped and we make sure \nis followed by \".\" and a digit.\n\nRegardless of what is, we just discard the \"fractional part\nof seconds\" (assuming that is the \"seconds\" part).\n\n> +\t\t}\n> \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n> \t\t\ttm->tm_hour = num;\n> \t\t\ttm->tm_min = num2;\n\nAnd after all that is done, if (and others) are within a\nreasonable range, we use that as HH:MM:SS. \n\nOK. If (or , or even for that matter) weren't\nreasonable, is it still sensible to discard the fractional part?\nThe answer is not immediately obvious to me.\n\nTo be safe, it might make sense to extract a helper function from\nthe next conditional, i.e.\n\nstatic int is_hms(long num1, long num2, long num3)\n{\n\treturn (0 <= num1 && num1 < 25 &&\n\t\t0 <= num2 && num2 < 60 &&\n\t\t0 <= num3 && num3 <= 60);\n}\n\nand use it in the new \"else if\" block like so?\n\n\n\t} else if (*end == '.' && isdigit(end[1]) &&\n\t\t is_date(tm->tm_year, tm->tm_mon, tm->tm_mday, NULL, now, tm) &&\n\t\t is_hms(num, num2, num3)) {\n\t\t/* Discard \".\" from \"HH:MM:SS.\" */\n\t\t(void) strtol(end + 1, &end, 10);\n\t}\n\n\tif (is_hms(num, num2, num3)) {\n\t\t...\n\n\n\n> diff --git a/t/t0006-date.sh b/t/t0006-date.sh\n> index d9fcc829a9..80917c81c3 100755\n> --- a/t/t0006-date.sh\n> +++ b/t/t0006-date.sh\n> @@ -81,6 +81,8 @@ check_parse 2008-02 bad\n> check_parse 2008-02-14 bad\n> check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n> check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n> +check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n> +check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n> check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n> check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n> check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n> @@ -103,6 +105,7 @@ check_approxidate 5.seconds.ago '2009-08-30 19:19:55'\n> check_approxidate 10.minutes.ago '2009-08-30 19:10:00'\n> check_approxidate yesterday '2009-08-29 19:20:00'\n> check_approxidate 3.days.ago '2009-08-27 19:20:00'\n> +check_approxidate '12:34:56.3.days.ago' '2009-08-27 12:34:56'\n> check_approxidate 3.weeks.ago '2009-08-09 19:20:00'\n> check_approxidate 3.months.ago '2009-05-30 19:20:00'\n> check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'\n"},{"id":"395915","messageId":"xmqqftcvjv80.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"225b6401bd1f7eddc245acfd2c4b37c50c978491.1587559135.git.congdanhqx@gmail.com","subject":"Re: [PATCH v3 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-22T17:17:35Z","receivedAt":"2020-04-22T17:17:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> Signed-off-by: Đoàn Trần Công Danh \n> ---\n> date.c | 22 ++++++++++++++++++++++\n> t/t0006-date.sh | 3 +++\n> 2 files changed, 25 insertions(+)\n>\n> diff --git a/date.c b/date.c\n> index 62f23b4702..882242c2db 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -672,6 +672,28 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n> \t\tn++;\n> \t} while (isdigit(date[n]));\n> \n> +\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n> +\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n> +\tif (n == 8 || n == 6) {\n> +\t\tunsigned int num1 = num / 10000;\n> +\t\tunsigned int num2 = (num % 10000) / 100;\n> +\t\tunsigned int num3 = num % 100;\n> +\t\tif (n == 8 && num1 > 1900 &&\n> +\t\t num2 > 0 && num2 <= 12 &&\n> +\t\t num3 > 0 && num3 <= 31) {\n> +\t\t\ttm->tm_year = num1 - 1900;\n> +\t\t\ttm->tm_mon = num2 - 1;\n> +\t\t\ttm->tm_mday = num3;\n> +\t\t} else if (n == 6 && num1 < 60 && num2 < 60 && num3 <= 60) {\n> +\t\t\ttm->tm_hour = num1;\n> +\t\t\ttm->tm_min = num2;\n> +\t\t\ttm->tm_sec = num3;\n> +\t\t\tif (*end == '.' && isdigit(end[1]))\n> +\t\t\t\tstrtoul(end + 1, &end, 10);\n> +\t\t}\n> +\t\treturn end - date;\n> +\t}\n> +\n\nLooks sensible except that on our planet, one day has only 24 hours\n;-).\n\nI think we should try to reuse existing helpers as much as possible\nin date.c to avoid such stupid errors. During my review of [1/2] I\nfound is_date() would be a good thing to try reusing and also\nextracted is_hms() as another candidate we could reuse.\n\n> \t/* Four-digit year or a timezone? */\n> \tif (n == 4) {\n> \t\tif (num <= 1400 && *offset == -1) {\n> diff --git a/t/t0006-date.sh b/t/t0006-date.sh\n> index 80917c81c3..75ee9a96b8 100755\n> --- a/t/t0006-date.sh\n> +++ b/t/t0006-date.sh\n> @@ -82,6 +82,9 @@ check_parse 2008-02-14 bad\n> check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n> check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n> check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n> +check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'\n> +check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'\n> +check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'\n> check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n> check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n> check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n"},{"id":"395973","messageId":"20200423011812.GA1930@danh.dev","threadId":"53221","inReplyTo":"xmqqk127jvrh.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-23T01:18:12Z","receivedAt":"2020-04-23T01:18:17Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-22 10:05:54-0700, Junio C Hamano wrote:\n> Đoàn Trần Công Danh writes:\n> \n> > \t/* Time? Date? */\n> > \tswitch (c) {\n> > \tcase ':':\n> > -\t\tif (num3 < 0)\n> > +\t\tif (num3 < 0) {\n> > \t\t\tnum3 = 0;\n> > +\t\t} else if (*end == '.' && isdigit(end[1]) &&\n> > +\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 &&\n> > +\t\t\t tm->tm_mday != -1) {\n> > +\t\t\t/* Attempt to guess meaning of in HHMMSS.\n> > +\t\t\t * only interpret as fractional when %Y %m %d is known.\n> > +\t\t\t */\n> > +\t\t\tstrtol(end + 1, &end, 10);\n> \n> OK, so we saw ':' and parsed after it, and then saw ':' and\n> , which we know is a good positive number. We haven't checked\n> what is at this point, but it has to be 0 or more digits\n> (because we wouldn't have parsed for if it weren't terminated\n> with the same c, i.e. ':').\n> \n> *end points at the byte that stopped and we make sure \n> is followed by \".\" and a digit.\n> \n> Regardless of what is, we just discard the \"fractional part\n> of seconds\" (assuming that is the \"seconds\" part).\n> \n> > +\t\t}\n> > \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n> > \t\t\ttm->tm_hour = num;\n> > \t\t\ttm->tm_min = num2;\n> \n> And after all that is done, if (and others) are within a\n> reasonable range, we use that as HH:MM:SS. \n> \n> OK. If (or , or even for that matter) weren't\n> reasonable, is it still sensible to discard the fractional part?\n> The answer is not immediately obvious to me.\n> \n> To be safe, it might make sense to extract a helper function from\n> the next conditional, i.e.\n> \n> static int is_hms(long num1, long num2, long num3)\n\nI'll make it `is_time` on par with is_date check.\nI'll look into it and check if int or long is suitable for parameter's\ntype.\n\n> {\n> \treturn (0 <= num1 && num1 < 25 &&\n> \t\t0 <= num2 && num2 < 60 &&\n> \t\t0 <= num3 && num3 <= 60);\n\nDoes it worth to add an explicit comment that we intentionally ignore\nthe old-and-defective 2nd leap seconds (i.e. second with value 61)?\n\nI saw in one of your previous email doubting if we should check for\n`num3 <= 61` or not.\n\n> }\n> \n> and use it in the new \"else if\" block like so?\n> \n> \n> \t} else if (*end == '.' && isdigit(end[1]) &&\n> \t\t is_date(tm->tm_year, tm->tm_mon, tm->tm_mday, NULL, now, tm) &&\n\nWhen running into this, the code patch for non-approxidate hasn't\ninitialised value for now, yet.\n\nAnd for non-approxidate, when trying to parse date, we initialise now\nwhen guessing which value is year, month, day.\n\nDoes it make sense to initialise now from parse_date_basic, passed it\nto match_multi_number via match_digit?\n\nTo me, it's too much a pain.\n\nI /think/ it isn't worth to warrant a time(NULL) and another is_date\ncall here. We've checked it one when we set it before, no?\n\n> \t\t is_hms(num, num2, num3)) {\n> \t\t/* Discard \".\" from \"HH:MM:SS.\" */\n\nYeah, is better comment, since num meant different thing in\nthis file.\n\n\n-- \nDanh\n"},{"id":"395974","messageId":"20200423012040.GB1930@danh.dev","threadId":"53221","inReplyTo":"xmqqftcvjv80.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 2/2] date.c: allow compact version of ISO-8601 datetime","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-23T01:20:40Z","receivedAt":"2020-04-23T01:20:45Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-22 10:17:35-0700, Junio C Hamano wrote:\n> Đoàn Trần Công Danh writes:\n> \n> > Signed-off-by: Đoàn Trần Công Danh \n> > ---\n> > date.c | 22 ++++++++++++++++++++++\n> > t/t0006-date.sh | 3 +++\n> > 2 files changed, 25 insertions(+)\n> >\n> > diff --git a/date.c b/date.c\n> > index 62f23b4702..882242c2db 100644\n> > --- a/date.c\n> > +++ b/date.c\n> > @@ -672,6 +672,28 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n> > \t\tn++;\n> > \t} while (isdigit(date[n]));\n> > \n> > +\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n> > +\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n> > +\tif (n == 8 || n == 6) {\n> > +\t\tunsigned int num1 = num / 10000;\n> > +\t\tunsigned int num2 = (num % 10000) / 100;\n> > +\t\tunsigned int num3 = num % 100;\n> > +\t\tif (n == 8 && num1 > 1900 &&\n> > +\t\t num2 > 0 && num2 <= 12 &&\n> > +\t\t num3 > 0 && num3 <= 31) {\n> > +\t\t\ttm->tm_year = num1 - 1900;\n> > +\t\t\ttm->tm_mon = num2 - 1;\n> > +\t\t\ttm->tm_mday = num3;\n> > +\t\t} else if (n == 6 && num1 < 60 && num2 < 60 && num3 <= 60) {\n> > +\t\t\ttm->tm_hour = num1;\n> > +\t\t\ttm->tm_min = num2;\n> > +\t\t\ttm->tm_sec = num3;\n> > +\t\t\tif (*end == '.' && isdigit(end[1]))\n> > +\t\t\t\tstrtoul(end + 1, &end, 10);\n> > +\t\t}\n> > +\t\treturn end - date;\n> > +\t}\n> > +\n> \n> Looks sensible except that on our planet, one day has only 24 hours\n> ;-).\n\nMy bad, I admit that I wouldn't run into this error if we have the\nhelper is_hms (or is_time)\n\n> \n> I think we should try to reuse existing helpers as much as possible\n> in date.c to avoid such stupid errors. During my review of [1/2] I\n> found is_date() would be a good thing to try reusing and also\n\nI'll look into this and see which value should be passed to is_date\n\n> extracted is_hms() as another candidate we could reuse.\n\n-- \nDanh\n"},{"id":"396009","messageId":"cover.1587644889.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586856398.git.congdanhqx@gmail.com","subject":"[PATCH v4 0/4] More ISO-8601 support","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-23T13:52:37Z","receivedAt":"2020-04-23T13:52:50Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"This series aims to extend support for ISO-8601 datetime format\nto allow compact version, and fractional part of ISO-8601.\n\nChanges in v4 from v3:\n* s/is_date/set_date/ the function's name suggest it only does validation,\n but it does more than that. Junio suggested to me to use it for validation,\n When I looked more into it, I think it's better to not use it, and rename\n the function to reduce the confusion\n* Extract the validate and set time to its own function\n* Correct a check for time in compact ISO-8601\n\nChanges in v3 from v2:\n* Add example for fractional parts of second in documentation\n* Add/Fix regression test on 12:34:56.7.days.ago\n\nĐoàn Trần Công Danh (4):\n date.c: s/is_date/set_date/\n date.c: validate and set time in a helper function\n date.c: skip fractional second part of ISO-8601\n date.c: allow compact version of ISO-8601 datetime\n\n Documentation/date-formats.txt | 5 ++-\n date.c | 66 +++++++++++++++++++++++++---------\n t/t0006-date.sh | 6 ++++\n 3 files changed, 59 insertions(+), 18 deletions(-)\n\nRange-diff against v3:\n-: ---------- > 1: 1fe69008fc date.c: s/is_date/set_date/\n-: ---------- > 2: 0d0e4d8edc date.c: validate and set time in a helper function\n1: c6d42785bb ! 3: 8b18d0ee5d date.c: skip fractional second part of ISO-8601\n @@ Commit message\n \n 2008-02-14 20:30:45.019-04:00\n \n + While doing this, make sure that we only interpret the number after the\n + second and the dot as fractional when and only when the date is known,\n + since only ISO-8601 allows the fractional part, and we've taught our\n + users to interpret \"12:34:56.7.days.ago\" as a way to specify a time\n + relative to current time.\n +\n Reported-by: Brian M. Carlson \n Signed-off-by: Đoàn Trần Công Danh \n \n @@ date.c: static int match_multi_number(timestamp_t num, char c, const char *date,\n +\t\tif (num3 < 0) {\n \t\t\tnum3 = 0;\n +\t\t} else if (*end == '.' && isdigit(end[1]) &&\n -+\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 &&\n -+\t\t\t tm->tm_mday != -1) {\n -+\t\t\t/* Attempt to guess meaning of in HHMMSS.\n -+\t\t\t * only interpret as fractional when %Y %m %d is known.\n -+\t\t\t */\n ++\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 && tm->tm_mday != -1 &&\n ++\t\t\t set_time(num, num2, num3, tm) == 0) {\n ++\t\t\t/* %Y%m%d is known, ignore fractional in HHMMSS. */\n +\t\t\tstrtol(end + 1, &end, 10);\n +\t\t}\n - \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n - \t\t\ttm->tm_hour = num;\n - \t\t\ttm->tm_min = num2;\n + \t\tif (set_time(num, num2, num3, tm) == 0)\n + \t\t\tbreak;\n + \t\treturn 0;\n \n ## t/t0006-date.sh ##\n @@ t/t0006-date.sh: check_parse 2008-02 bad\n2: 225b6401bd ! 4: 2812439a26 date.c: allow compact version of ISO-8601 datetime\n @@ date.c: static int match_digit(const char *date, struct tm *tm, int *offset, int\n +\t\tunsigned int num1 = num / 10000;\n +\t\tunsigned int num2 = (num % 10000) / 100;\n +\t\tunsigned int num3 = num % 100;\n -+\t\tif (n == 8 && num1 > 1900 &&\n -+\t\t num2 > 0 && num2 <= 12 &&\n -+\t\t num3 > 0 && num3 <= 31) {\n -+\t\t\ttm->tm_year = num1 - 1900;\n -+\t\t\ttm->tm_mon = num2 - 1;\n -+\t\t\ttm->tm_mday = num3;\n -+\t\t} else if (n == 6 && num1 < 60 && num2 < 60 && num3 <= 60) {\n -+\t\t\ttm->tm_hour = num1;\n -+\t\t\ttm->tm_min = num2;\n -+\t\t\ttm->tm_sec = num3;\n -+\t\t\tif (*end == '.' && isdigit(end[1]))\n -+\t\t\t\tstrtoul(end + 1, &end, 10);\n -+\t\t}\n ++\t\tif (n == 8)\n ++\t\t\tset_date(num1, num2, num3, NULL, time(NULL), tm);\n ++\t\telse if (n == 6 && set_time(num1, num2, num3, tm) == 0 &&\n ++\t\t\t *end == '.' && isdigit(end[1]))\n ++\t\t\tstrtoul(end + 1, &end, 10);\n +\t\treturn end - date;\n +\t}\n +\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396010","messageId":"1fe69008fc79e6a74e8613011504bc7e342291ab.1587644889.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587644889.git.congdanhqx@gmail.com","subject":"[PATCH v4 1/4] date.c: s/is_date/set_date/","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-23T13:52:38Z","receivedAt":"2020-04-23T13:52:52Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"The function is_date, confusingly also set tm_year. tm_mon, and tm_mday\nafter validating input.\n\nRename to set_date to reflect its real usage.\n\nAlso, change return value is 0 on success and -1 on failure following\nour convention on function do some real work.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n date.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex b0d9a8421d..b67c5abe24 100644\n--- a/date.c\n+++ b/date.c\n@@ -497,7 +497,7 @@ static int match_alpha(const char *date, struct tm *tm, int *offset)\n \treturn skip_alpha(date);\n }\n \n-static int is_date(int year, int month, int day, struct tm *now_tm, time_t now, struct tm *tm)\n+static int set_date(int year, int month, int day, struct tm *now_tm, time_t now, struct tm *tm)\n {\n \tif (month > 0 && month < 13 && day > 0 && day < 32) {\n \t\tstruct tm check = *tm;\n@@ -518,9 +518,9 @@ static int is_date(int year, int month, int day, struct tm *now_tm, time_t now,\n \t\telse if (year < 38)\n \t\t\tr->tm_year = year + 100;\n \t\telse\n-\t\t\treturn 0;\n+\t\t\treturn -1;\n \t\tif (!now_tm)\n-\t\t\treturn 1;\n+\t\t\treturn 0;\n \n \t\tspecified = tm_to_time_t(r);\n \n@@ -529,14 +529,14 @@ static int is_date(int year, int month, int day, struct tm *now_tm, time_t now,\n \t\t * sure it is not later than ten days from now...\n \t\t */\n \t\tif ((specified != -1) && (now + 10*24*3600 < specified))\n-\t\t\treturn 0;\n+\t\t\treturn -1;\n \t\ttm->tm_mon = r->tm_mon;\n \t\ttm->tm_mday = r->tm_mday;\n \t\tif (year != -1)\n \t\t\ttm->tm_year = r->tm_year;\n-\t\treturn 1;\n+\t\treturn 0;\n \t}\n-\treturn 0;\n+\treturn -1;\n }\n \n static int match_multi_number(timestamp_t num, char c, const char *date,\n@@ -575,10 +575,10 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \n \t\tif (num > 70) {\n \t\t\t/* yyyy-mm-dd? */\n-\t\t\tif (is_date(num, num2, num3, NULL, now, tm))\n+\t\t\tif (set_date(num, num2, num3, NULL, now, tm) == 0)\n \t\t\t\tbreak;\n \t\t\t/* yyyy-dd-mm? */\n-\t\t\tif (is_date(num, num3, num2, NULL, now, tm))\n+\t\t\tif (set_date(num, num3, num2, NULL, now, tm) == 0)\n \t\t\t\tbreak;\n \t\t}\n \t\t/* Our eastern European friends say dd.mm.yy[yy]\n@@ -586,14 +586,14 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \t\t * mm/dd/yy[yy] form only when separator is not '.'\n \t\t */\n \t\tif (c != '.' &&\n-\t\t is_date(num3, num, num2, refuse_future, now, tm))\n+\t\t set_date(num3, num, num2, refuse_future, now, tm) == 0)\n \t\t\tbreak;\n \t\t/* European dd.mm.yy[yy] or funny US dd/mm/yy[yy] */\n-\t\tif (is_date(num3, num2, num, refuse_future, now, tm))\n+\t\tif (set_date(num3, num2, num, refuse_future, now, tm) == 0)\n \t\t\tbreak;\n \t\t/* Funny European mm.dd.yy */\n \t\tif (c == '.' &&\n-\t\t is_date(num3, num, num2, refuse_future, now, tm))\n+\t\t set_date(num3, num, num2, refuse_future, now, tm) == 0)\n \t\t\tbreak;\n \t\treturn 0;\n \t}\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396011","messageId":"0d0e4d8edce37dfef13e573588f0c043ddf07f6a.1587644889.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587644889.git.congdanhqx@gmail.com","subject":"[PATCH v4 2/4] date.c: validate and set time in a helper function","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-23T13:52:39Z","receivedAt":"2020-04-23T13:52:54Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In a later patch, we will reuse this logic, move it to a helper, now.\n\nWhile we're at it, explicit states that we intentionally ignore\nold-and-defective 2nd leap second.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n date.c | 22 +++++++++++++++++-----\n 1 file changed, 17 insertions(+), 5 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex b67c5abe24..f5d5a91208 100644\n--- a/date.c\n+++ b/date.c\n@@ -539,6 +539,22 @@ static int set_date(int year, int month, int day, struct tm *now_tm, time_t now,\n \treturn -1;\n }\n \n+static int set_time(long hour, long minute, long second, struct tm *tm)\n+{\n+\t/* C90 and old POSIX accepts 2 leap seconds, it's a defect,\n+\t * ignore second number 61\n+\t */\n+\tif (0 <= hour && hour <= 24 &&\n+\t 0 <= minute && minute < 60 &&\n+\t 0 <= second && second <= 60) {\n+\t\ttm->tm_hour = hour;\n+\t\ttm->tm_min = minute;\n+\t\ttm->tm_sec = second;\n+\t\treturn 0;\n+\t}\n+\treturn -1;\n+}\n+\n static int match_multi_number(timestamp_t num, char c, const char *date,\n \t\t\t char *end, struct tm *tm, time_t now)\n {\n@@ -556,12 +572,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \tcase ':':\n \t\tif (num3 < 0)\n \t\t\tnum3 = 0;\n-\t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n-\t\t\ttm->tm_hour = num;\n-\t\t\ttm->tm_min = num2;\n-\t\t\ttm->tm_sec = num3;\n+\t\tif (set_time(num, num2, num3, tm) == 0)\n \t\t\tbreak;\n-\t\t}\n \t\treturn 0;\n \n \tcase '-':\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396012","messageId":"8b18d0ee5d6f08394e54e16ca7618c687791a509.1587644889.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587644889.git.congdanhqx@gmail.com","subject":"[PATCH v4 3/4] date.c: skip fractional second part of ISO-8601","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-23T13:52:40Z","receivedAt":"2020-04-23T13:52:56Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"git-commit(1) says ISO-8601 is one of our supported date format.\n\nISO-8601 allows timestamps to have a fractional number of seconds.\nWe represent time only in terms of whole seconds, so we never bothered\nparsing fractional seconds. However, it's better for us to parse and\nthrow away the fractional part than to refuse to parse the timestamp\nat all.\n\nAnd refusing parsing fractional second part may confuse the parse to\nthink fractional and timezone as day and month in this example:\n\n\t2008-02-14 20:30:45.019-04:00\n\nWhile doing this, make sure that we only interpret the number after the\nsecond and the dot as fractional when and only when the date is known,\nsince only ISO-8601 allows the fractional part, and we've taught our\nusers to interpret \"12:34:56.7.days.ago\" as a way to specify a time\nrelative to current time.\n\nReported-by: Brian M. Carlson \nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/date-formats.txt | 5 ++++-\n date.c | 8 +++++++-\n t/t0006-date.sh | 3 +++\n 3 files changed, 14 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt\nindex 6926e0a4c8..7e7eaba643 100644\n--- a/Documentation/date-formats.txt\n+++ b/Documentation/date-formats.txt\n@@ -20,7 +20,10 @@ RFC 2822::\n ISO 8601::\n \tTime and date specified by the ISO 8601 standard, for example\n \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n-\t`T` character as well.\n+\t`T` character as well. Fractional parts of a second will be ignored,\n+\tfor example `2005-04-07T22:13:13.019` will be treated as\n+\t`2005-04-07T22:13:13`\n+\n +\n NOTE: In addition, the date part is accepted in the following formats:\n `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\ndiff --git a/date.c b/date.c\nindex f5d5a91208..5d635031e0 100644\n--- a/date.c\n+++ b/date.c\n@@ -570,8 +570,14 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \t/* Time? Date? */\n \tswitch (c) {\n \tcase ':':\n-\t\tif (num3 < 0)\n+\t\tif (num3 < 0) {\n \t\t\tnum3 = 0;\n+\t\t} else if (*end == '.' && isdigit(end[1]) &&\n+\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 && tm->tm_mday != -1 &&\n+\t\t\t set_time(num, num2, num3, tm) == 0) {\n+\t\t\t/* %Y%m%d is known, ignore fractional in HHMMSS. */\n+\t\t\tstrtol(end + 1, &end, 10);\n+\t\t}\n \t\tif (set_time(num, num2, num3, tm) == 0)\n \t\t\tbreak;\n \t\treturn 0;\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex d9fcc829a9..80917c81c3 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -81,6 +81,8 @@ check_parse 2008-02 bad\n check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n@@ -103,6 +105,7 @@ check_approxidate 5.seconds.ago '2009-08-30 19:19:55'\n check_approxidate 10.minutes.ago '2009-08-30 19:10:00'\n check_approxidate yesterday '2009-08-29 19:20:00'\n check_approxidate 3.days.ago '2009-08-27 19:20:00'\n+check_approxidate '12:34:56.3.days.ago' '2009-08-27 12:34:56'\n check_approxidate 3.weeks.ago '2009-08-09 19:20:00'\n check_approxidate 3.months.ago '2009-05-30 19:20:00'\n check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396013","messageId":"2812439a26c3c98150bb7be3c6e603e2ed95fab3.1587644889.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587644889.git.congdanhqx@gmail.com","subject":"[PATCH v4 4/4] date.c: allow compact version of ISO-8601 datetime","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-23T13:52:41Z","receivedAt":"2020-04-23T13:52:58Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh \n---\n date.c | 14 ++++++++++++++\n t/t0006-date.sh | 3 +++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/date.c b/date.c\nindex 5d635031e0..0c66e5a5ae 100644\n--- a/date.c\n+++ b/date.c\n@@ -682,6 +682,20 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n \t\tn++;\n \t} while (isdigit(date[n]));\n \n+\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n+\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n+\tif (n == 8 || n == 6) {\n+\t\tunsigned int num1 = num / 10000;\n+\t\tunsigned int num2 = (num % 10000) / 100;\n+\t\tunsigned int num3 = num % 100;\n+\t\tif (n == 8)\n+\t\t\tset_date(num1, num2, num3, NULL, time(NULL), tm);\n+\t\telse if (n == 6 && set_time(num1, num2, num3, tm) == 0 &&\n+\t\t\t *end == '.' && isdigit(end[1]))\n+\t\t\tstrtoul(end + 1, &end, 10);\n+\t\treturn end - date;\n+\t}\n+\n \t/* Four-digit year or a timezone? */\n \tif (n == 4) {\n \t\tif (num <= 1400 && *offset == -1) {\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 80917c81c3..75ee9a96b8 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -82,6 +82,9 @@ check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396024","messageId":"xmqqmu72gfxu.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"20200423011812.GA1930@danh.dev","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-23T19:28:13Z","receivedAt":"2020-04-23T19:28:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Danh Doan writes:\n\n>> > \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n>> > \t\t\ttm->tm_hour = num;\n>> > \t\t\ttm->tm_min = num2;\n>> \n>> And after all that is done, if (and others) are within a\n>> reasonable range, we use that as HH:MM:SS. \n>> \n>> OK. If (or , or even for that matter) weren't\n>> reasonable, is it still sensible to discard the fractional part?\n>> The answer is not immediately obvious to me.\n>> \n>> To be safe, it might make sense to extract a helper function from\n>> the next conditional, i.e.\n>> \n>> static int is_hms(long num1, long num2, long num3)\n>\n> I'll make it `is_time` on par with is_date check.\n\nThat is probably a lot more readable name than is_hms(). \n\nI do not worry too much if the name is not \"on par with\" it, though,\nbecause is_date() does more than just \"check\", as you noticed below,\nunlike the \"is hour-minute-seconds are within reasonable range?\"\ncheck.\n\n> I'll look into it and check if int or long is suitable for parameter's\n> type.\n>\n>> {\n>> \treturn (0 <= num1 && num1 < 25 &&\n>> \t\t0 <= num2 && num2 < 60 &&\n>> \t\t0 <= num3 && num3 <= 60);\n>\n> Does it worth to add an explicit comment that we intentionally ignore\n> the old-and-defective 2nd leap seconds (i.e. second with value 61)?\n>\n> I saw in one of your previous email doubting if we should check for\n> `num3 <= 61` or not.\n\nI wrote that without checking anything, even what our own code does.\nAs the existing code seems to want to work only with a second part\nthat is 60 or less, not allowing a minute with 62 seconds, I think\nsticking to that and saying \"0 <= num3 && num3 <= 60\" is good.\n\n>> }\n>> \n>> and use it in the new \"else if\" block like so?\n>> \n>> \n>> \t} else if (*end == '.' && isdigit(end[1]) &&\n>> \t\t is_date(tm->tm_year, tm->tm_mon, tm->tm_mday, NULL, now, tm) &&\n>\n> When running into this, the code patch for non-approxidate hasn't\n> initialised value for now, yet.\n\nWe may want to separate the logic that relies on the value of 'now'\nand 'now_tm->tm_year' out of is_date() to make it more easily\nreusable. In this generic codepath, for example, we do not\nnecessarily want to say \"we refuse to parse timestamp that is 10\ndays or more into the future\".\n\nThe longer I stare at is_date(), the more I am inclined to say it is\na bit of impedance mismatch, and we instead should have the is_hms()\nhelper as I suggested in the message you are responding to, plus\nsomething like:\n\nstatic int is_ymd(int year, int month, int day)\n{\n /* like tm_to_time_t(), we only work between 1970-2099 */\n\tstatic const int days_in_month[] = {\n\t\t31, 28, 31, 30, ..., 31\n\t};\n\n\treturn !(year < 1970 || 2100 <= year ||\n\t\tmonth <= 0 || 13 <= month ||\n\t\tday <= 0 || year / 4 + days_in_month[month-1] <= day);\n}\n\nand use it here. I am not sure if we can reuse this inside\nis_date(), but if we can do so that would be good (and if we cannot,\nthat is fine, too).\n\nThanks.\n"},{"id":"396028","messageId":"xmqq5zdqge2g.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"1fe69008fc79e6a74e8613011504bc7e342291ab.1587644889.git.congdanhqx@gmail.com","subject":"Re: [PATCH v4 1/4] date.c: s/is_date/set_date/","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-23T20:08:39Z","receivedAt":"2020-04-23T20:08:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> The function is_date, confusingly also set tm_year. tm_mon, and tm_mday\n> after validating input.\n>\n> Rename to set_date to reflect its real usage.\n\nYup, I think I can agree with the reasoning here. It is a good\nclean-up regardless of the remainder of the series.\n\n"},{"id":"396029","messageId":"xmqqzhb2ez1q.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"0d0e4d8edce37dfef13e573588f0c043ddf07f6a.1587644889.git.congdanhqx@gmail.com","subject":"Re: [PATCH v4 2/4] date.c: validate and set time in a helper function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-23T20:18:25Z","receivedAt":"2020-04-23T20:18:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> In a later patch, we will reuse this logic, move it to a helper, now.\n>\n> While we're at it, explicit states that we intentionally ignore\n\n\"explicitly state\", perhaps.\n\n> old-and-defective 2nd leap second.\n>\n> Signed-off-by: Đoàn Trần Công Danh \n> ---\n> date.c | 22 +++++++++++++++++-----\n> 1 file changed, 17 insertions(+), 5 deletions(-)\n>\n> diff --git a/date.c b/date.c\n> index b67c5abe24..f5d5a91208 100644\n> --- a/date.c\n> +++ b/date.c\n> @@ -539,6 +539,22 @@ static int set_date(int year, int month, int day, struct tm *now_tm, time_t now,\n> \treturn -1;\n> }\n> \n> +static int set_time(long hour, long minute, long second, struct tm *tm)\n> +{\n> +\t/* C90 and old POSIX accepts 2 leap seconds, it's a defect,\n> +\t * ignore second number 61\n> +\t */\n\n\t/*\n\t * Style: our multi-line comments ought to be\n\t * formatted like this. Slash-asterisk that opens,\n\t * and asterisk-slash that closes, are both on their\n\t * own lines.\n\t */\n\nBut I am not sure we want to even have a new comment here. After\nall we are extracting/reinventing exactly the same logic as the\noriginal. Why we allow \"60\" might be worth commenting, but if a\nminute that has 62 seconds is a mere historical curiosity, then is\nit worth explaining why \"61\", which we never even wrote in the code,\nis missing from here?\n\n> +\tif (0 <= hour && hour <= 24 &&\n> +\t 0 <= minute && minute < 60 &&\n> +\t 0 <= second && second <= 60) {\n> +\t\ttm->tm_hour = hour;\n> +\t\ttm->tm_min = minute;\n> +\t\ttm->tm_sec = second;\n> +\t\treturn 0;\n> +\t}\n> +\treturn -1;\n> +}\n\nI am a bit surprised to see that you chose to unify with the \"check\nand set\" interface of is_date (now set_date). I was expecting to\nsee that we'd have \"check-only\" helper functions.\n\nThis is not a complaint, at least not yet until we see the result of\nusing it in new code; it may very well be possible that the \"check\nand set\" interface would make the new caller(s) clearer.\n\n> static int match_multi_number(timestamp_t num, char c, const char *date,\n> \t\t\t char *end, struct tm *tm, time_t now)\n> {\n> @@ -556,12 +572,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> \tcase ':':\n> \t\tif (num3 < 0)\n> \t\t\tnum3 = 0;\n> -\t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n> -\t\t\ttm->tm_hour = num;\n> -\t\t\ttm->tm_min = num2;\n> -\t\t\ttm->tm_sec = num3;\n> +\t\tif (set_time(num, num2, num3, tm) == 0)\n> \t\t\tbreak;\n> -\t\t}\n> \t\treturn 0;\n\nThis caller does become easier to follow, I would say. Nicely done.\n\n> \tcase '-':\n"},{"id":"396030","messageId":"xmqqv9lqeyji.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"8b18d0ee5d6f08394e54e16ca7618c687791a509.1587644889.git.congdanhqx@gmail.com","subject":"Re: [PATCH v4 3/4] date.c: skip fractional second part of ISO-8601","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-23T20:29:21Z","receivedAt":"2020-04-23T20:29:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Đoàn Trần Công Danh writes:\n\n> -\t\tif (num3 < 0)\n> +\t\tif (num3 < 0) {\n> \t\t\tnum3 = 0;\n> +\t\t} else if (*end == '.' && isdigit(end[1]) &&\n> +\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 && tm->tm_mday != -1 &&\n> +\t\t\t set_time(num, num2, num3, tm) == 0) {\n> +\t\t\t/* %Y%m%d is known, ignore fractional in HHMMSS. */\n> +\t\t\tstrtol(end + 1, &end, 10);\n> +\t\t}\n> \t\tif (set_time(num, num2, num3, tm) == 0)\n\nHmmm. \n\nWhile calling set_time() on the same 4-tuple might be idempotent\nwhen it succeeds, the call with a potential new side effect in the\nconditional part leaves a bad taste in my mouth.\n\nI am wondering if the follwoing would be a more readable\nalternative.\n\n date.c | 10 +++++++++-\n 1 file changed, 9 insertions(+), 1 deletion(-)\n\ndiff --git a/date.c b/date.c\nindex f5d5a91208..3eb9b6032c 100644\n--- a/date.c\n+++ b/date.c\n@@ -572,8 +572,16 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \tcase ':':\n \t\tif (num3 < 0)\n \t\t\tnum3 = 0;\n-\t\tif (set_time(num, num2, num3, tm) == 0)\n+\t\tif (set_time(num, num2, num3, tm) == 0) {\n+\t\t\t/* \n+\t\t\t * Is the \"HH:MM:SS\" we just parsed followed by\n+\t\t\t * \".\" (fractinal second)? Discard it\n+\t\t\t * only when we already have YY/MM/DD.\n+\t\t\t */\n+\t\t\tif (*end == '.' && isdigit(end[1]) && date_known(tm))\n+\t\t\t\t(void) strtol(end + 1, &end, 10);\n \t\t\tbreak;\n+\t\t}\n \t\treturn 0;\n \n \tcase '-':\n"},{"id":"396032","messageId":"1861c472-7756-d433-9185-d83c03d72b9b@iee.email","threadId":"53221","inReplyTo":"xmqqmu72gfxu.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-04-23T20:41:49Z","receivedAt":"2020-04-23T20:41:57Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 23/04/2020 20:28, Junio C Hamano wrote:\n> Danh Doan writes:\n>\n>>>> \t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n>>>> \t\t\ttm->tm_hour = num;\n>>>> \t\t\ttm->tm_min = num2;\n>>> And after all that is done, if (and others) are within a\n>>> reasonable range, we use that as HH:MM:SS. \n>>>\n>>> OK. If (or , or even for that matter) weren't\n>>> reasonable, is it still sensible to discard the fractional part?\n>>> The answer is not immediately obvious to me.\n>>>\n>>> To be safe, it might make sense to extract a helper function from\n>>> the next conditional, i.e.\n>>>\n>>> static int is_hms(long num1, long num2, long num3)\n>> I'll make it `is_time` on par with is_date check.\n> That is probably a lot more readable name than is_hms(). \n\nI didn't immediately recognise hms and ymd (below) as abbreviations.\n>\n> I do not worry too much if the name is not \"on par with\" it, though,\n> because is_date() does more than just \"check\", as you noticed below,\n> unlike the \"is hour-minute-seconds are within reasonable range?\"\n> check.\n>\n>> I'll look into it and check if int or long is suitable for parameter's\n>> type.\n>>\n>>> {\n>>> \treturn (0 <= num1 && num1 < 25 &&\n>>> \t\t0 <= num2 && num2 < 60 &&\n>>> \t\t0 <= num3 && num3 <= 60);\n>> Does it worth to add an explicit comment that we intentionally ignore\n>> the old-and-defective 2nd leap seconds (i.e. second with value 61)?\n>>\n>> I saw in one of your previous email doubting if we should check for\n>> `num3 <= 61` or not.\n> I wrote that without checking anything, even what our own code does.\n> As the existing code seems to want to work only with a second part\n> that is 60 or less, not allowing a minute with 62 seconds, I think\n> sticking to that and saying \"0 <= num3 && num3 <= 60\" is good.\n>\n>>> }\n>>>\n>>> and use it in the new \"else if\" block like so?\n>>>\n>>>\n>>> \t} else if (*end == '.' && isdigit(end[1]) &&\n>>> \t\t is_date(tm->tm_year, tm->tm_mon, tm->tm_mday, NULL, now, tm) &&\n>> When running into this, the code patch for non-approxidate hasn't\n>> initialised value for now, yet.\n> We may want to separate the logic that relies on the value of 'now'\n> and 'now_tm->tm_year' out of is_date() to make it more easily\n> reusable. In this generic codepath, for example, we do not\n> necessarily want to say \"we refuse to parse timestamp that is 10\n> days or more into the future\".\n>\n> The longer I stare at is_date(), the more I am inclined to say it is\n> a bit of impedance mismatch, and we instead should have the is_hms()\n> helper as I suggested in the message you are responding to, plus\n> something like:\n\nWould is_hhmmss() and is_yyyymmdd() be more obvious abbreviations for\nmost readers?\n\nNow that I type them, they do feel that bit too long... , as naming is\nhard, maybe stick with the yms and hms, though I do keep wanting to type\nytd for the former..\n>\n> static int is_ymd(int year, int month, int day)\n> {\n> /* like tm_to_time_t(), we only work between 1970-2099 */\n> \tstatic const int days_in_month[] = {\n> \t\t31, 28, 31, 30, ..., 31\n> \t};\n>\n> \treturn !(year < 1970 || 2100 <= year ||\n> \t\tmonth <= 0 || 13 <= month ||\n> \t\tday <= 0 || year / 4 + days_in_month[month-1] <= day);\n> }\n>\n> and use it here. I am not sure if we can reuse this inside\n> is_date(), but if we can do so that would be good (and if we cannot,\n> that is fine, too).\n>\n> Thanks.\n--\nPhilip\n"},{"id":"396075","messageId":"20200424000707.GB1949@danh.dev","threadId":"53221","inReplyTo":"1861c472-7756-d433-9185-d83c03d72b9b@iee.email","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-24T00:07:07Z","receivedAt":"2020-04-24T00:07:12Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-23 21:41:49+0100, Philip Oakley wrote:\n> On 23/04/2020 20:28, Junio C Hamano wrote:\n> > Danh Doan writes:\n> Would is_hhmmss() and is_yyyymmdd() be more obvious abbreviations for\n> most readers?\n> \n> Now that I type them, they do feel that bit too long... , as naming is\n> hard, maybe stick with the yms and hms, though I do keep wanting to type\n> ytd for the former..\n\nNot sure if I interpret your opinion correctly,\nDid you mean s/yms/ymd/ and s/ytd/ymd/?\n\nEven that, I couldn't grasp the meaning of the last phase?\n\n-- \nDanh\n"},{"id":"396082","messageId":"xmqqh7x9emn7.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"20200424000707.GB1949@danh.dev","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-24T00:46:20Z","receivedAt":"2020-04-24T00:46:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Danh Doan writes:\n\n> On 2020-04-23 21:41:49+0100, Philip Oakley wrote:\n>> On 23/04/2020 20:28, Junio C Hamano wrote:\n>> > Danh Doan writes:\n>> Would is_hhmmss() and is_yyyymmdd() be more obvious abbreviations for\n>> most readers?\n>> \n>> Now that I type them, they do feel that bit too long... , as naming is\n>> hard, maybe stick with the yms and hms, though I do keep wanting to type\n>> ytd for the former..\n>\n> Not sure if I interpret your opinion correctly,\n> Did you mean s/yms/ymd/ and s/ytd/ymd/?\n>\n> Even that, I couldn't grasp the meaning of the last phase?\n\nHere is how I understood it.\n\nPhilip thinks, and I admit I have to agree with, that HMS would not\nbe understood as hour-minute-seconds by most people, and YMD would\nnot be as yearh-month-day, either.\n\nHis \"yms\" in \"stick with the yms and hms\" is a typo of \"ymd\". He is\nsaying that even though YYMMDD and HHMMSS would look a lot more\nnatural, it is too long to type so YMD and HMS may not be so\nterrible a compromise.\n\nWith the \"ytd\" in the last one, he is saying that another downside\nof saying \"ymd\" (other than that it is not how we usually spell\nyear-month-date), even though \"ymd\" might be an acceptable\ncompromise, is that it is too easy to get confused with year-to-date\nthat is commonly abbreviated as \"YTD\".\n"},{"id":"396124","messageId":"20200424114302.GG1949@danh.dev","threadId":"53221","inReplyTo":"xmqqzhb2ez1q.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v4 2/4] date.c: validate and set time in a helper function","fromName":"Danh Doan","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-24T11:43:02Z","receivedAt":"2020-04-24T11:43:09Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"On 2020-04-23 13:18:25-0700, Junio C Hamano wrote:\n> Đoàn Trần Công Danh writes:\n> \n> > In a later patch, we will reuse this logic, move it to a helper, now.\n> >\n> > While we're at it, explicit states that we intentionally ignore\n> \n> \"explicitly state\", perhaps.\n> \n> > old-and-defective 2nd leap second.\n> >\n> > Signed-off-by: Đoàn Trần Công Danh \n> > ---\n> > date.c | 22 +++++++++++++++++-----\n> > 1 file changed, 17 insertions(+), 5 deletions(-)\n> >\n> > diff --git a/date.c b/date.c\n> > index b67c5abe24..f5d5a91208 100644\n> > --- a/date.c\n> > +++ b/date.c\n> > @@ -539,6 +539,22 @@ static int set_date(int year, int month, int day, struct tm *now_tm, time_t now,\n> > \treturn -1;\n> > }\n> > \n> > +static int set_time(long hour, long minute, long second, struct tm *tm)\n> > +{\n> > +\t/* C90 and old POSIX accepts 2 leap seconds, it's a defect,\n> > +\t * ignore second number 61\n> > +\t */\n> \n> \t/*\n> \t * Style: our multi-line comments ought to be\n> \t * formatted like this. Slash-asterisk that opens,\n> \t * and asterisk-slash that closes, are both on their\n> \t * own lines.\n> \t */\n> \n> But I am not sure we want to even have a new comment here. After\n> all we are extracting/reinventing exactly the same logic as the\n> original. Why we allow \"60\" might be worth commenting, but if a\n> minute that has 62 seconds is a mere historical curiosity, then is\n> it worth explaining why \"61\", which we never even wrote in the code,\n> is missing from here?\n\nI think single line like:\n\n\t/* We accept 61st second for the single? leap second */\n\nOr something along the time, is good enough. Not sure if we want the\nword \"single\" there, though.\n\nI think majority of people don't even know about leap second.\nProbability that know about 62nd second is rarer, I think.\n\n> > +\tif (0 <= hour && hour <= 24 &&\n> > +\t 0 <= minute && minute < 60 &&\n> > +\t 0 <= second && second <= 60) {\n> > +\t\ttm->tm_hour = hour;\n> > +\t\ttm->tm_min = minute;\n> > +\t\ttm->tm_sec = second;\n> > +\t\treturn 0;\n> > +\t}\n> > +\treturn -1;\n> > +}\n> \n> I am a bit surprised to see that you chose to unify with the \"check\n> and set\" interface of is_date (now set_date). I was expecting to\n> see that we'd have \"check-only\" helper functions.\n> \n> This is not a complaint, at least not yet until we see the result of\n> using it in new code; it may very well be possible that the \"check\n> and set\" interface would make the new caller(s) clearer.\n> \n> > static int match_multi_number(timestamp_t num, char c, const char *date,\n> > \t\t\t char *end, struct tm *tm, time_t now)\n> > {\n> > @@ -556,12 +572,8 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n> > \tcase ':':\n> > \t\tif (num3 < 0)\n> > \t\t\tnum3 = 0;\n> > -\t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n> > -\t\t\ttm->tm_hour = num;\n> > -\t\t\ttm->tm_min = num2;\n> > -\t\t\ttm->tm_sec = num3;\n> > +\t\tif (set_time(num, num2, num3, tm) == 0)\n> > \t\t\tbreak;\n> > -\t\t}\n> > \t\treturn 0;\n> \n> This caller does become easier to follow, I would say. Nicely done.\n\nYes, when I looked around date.c\n\nI saw that the only usecase for validate time is for setting it.\nAnd the incoming patch also has that usage.\n\nI chose to unify those code path to not buy me too much trouble.\n\nI'll take that \"Nicely done\" means this unification is OK for this\nseries.\n\n-- \nDanh\n"},{"id":"396134","messageId":"cover.1587740682.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1586856398.git.congdanhqx@gmail.com","subject":"[PATCH v5 0/4] More ISO-8601 support","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-24T15:07:28Z","receivedAt":"2020-04-24T15:07:41Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"This series aims to extend support for ISO-8601 datetime format\nto allow compact version, and fractional part of ISO-8601.\n\nChange in v5 from v4:\n* cleanup [3/4] following Junio's suggestion.\n\nChanges in v4 from v3:\n* s/is_date/set_date/ the function's name suggest it only does validation,\n but it does more than that. Junio suggested to me to use it for validation,\n When I looked more into it, I think it's better to not use it, and rename\n the function to reduce the confusion\n* Extract the validate and set time to its own function\n* Correct a check for time in compact ISO-8601\n\nChanges in v3 from v2:\n* Add example for fractional parts of second in documentation\n* Add/Fix regression test on 12:34:56.7.days.ago\n\nĐoàn Trần Công Danh (4):\n date.c: s/is_date/set_date/\n date.c: validate and set time in a helper function\n date.c: skip fractional second part of ISO-8601\n date.c: allow compact version of ISO-8601 datetime\n\nĐoàn Trần Công Danh (4):\n date.c: s/is_date/set_date/\n date.c: validate and set time in a helper function\n date.c: skip fractional second part of ISO-8601\n date.c: allow compact version of ISO-8601 datetime\n\n Documentation/date-formats.txt | 5 ++-\n date.c | 67 ++++++++++++++++++++++++++--------\n t/t0006-date.sh | 6 +++\n 3 files changed, 62 insertions(+), 16 deletions(-)\n\nRange-diff against v4:\n1: 1fe69008fc = 1: 1fe69008fc date.c: s/is_date/set_date/\n2: 0d0e4d8edc ! 2: a6b97b19f2 date.c: validate and set time in a helper function\n @@ Commit message\n \n Signed-off-by: Đoàn Trần Công Danh \n \n +\n + ## Notes ##\n + I intentionally leave a pair of bracket around if (set_time(...))\n + to reduce the noise in next patch\n +\n ## date.c ##\n @@ date.c: static int set_date(int year, int month, int day, struct tm *now_tm, time_t now,\n \treturn -1;\n @@ date.c: static int set_date(int year, int month, int day, struct tm *now_tm, tim\n \n +static int set_time(long hour, long minute, long second, struct tm *tm)\n +{\n -+\t/* C90 and old POSIX accepts 2 leap seconds, it's a defect,\n -+\t * ignore second number 61\n -+\t */\n ++\t/* We accept 61st second because of leap second */\n +\tif (0 <= hour && hour <= 24 &&\n +\t 0 <= minute && minute < 60 &&\n +\t 0 <= second && second <= 60) {\n @@ date.c: static int match_multi_number(timestamp_t num, char c, const char *date,\n -\t\t\ttm->tm_hour = num;\n -\t\t\ttm->tm_min = num2;\n -\t\t\ttm->tm_sec = num3;\n -+\t\tif (set_time(num, num2, num3, tm) == 0)\n ++\t\tif (set_time(num, num2, num3, tm) == 0) {\n \t\t\tbreak;\n --\t\t}\n + \t\t}\n \t\treturn 0;\n - \n - \tcase '-':\n3: 8b18d0ee5d ! 3: f21aa2dcf5 date.c: skip fractional second part of ISO-8601\n @@ Commit message\n relative to current time.\n \n Reported-by: Brian M. Carlson \n + Helped-by: Junio C Hamano \n Signed-off-by: Đoàn Trần Công Danh \n \n ## Documentation/date-formats.txt ##\n @@ Documentation/date-formats.txt: RFC 2822::\n `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\n \n ## date.c ##\n +@@ date.c: static int set_time(long hour, long minute, long second, struct tm *tm)\n + \treturn -1;\n + }\n + \n ++static int is_date_known(struct tm *tm)\n ++{\n ++\treturn tm->tm_year != -1 && tm->tm_mon != -1 && tm->tm_mday != -1;\n ++}\n ++\n + static int match_multi_number(timestamp_t num, char c, const char *date,\n + \t\t\t char *end, struct tm *tm, time_t now)\n + {\n @@ date.c: static int match_multi_number(timestamp_t num, char c, const char *date,\n - \t/* Time? Date? */\n - \tswitch (c) {\n - \tcase ':':\n --\t\tif (num3 < 0)\n -+\t\tif (num3 < 0) {\n + \t\tif (num3 < 0)\n \t\t\tnum3 = 0;\n -+\t\t} else if (*end == '.' && isdigit(end[1]) &&\n -+\t\t\t tm->tm_year != -1 && tm->tm_mon != -1 && tm->tm_mday != -1 &&\n -+\t\t\t set_time(num, num2, num3, tm) == 0) {\n -+\t\t\t/* %Y%m%d is known, ignore fractional in HHMMSS. */\n -+\t\t\tstrtol(end + 1, &end, 10);\n -+\t\t}\n - \t\tif (set_time(num, num2, num3, tm) == 0)\n + \t\tif (set_time(num, num2, num3, tm) == 0) {\n ++\t\t\t/*\n ++\t\t\t * If %H:%M:%S was just parsed followed by: .\n ++\t\t\t * Consider (& discard) it as fractional second\n ++\t\t\t * if %Y%m%d is parsed before.\n ++\t\t\t */\n ++\t\t\tif (*end == '.' && isdigit(end[1]) && is_date_known(tm))\n ++\t\t\t\tstrtol(end + 1, &end, 10);\n \t\t\tbreak;\n + \t\t}\n \t\treturn 0;\n \n ## t/t0006-date.sh ##\n4: 2812439a26 = 4: 51aa60c069 date.c: allow compact version of ISO-8601 datetime\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396135","messageId":"1fe69008fc79e6a74e8613011504bc7e342291ab.1587740682.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587740682.git.congdanhqx@gmail.com","subject":"[PATCH v5 1/4] date.c: s/is_date/set_date/","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-24T15:07:29Z","receivedAt":"2020-04-24T15:07:43Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"The function is_date, confusingly also set tm_year. tm_mon, and tm_mday\nafter validating input.\n\nRename to set_date to reflect its real usage.\n\nAlso, change return value is 0 on success and -1 on failure following\nour convention on function do some real work.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n date.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex b0d9a8421d..b67c5abe24 100644\n--- a/date.c\n+++ b/date.c\n@@ -497,7 +497,7 @@ static int match_alpha(const char *date, struct tm *tm, int *offset)\n \treturn skip_alpha(date);\n }\n \n-static int is_date(int year, int month, int day, struct tm *now_tm, time_t now, struct tm *tm)\n+static int set_date(int year, int month, int day, struct tm *now_tm, time_t now, struct tm *tm)\n {\n \tif (month > 0 && month < 13 && day > 0 && day < 32) {\n \t\tstruct tm check = *tm;\n@@ -518,9 +518,9 @@ static int is_date(int year, int month, int day, struct tm *now_tm, time_t now,\n \t\telse if (year < 38)\n \t\t\tr->tm_year = year + 100;\n \t\telse\n-\t\t\treturn 0;\n+\t\t\treturn -1;\n \t\tif (!now_tm)\n-\t\t\treturn 1;\n+\t\t\treturn 0;\n \n \t\tspecified = tm_to_time_t(r);\n \n@@ -529,14 +529,14 @@ static int is_date(int year, int month, int day, struct tm *now_tm, time_t now,\n \t\t * sure it is not later than ten days from now...\n \t\t */\n \t\tif ((specified != -1) && (now + 10*24*3600 < specified))\n-\t\t\treturn 0;\n+\t\t\treturn -1;\n \t\ttm->tm_mon = r->tm_mon;\n \t\ttm->tm_mday = r->tm_mday;\n \t\tif (year != -1)\n \t\t\ttm->tm_year = r->tm_year;\n-\t\treturn 1;\n+\t\treturn 0;\n \t}\n-\treturn 0;\n+\treturn -1;\n }\n \n static int match_multi_number(timestamp_t num, char c, const char *date,\n@@ -575,10 +575,10 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \n \t\tif (num > 70) {\n \t\t\t/* yyyy-mm-dd? */\n-\t\t\tif (is_date(num, num2, num3, NULL, now, tm))\n+\t\t\tif (set_date(num, num2, num3, NULL, now, tm) == 0)\n \t\t\t\tbreak;\n \t\t\t/* yyyy-dd-mm? */\n-\t\t\tif (is_date(num, num3, num2, NULL, now, tm))\n+\t\t\tif (set_date(num, num3, num2, NULL, now, tm) == 0)\n \t\t\t\tbreak;\n \t\t}\n \t\t/* Our eastern European friends say dd.mm.yy[yy]\n@@ -586,14 +586,14 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \t\t * mm/dd/yy[yy] form only when separator is not '.'\n \t\t */\n \t\tif (c != '.' &&\n-\t\t is_date(num3, num, num2, refuse_future, now, tm))\n+\t\t set_date(num3, num, num2, refuse_future, now, tm) == 0)\n \t\t\tbreak;\n \t\t/* European dd.mm.yy[yy] or funny US dd/mm/yy[yy] */\n-\t\tif (is_date(num3, num2, num, refuse_future, now, tm))\n+\t\tif (set_date(num3, num2, num, refuse_future, now, tm) == 0)\n \t\t\tbreak;\n \t\t/* Funny European mm.dd.yy */\n \t\tif (c == '.' &&\n-\t\t is_date(num3, num, num2, refuse_future, now, tm))\n+\t\t set_date(num3, num, num2, refuse_future, now, tm) == 0)\n \t\t\tbreak;\n \t\treturn 0;\n \t}\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396136","messageId":"a6b97b19f2b3177ad3d7460a024ce08d6f5f0c55.1587740682.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587740682.git.congdanhqx@gmail.com","subject":"[PATCH v5 2/4] date.c: validate and set time in a helper function","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-24T15:07:30Z","receivedAt":"2020-04-24T15:07:44Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"In a later patch, we will reuse this logic, move it to a helper, now.\n\nWhile we're at it, explicit states that we intentionally ignore\nold-and-defective 2nd leap second.\n\nSigned-off-by: Đoàn Trần Công Danh \n---\n\nNotes:\n I intentionally leave a pair of bracket around if (set_time(...))\n to reduce the noise in next patch\n\n date.c | 19 +++++++++++++++----\n 1 file changed, 15 insertions(+), 4 deletions(-)\n\ndiff --git a/date.c b/date.c\nindex b67c5abe24..fa39e5e8a5 100644\n--- a/date.c\n+++ b/date.c\n@@ -539,6 +539,20 @@ static int set_date(int year, int month, int day, struct tm *now_tm, time_t now,\n \treturn -1;\n }\n \n+static int set_time(long hour, long minute, long second, struct tm *tm)\n+{\n+\t/* We accept 61st second because of leap second */\n+\tif (0 <= hour && hour <= 24 &&\n+\t 0 <= minute && minute < 60 &&\n+\t 0 <= second && second <= 60) {\n+\t\ttm->tm_hour = hour;\n+\t\ttm->tm_min = minute;\n+\t\ttm->tm_sec = second;\n+\t\treturn 0;\n+\t}\n+\treturn -1;\n+}\n+\n static int match_multi_number(timestamp_t num, char c, const char *date,\n \t\t\t char *end, struct tm *tm, time_t now)\n {\n@@ -556,10 +570,7 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \tcase ':':\n \t\tif (num3 < 0)\n \t\t\tnum3 = 0;\n-\t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n-\t\t\ttm->tm_hour = num;\n-\t\t\ttm->tm_min = num2;\n-\t\t\ttm->tm_sec = num3;\n+\t\tif (set_time(num, num2, num3, tm) == 0) {\n \t\t\tbreak;\n \t\t}\n \t\treturn 0;\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396137","messageId":"f21aa2dcf507f9eb13a623b6718a170d2661f15d.1587740682.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587740682.git.congdanhqx@gmail.com","subject":"[PATCH v5 3/4] date.c: skip fractional second part of ISO-8601","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-24T15:07:31Z","receivedAt":"2020-04-24T15:07:47Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"git-commit(1) says ISO-8601 is one of our supported date format.\n\nISO-8601 allows timestamps to have a fractional number of seconds.\nWe represent time only in terms of whole seconds, so we never bothered\nparsing fractional seconds. However, it's better for us to parse and\nthrow away the fractional part than to refuse to parse the timestamp\nat all.\n\nAnd refusing parsing fractional second part may confuse the parse to\nthink fractional and timezone as day and month in this example:\n\n\t2008-02-14 20:30:45.019-04:00\n\nWhile doing this, make sure that we only interpret the number after the\nsecond and the dot as fractional when and only when the date is known,\nsince only ISO-8601 allows the fractional part, and we've taught our\nusers to interpret \"12:34:56.7.days.ago\" as a way to specify a time\nrelative to current time.\n\nReported-by: Brian M. Carlson \nHelped-by: Junio C Hamano \nSigned-off-by: Đoàn Trần Công Danh \n---\n Documentation/date-formats.txt | 5 ++++-\n date.c | 12 ++++++++++++\n t/t0006-date.sh | 3 +++\n 3 files changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/date-formats.txt b/Documentation/date-formats.txt\nindex 6926e0a4c8..7e7eaba643 100644\n--- a/Documentation/date-formats.txt\n+++ b/Documentation/date-formats.txt\n@@ -20,7 +20,10 @@ RFC 2822::\n ISO 8601::\n \tTime and date specified by the ISO 8601 standard, for example\n \t`2005-04-07T22:13:13`. The parser accepts a space instead of the\n-\t`T` character as well.\n+\t`T` character as well. Fractional parts of a second will be ignored,\n+\tfor example `2005-04-07T22:13:13.019` will be treated as\n+\t`2005-04-07T22:13:13`\n+\n +\n NOTE: In addition, the date part is accepted in the following formats:\n `YYYY.MM.DD`, `MM/DD/YYYY` and `DD.MM.YYYY`.\ndiff --git a/date.c b/date.c\nindex fa39e5e8a5..2c9071d53f 100644\n--- a/date.c\n+++ b/date.c\n@@ -553,6 +553,11 @@ static int set_time(long hour, long minute, long second, struct tm *tm)\n \treturn -1;\n }\n \n+static int is_date_known(struct tm *tm)\n+{\n+\treturn tm->tm_year != -1 && tm->tm_mon != -1 && tm->tm_mday != -1;\n+}\n+\n static int match_multi_number(timestamp_t num, char c, const char *date,\n \t\t\t char *end, struct tm *tm, time_t now)\n {\n@@ -571,6 +576,13 @@ static int match_multi_number(timestamp_t num, char c, const char *date,\n \t\tif (num3 < 0)\n \t\t\tnum3 = 0;\n \t\tif (set_time(num, num2, num3, tm) == 0) {\n+\t\t\t/*\n+\t\t\t * If %H:%M:%S was just parsed followed by: .\n+\t\t\t * Consider (& discard) it as fractional second\n+\t\t\t * if %Y%m%d is parsed before.\n+\t\t\t */\n+\t\t\tif (*end == '.' && isdigit(end[1]) && is_date_known(tm))\n+\t\t\t\tstrtol(end + 1, &end, 10);\n \t\t\tbreak;\n \t\t}\n \t\treturn 0;\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex d9fcc829a9..80917c81c3 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -81,6 +81,8 @@ check_parse 2008-02 bad\n check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -5:' '2008-02-14 20:30:45 +0000'\n@@ -103,6 +105,7 @@ check_approxidate 5.seconds.ago '2009-08-30 19:19:55'\n check_approxidate 10.minutes.ago '2009-08-30 19:10:00'\n check_approxidate yesterday '2009-08-29 19:20:00'\n check_approxidate 3.days.ago '2009-08-27 19:20:00'\n+check_approxidate '12:34:56.3.days.ago' '2009-08-27 12:34:56'\n check_approxidate 3.weeks.ago '2009-08-09 19:20:00'\n check_approxidate 3.months.ago '2009-05-30 19:20:00'\n check_approxidate 2.years.3.months.ago '2007-05-30 19:20:00'\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396138","messageId":"51aa60c06965e86d30619006f9f431e4f8281104.1587740682.git.congdanhqx@gmail.com","threadId":"53221","inReplyTo":"cover.1587740682.git.congdanhqx@gmail.com","subject":"[PATCH v5 4/4] date.c: allow compact version of ISO-8601 datetime","fromName":"Đoàn Trần Công Danh","fromEmail":"congdanhqx@gmail.com","sentAt":"2020-04-24T15:07:32Z","receivedAt":"2020-04-24T15:07:49Z","isPatch":true,"sender":{"key":"congdanhqx@gmail.com","avatar":"https://avatars.githubusercontent.com/u/42673067?v=4"},"body":"Signed-off-by: Đoàn Trần Công Danh \n---\n date.c | 14 ++++++++++++++\n t/t0006-date.sh | 3 +++\n 2 files changed, 17 insertions(+)\n\ndiff --git a/date.c b/date.c\nindex 2c9071d53f..f9ea807b3a 100644\n--- a/date.c\n+++ b/date.c\n@@ -687,6 +687,20 @@ static int match_digit(const char *date, struct tm *tm, int *offset, int *tm_gmt\n \t\tn++;\n \t} while (isdigit(date[n]));\n \n+\t/* 8 digits, compact style of ISO-8601's date: YYYYmmDD */\n+\t/* 6 digits, compact style of ISO-8601's time: HHMMSS */\n+\tif (n == 8 || n == 6) {\n+\t\tunsigned int num1 = num / 10000;\n+\t\tunsigned int num2 = (num % 10000) / 100;\n+\t\tunsigned int num3 = num % 100;\n+\t\tif (n == 8)\n+\t\t\tset_date(num1, num2, num3, NULL, time(NULL), tm);\n+\t\telse if (n == 6 && set_time(num1, num2, num3, tm) == 0 &&\n+\t\t\t *end == '.' && isdigit(end[1]))\n+\t\t\tstrtoul(end + 1, &end, 10);\n+\t\treturn end - date;\n+\t}\n+\n \t/* Four-digit year or a timezone? */\n \tif (n == 4) {\n \t\tif (num <= 1400 && *offset == -1) {\ndiff --git a/t/t0006-date.sh b/t/t0006-date.sh\nindex 80917c81c3..75ee9a96b8 100755\n--- a/t/t0006-date.sh\n+++ b/t/t0006-date.sh\n@@ -82,6 +82,9 @@ check_parse 2008-02-14 bad\n check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'\n check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n check_parse '2008.02.14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'\n+check_parse '20080214T203045-04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045 -04:00' '2008-02-14 20:30:45 -0400'\n+check_parse '20080214T203045.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45.019-04:00' '2008-02-14 20:30:45 -0400'\n check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'\n check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 +0000'\n-- \n2.26.2.384.g435bf60bd5\n\n"},{"id":"396162","messageId":"97fb6f7a-bd6d-6f53-e944-bfc426b81b0d@iee.email","threadId":"53221","inReplyTo":"20200424000707.GB1949@danh.dev","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-04-24T17:30:10Z","receivedAt":"2020-04-24T17:30:13Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Danh,\nOn 24/04/2020 01:07, Danh Doan wrote:\n> On 2020-04-23 21:41:49+0100, Philip Oakley wrote:\n>> On 23/04/2020 20:28, Junio C Hamano wrote:\n>>> Danh Doan writes:\n>> Would is_hhmmss() and is_yyyymmdd() be more obvious abbreviations for\n>> most readers?\n>>\n>> Now that I type them, they do feel that bit too long... , as naming is\n>> hard, maybe stick with the yms and hms, though I do keep wanting to type\n>> ytd for the former..\n> Not sure if I interpret your opinion correctly,\n> Did you mean s/yms/ymd/ and s/ytd/ymd/?\nYes, and then no.\n>\n> Even that, I couldn't grasp the meaning of the last phase?\nTo explain the last part. I don't immediately recognise 'ymd' as a\ntypical abbreviation. A common three letter abbreviation, for me, would\nbe 'ytd' (year to date).\n\nTaken together (my miss-spelling and alternate abbreviation), it\nindicates that I was not tuned-in to the chosen abbreviations.\n--\nPhilip\n\n\n"},{"id":"396163","messageId":"2db49b07-8244-269a-368b-63f6890e31d5@iee.email","threadId":"53221","inReplyTo":"xmqqh7x9emn7.fsf@gitster.c.googlers.com","subject":"Re: [PATCH v3 1/2] date.c: skip fractional second part of ISO-8601","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2020-04-24T17:32:05Z","receivedAt":"2020-04-24T17:32:08Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Junio is correct.\nOn 24/04/2020 01:46, Junio C Hamano wrote:\n> Danh Doan writes:\n>\n>> On 2020-04-23 21:41:49+0100, Philip Oakley wrote:\n>>> On 23/04/2020 20:28, Junio C Hamano wrote:\n>>>> Danh Doan writes:\n>>> Would is_hhmmss() and is_yyyymmdd() be more obvious abbreviations for\n>>> most readers?\n>>>\n>>> Now that I type them, they do feel that bit too long... , as naming is\n>>> hard, maybe stick with the yms and hms, though I do keep wanting to type\n>>> ytd for the former..\n>> Not sure if I interpret your opinion correctly,\n>> Did you mean s/yms/ymd/ and s/ytd/ymd/?\n>>\n>> Even that, I couldn't grasp the meaning of the last phase?\n> Here is how I understood it.\n>\n> Philip thinks, and I admit I have to agree with, that HMS would not\n> be understood as hour-minute-seconds by most people, and YMD would\n> not be as yearh-month-day, either.\n>\n> His \"yms\" in \"stick with the yms and hms\" is a typo of \"ymd\". He is\n> saying that even though YYMMDD and HHMMSS would look a lot more\n> natural, it is too long to type so YMD and HMS may not be so\n> terrible a compromise.\n>\n> With the \"ytd\" in the last one, he is saying that another downside\n> of saying \"ymd\" (other than that it is not how we usually spell\n> year-month-date), even though \"ymd\" might be an acceptable\n> compromise, is that it is too easy to get confused with year-to-date\n> that is commonly abbreviated as \"YTD\".\nTrue.\n--\nPhilip\n"},{"id":"396177","messageId":"xmqq368sd3vu.fsf@gitster.c.googlers.com","threadId":"53221","inReplyTo":"20200424114302.GG1949@danh.dev","subject":"Re: [PATCH v4 2/4] date.c: validate and set time in a helper function","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-24T20:29:09Z","receivedAt":"2020-04-24T20:33:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Danh Doan writes:\n\n> I think single line like:\n>\n> \t/* We accept 61st second for the single? leap second */\n>\n> Or something along the time, is good enough. Not sure if we want the\n> word \"single\" there, though.\n\nI do not particularly want to see the single but without it, the\nsingle-one comment looks perfect.\n\n>> > -\t\tif (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {\n>> > -\t\t\ttm->tm_hour = num;\n>> > -\t\t\ttm->tm_min = num2;\n>> > -\t\t\ttm->tm_sec = num3;\n>> > +\t\tif (set_time(num, num2, num3, tm) == 0)\n>> > \t\t\tbreak;\n>> > -\t\t}\n>> > \t\treturn 0;\n>> \n>> This caller does become easier to follow, I would say. Nicely done.\n>\n> Yes, when I looked around date.c\n>\n> I saw that the only usecase for validate time is for setting it.\n> And the incoming patch also has that usage.\n>\n> I chose to unify those code path to not buy me too much trouble.\n>\n> I'll take that \"Nicely done\" means this unification is OK for this\n> series.\n\nThe OK was meant for this single place that was converted, not any\nother place you'd use in the remainder of the series.\n\nAnd I think it was not such a good idea to use it twice, but I think\nwith the suggested rewrite you took in v5, the other call site is\nalso OK.\n\nThanks.\n"}]}