Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx> writes:
> git svn log --show-commit had no tests and, consequently, no attention
> by the author of
>
> b1b4755 (git-log: put space after commit mark, 2011-03-10)
>
> who kept git svn log working only without --show-commit.
>
> Introduce a test and fix it.
>
> Reported-by: Bernt Hansen <bernt@xxxxxxxxx>
> Signed-off-by: Michael J Gruber <git@xxxxxxxxxxxxxxxxxxxx>
> ---
> git svn scares me.
Sorry about this breakage.
> diff --git a/git-svn.perl b/git-svn.perl
> index a5857c1..0cee0e9 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -5735,7 +5735,7 @@ sub cmd_show_log {
> my $esc_color = qr/(?:\033\[(?:(?:\d+;)*\d*)?m)*/;
> while (<$log>) {
> if (/^${esc_color}commit (- )?($::sha1_short)/o) {
> - my $cmt = $1;
> + my $cmt = $2;
Even more defensive approach would be not to grab the grouping by doing:
- if (/^${esc_color}commit (- )?($::sha1_short)/o) {
+ if (/^${esc_color}commit (?:- )?($::sha1_short)/o) {
and not to change anything else. I should have noticed the $1 reference
that was immediately on the next line when I saw and applied your patch,
but if there were more references in the scope that is outside of the
patch context, the same bug would be likely to have gone unnoticed.
I do not have enough bandwidth to read every single line of the patch from
everybody, so small bugs in patches from known to be good people (you
included) can slip through, unless marked with "I am not familiar with
this codepath" or "I am not strong in Perl regexp" or somesuch, in which
case I try to allocate more time to give it another pass of eyeballing.
In any case, thanks for the fix. I think being defensive with (?:) would
be a better idea, so I'll tweak the patch before applying with your test.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html