Re: [PATCH] blame: Allow to blame paths freshly added to the index

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 15, 2016, 10:45 UTC
Message-ID: <alpine.DEB.2.20.1607151242020.6426@virtualbox>
In-Reply-To: <20160715024254.29186-1-mh@glandium.org>

Hi Mike,

On Fri, 15 Jul 2016, Mike Hommey wrote:

Show 12 quoted lines

> When blaming files, changes in the work tree are taken into account
> and displayed as being "Not Committed Yet".
> 
> However, when blaming a file that is not known to the current HEAD,
> git blame fails with `no such path 'foo' in HEAD`, even when the file
> was git add'ed.
> 
> This would seem uninteresting with the plain `git blame` case, which
> it is, but it becomes useful when using copy detection, and the new file
> was created from pieces already in HEAD, moved or copied from other
> files.
> ---

Well explained.

Please add your sign-off.

Show 18 quoted lines

>  static struct commit_list **append_parent(struct commit_list **tail, const unsigned char *sha1)
> diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
> index a9b266f..a0a09e2 100755
> --- a/t/t8003-blame-corner-cases.sh
> +++ b/t/t8003-blame-corner-cases.sh
> @@ -137,6 +137,29 @@ test_expect_success 'blame wholesale copy and more' '
>  
>  '
>  
> +test_expect_success 'blame wholesale copy and more in the index' '
> +
> +	{
> +		echo ABC
> +		echo DEF
> +		echo XXXX
> +		echo YYYY
> +		echo GHIJK
> +	} >horse &&

A more common way to do this in our test scripts is by using here documents. However, in this case I would suggest

	test_write_lines ABC DEF XXXX YYYY GHIJK >horse

instead. The equivalent applies to the 'expected' file below:

Show 11 quoted lines

> +	git add horse &&
> +	git blame -f -C -C1 -- horse | sed -e "$pick_fc" >current &&
> +	{
> +		echo mouse-Initial
> +		echo mouse-Second
> +		echo cow-Fifth
> +		echo horse-Not
> +		echo mouse-Third
> +	} >expected &&
> +	test_cmp expected current &&
> +	git rm -f horse

Should this not be a

	test_when_finished "git rm -f horse"

at the beginning?

Otherwise it looks really good to me.

Ciao, Dscho