git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Rohit Ashiwal <rohit.ashiwal265@gmail.com>,
	Johannes.Schindelin@gmx.de, git@vger.kernel.org,
	martin.agren@gmail.com, newren@gmail.com, t.gummerer@gmail.com
Subject: Re: [GSoC][PATCH v2 3/6] rebase -i: support --committer-date-is-author-date
Date: Wed, 14 Aug 2019 19:38:45 +0100
Message-ID: <0a0a6fd3-933e-c0a4-03c2-8c1c69e5ffca@gmail.com> (raw)
In-Reply-To: <xmqqsgq5805u.fsf@gitster-ct.c.googlers.com>

On 13/08/2019 18:06, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
>  [...]
>>> +
>>> +		strbuf_addf(&date, "@%s",ident.date_begin);
>>
>> I think we should use %s.* and ident.date_end to be sure we getting
>> what we want. Your version is OK if the author is formatted correctly
>> but I'm uneasy about relying on that when we can get the verified end
>> from ident.
> 
> If the author line is not formatted correctly, split_ident_line()
> would notice and return NULL in these fields, I think (in other
> words, my take on the call to split_ident_line() above is not
> necessarily done in order to "split", but primarily to validate that
> the line is formatted correctly---and find the beginning of the
> timestamp field).

I just had a read through split_ident_line() and it looks to me like it 
will ignore any junk after the timezone. So long as it sees '+' or '-' 
followed by at least one digit it will use that for the time zone and 
return success regardless of what follows it so I think we want to pay 
attention to the end data it returns for the date and timezone.

> But your "pay attention to date_end" raises an interesting point.
> 
> The string that follows ident.date_begin would be a large integer
> (i.e. number of seconds since epoch), a SP, a positive or negative
> sign (i.e. east or west of GMT), 4 digits (i.e. timezone offset), so
> if you want to leave something like "@1544328981" in the buffer, you
> need to stop at .date_end to omit the timezone information.
> 
> On the other hand, if you do want the timezone information as well
> (which I think is the case for this codepath), you should not stop
> at there and have something like "@1544328981 +0900", the code as
> written would give better result.

Good point, I had forgotten that split_ident_line() returned separate 
fields for the date and timezone. I agree that we want the timezone here 
too. I  I think it would be a good idea to beef up the tests to use a 
non default timezone to check that we are actually setting it correctly.

Best Wishes

