Re: [PATCH v2 3/3] t/t8006: test textconv support for blame

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 11, 2010, 23:52 UTC
Message-ID: <7vd3vxgm9x.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1275924218-20154-3-git-send-email-axel.bonnet@ensimag.imag.fr>

Axel Bonnet <axel.bonnet@ensimag.imag.fr> writes:

> Test the correct functionning of textconv with blame <file> and blame HEAD^ <file>.
> Test the case when no driver is specified.

Good to see tests for both positive and negative cases. Too many people forget the latter.

> +find_blame() {
> +	sed -e 's/^.*(/(/g'
> +}

Two issues:

 - No need for "g" as your pattern is anchored at the left;

 - As ".*" is greedy, you will eat a lot more than what you expect when
   the line in the blamed contents happen to have '(' on it.

I'd rewrite it as:

    sed -e 's/^[^(]*//'

Will queue all three patches, with this fix and a style fix for 2/3; no need to resend.

Thanks.