From: Haitao Li <lihaitao@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, Jeff King <peff@peff.net>
Subject: Re: [PATCH v3] date.c: Support iso8601 timezone formats
Date: Sat, 10 Sep 2011 16:06:50 +0800 [thread overview]
Message-ID: <CABk5KLgyk_MRd6DLkdaK2s_5pxLQNipKmLJtCjNO2TaLLEr6UA@mail.gmail.com> (raw)
In-Reply-To: <7vhb4lvflb.fsf@alter.siamese.dyndns.org>
On Sat, Sep 10, 2011 at 12:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Haitao Li <lihaitao@gmail.com> writes:
>
>> Timezone designators including additional separator (`:`) are ignored.
>> Actually zone designators in below formats are all valid according to
>> ISO8601:2004, section 4.3.2:
>> [+-]hh, [+-]hhmm, [+-]hh:mm
>
> Thanks for a re-roll.
>
>> This patch teaches git recognizing zone designators with hours and
>> minutes separated by colon, or minutes are empty.
>
> The last sentence above makes it sound as if you are accepting
>
> "2011-09-17 12:34:56 +09:"
>
> but I suspect that is not what you intend to allow. Perhaps "we allowed
> hh and hhmm and this teaches Git to recognize hh:mm format as well"?
>
Yes, this is a better one. Sorry for my English, and thanks for the suggestion.
>> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
>> index f87abb5..5235b7a 100755
>> --- a/t/t0006-date.sh
>> +++ b/t/t0006-date.sh
>> @@ -40,6 +40,11 @@ check_parse 2008-02 bad
>> check_parse 2008-02-14 bad
>> check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 +0000'
>> check_parse '2008-02-14 20:30:45 -0500' '2008-02-14 20:30:45 -0500'
>> +check_parse '2008-02-14 20:30:45 -0015' '2008-02-14 20:30:45 -0015'
>> +check_parse '2008-02-14 20:30:45 -5' '2008-02-14 20:30:45 -0500'
>> +check_parse '2008-02-14 20:30:45 -05' '2008-02-14 20:30:45 -0500'
>> +check_parse '2008-02-14 20:30:45 -:30' '2008-02-14 20:30:45 +0000'
>> +check_parse '2008-02-14 20:30:45 -05:00' '2008-02-14 20:30:45 -0500'
>> check_parse '2008-02-14 20:30:45' '2008-02-14 20:30:45 -0500' EST5
>
> The above are from Peff, no? We should credit him for tests in the
> proposed log message.
Yes, we should credit Peff. Sorry for not knowing log message is used for this.
>
> Because the three formats 8601 specifies are "hh", "hhmm", or "hh:mm"
> after +/-, among the above new tests, it appears to me that zone
> designators "-5" and "-:30" should yield "bad", instead of being accepted.
Yes, the spec clearly states 2 digits are mandatory. "-5" should be
regarded as invalid here.
The above test *ignores* ":30" by setting offset to "+0000", this is
to conform to how it works previously, less than 3 digits in offset
are ignored. I agree it's better to *reject* them instead.
> The same for "+09:" I mentioned above, which is not in the new test.
>
Will add.
Thanks again for the suggestions!
prev parent reply other threads:[~2011-09-10 8:07 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-06 14:56 [PATCH] date.c: Parse timezone with colon as separator Haitao Li
2011-09-06 20:24 ` Jeff King
2011-09-07 3:00 ` Haitao Li
2011-09-07 5:46 ` [PATCH v2] date.c: Support iso8601 timezone formats Haitao Li
2011-09-07 17:00 ` Junio C Hamano
2011-09-08 2:53 ` Haitao Li
2011-09-09 10:10 ` [PATCH v3] " Haitao Li
2011-09-09 16:35 ` Junio C Hamano
2011-09-09 17:04 ` Junio C Hamano
2011-09-09 20:46 ` Junio C Hamano
2011-09-10 8:29 ` Haitao Li
2011-09-10 8:06 ` Haitao Li [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABk5KLgyk_MRd6DLkdaK2s_5pxLQNipKmLJtCjNO2TaLLEr6UA@mail.gmail.com \
--to=lihaitao@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).