Phillip


  reply index

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-06 17:36 [GSoC][PATCHl 0/6] rebase -i: support more options Rohit Ashiwal
2019-08-06 17:36 ` [GSoC][PATCHl 1/6] rebase -i: add --ignore-whitespace flag Rohit Ashiwal
2019-08-07  5:38   ` Junio C Hamano
2019-08-07 20:25     ` Rohit Ashiwal
2019-08-08 16:44   ` Phillip Wood
2019-08-12 17:43     ` Rohit Ashiwal
2019-08-06 17:36 ` [GSoC][PATCHl 2/6] sequencer: add NULL checks under read_author_script Rohit Ashiwal
2019-08-06 17:36 ` [GSoC][PATCHl 3/6] rebase -i: support --committer-date-is-author-date Rohit Ashiwal
2019-08-08 11:29   ` Phillip Wood
2019-08-08 16:00     ` Junio C Hamano
2019-08-06 17:36 ` [GSoC][PATCHl 4/6] sequencer: rename amend_author to author_to_rename Rohit Ashiwal
2019-08-08 11:30   ` Phillip Wood
2019-08-06 17:36 ` [GSoC][PATCHl 5/6] rebase -i: support --ignore-date Rohit Ashiwal
2019-08-07 19:41   ` Johannes Schindelin
2019-08-07 20:22     ` Junio C Hamano
2019-08-07 20:33       ` Rohit Ashiwal
2019-08-08 11:42   ` Phillip Wood
2019-08-08 16:53     ` Phillip Wood
2019-08-06 17:36 ` [GSoC][PATCHl 6/6] rebase: add --author-date-is-committer-date Rohit Ashiwal
2019-08-08 11:42   ` Phillip Wood
2019-08-12 19:42 ` [GSoC][PATCH v2 0/6] rebase -i: support more options Rohit Ashiwal
2019-08-12 19:42   ` [GSoC][PATCH v2 1/6] rebase -i: add --ignore-whitespace flag Rohit Ashiwal
2019-08-13 12:07     ` Phillip Wood
2019-08-12 19:42   ` [GSoC][PATCH v2 2/6] sequencer: add NULL checks under read_author_script Rohit Ashiwal
2019-08-12 19:42   ` [GSoC][PATCH v2 3/6] rebase -i: support --committer-date-is-author-date Rohit Ashiwal
2019-08-13 10:38     ` Phillip Wood
2019-08-13 12:09       ` Phillip Wood
2019-08-13 17:06       ` Junio C Hamano
2019-08-14 18:38         ` Phillip Wood [this message]
2019-08-13 13:35     ` Phillip Wood
2019-08-12 19:42   ` [GSoC][PATCH v2 4/6] sequencer: rename amend_author to author_to_rename Rohit Ashiwal
2019-08-13 13:29     ` Phillip Wood
2019-08-12 19:42   ` [GSoC][PATCH v2 5/6] rebase -i: support --ignore-date Rohit Ashiwal
2019-08-13 13:28     ` Phillip Wood
2019-08-13 17:21       ` Junio C Hamano
2019-08-14 18:47         ` Phillip Wood
2019-08-13 21:45     ` Junio C Hamano
2019-08-14 18:51       ` Phillip Wood
2019-08-14 19:33         ` Junio C Hamano
2019-08-17  9:28           ` Phillip Wood
2019-08-12 19:43   ` [GSoC][PATCH v2 6/6] rebase: add --author-date-is-committer-date Rohit Ashiwal
2019-08-13 17:28     ` Junio C Hamano
2019-08-20  3:45 ` [PATCH v3 0/6] rebase -i: support more options Rohit Ashiwal
2019-08-20  3:45   ` [PATCH v3 1/6] rebase -i: add --ignore-whitespace flag Rohit Ashiwal
2019-08-20 18:40     ` Phillip Wood
2019-08-20 18:47       ` Rohit Ashiwal
2019-08-20  3:45   ` [PATCH v3 2/6] sequencer: add NULL checks under read_author_script Rohit Ashiwal
2019-08-23 15:20     ` Junio C Hamano
2019-08-20  3:45   ` [PATCH v3 3/6] rebase -i: support --committer-date-is-author-date Rohit Ashiwal
2019-08-20 13:32     ` Phillip Wood
2019-08-20  3:45   ` [PATCH v3 4/6] sequencer: rename amend_author to author_to_rename Rohit Ashiwal
2019-08-20  3:45   ` [PATCH v3 5/6] rebase -i: support --ignore-date Rohit Ashiwal
2019-08-20 13:45     ` Phillip Wood
2019-08-20 17:42     ` Junio C Hamano
2019-08-20 18:30       ` Phillip Wood
2019-08-20  3:45   ` [GSoC][PATCH v2 6/6] rebase: add --author-date-is-committer-date Rohit Ashiwal
2019-08-20  4:00     ` Rohit Ashiwal
2019-08-20  3:45   ` [PATCH v3 6/6] rebase: add --reset-author-date Rohit Ashiwal
2019-08-20  3:54   ` [PATCH v3 0/6] rebase -i: support more options Rohit Ashiwal
2019-08-20 13:56   ` Phillip Wood
2019-08-20 17:53     ` Junio C Hamano
2019-08-20 18:37       ` Phillip Wood
2019-09-07 11:50 ` [PATCH v4 " Rohit Ashiwal
2019-09-07 11:50   ` [PATCH v4 1/6] rebase -i: add --ignore-whitespace flag Rohit Ashiwal
2019-10-04  9:29     ` Phillip Wood
2019-10-05 18:12       ` Elijah Newren
2019-10-06 17:57       ` Rohit Ashiwal
2019-09-07 11:50   ` [PATCH v4 2/6] sequencer: allow callers of read_author_script() to ignore fields Rohit Ashiwal
2019-09-07 11:50   ` [PATCH v4 3/6] rebase -i: support --committer-date-is-author-date Rohit Ashiwal
2019-10-04  9:37     ` Phillip Wood
2019-10-06 17:57       ` Rohit Ashiwal
2019-10-24 13:28     ` Phillip Wood
2019-09-07 11:50   ` [PATCH v4 4/6] sequencer: rename amend_author to author_to_rename Rohit Ashiwal
2019-09-07 11:50   ` [PATCH v4 5/6] rebase -i: support --ignore-date Rohit Ashiwal
2019-09-07 20:56     ` Rohit Ashiwal
2019-09-27 10:00     ` Phillip Wood
2019-10-06 17:57       ` Rohit Ashiwal
2019-10-24 20:36         ` Phillip Wood
2019-09-07 11:50   ` [PATCH v4 6/6] rebase: add --reset-author-date Rohit Ashiwal
2019-09-09 18:02   ` [PATCH v4 0/6] rebase -i: support more options Junio C Hamano
2019-09-09 18:51     ` Phillip Wood
2019-09-09 19:24       ` Junio C Hamano
2019-11-01 13:59 ` [PATCH v5 " Rohit Ashiwal
2019-11-01 13:59   ` [PATCH v5 1/6] rebase -i: add --ignore-whitespace flag Rohit Ashiwal
2019-11-01 13:59   ` [PATCH v5 2/6] sequencer: allow callers of read_author_script() to ignore fields Rohit Ashiwal
2019-11-01 14:00   ` [PATCH v5 3/6] rebase -i: support --committer-date-is-author-date Rohit Ashiwal
2019-11-01 14:00   ` [PATCH v5 4/6] sequencer: rename amend_author to author_to_rename Rohit Ashiwal
2019-11-01 14:00   ` [PATCH v5 5/6] rebase -i: support --ignore-date Rohit Ashiwal
2019-11-02  7:32     ` Junio C Hamano
2019-11-02  7:48       ` Junio C Hamano
2019-11-01 14:00   ` [PATCH v5 6/6] rebase: add --reset-author-date Rohit Ashiwal
2019-11-02  7:34     ` Junio C Hamano
2019-11-21  6:14   ` [PATCH v5 0/6] rebase -i: support more options Junio C Hamano
2019-11-21  8:17     ` Alban Gruin
2019-11-22  6:32       ` Junio C Hamano
2019-11-28 11:14   ` Phillip Wood

Reply instructions:

You may reply publically 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0a0a6fd3-933e-c0a4-03c2-8c1c69e5ffca@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=newren@gmail.com \
    --cc=rohit.ashiwal265@gmail.com \
    --cc=t.gummerer@gmail.com \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git