Re: [PATCH v2] diff --shortstat --dirstat: remove duplicate output

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Mar 1, 2015, 14:23 UTC
Message-ID: <54F320E9.1000006@drmicha.warpmail.net>
In-Reply-To: <54F2E931.7020200@web.de>

Torsten Bögershausen venit, vidit, dixit 01.03.2015 11:25:

Show 20 quoted lines

> On 2015-03-01 08.39, Mårten Kongstad wrote:
> []
>  index ed7e093..128f7bf 100755
>> --- a/t/t4047-diff-dirstat.sh
>> +++ b/t/t4047-diff-dirstat.sh
>> @@ -973,4 +973,15 @@ test_expect_success 'diff.dirstat=future_param,0,lines should warn, but still wo
>>  	test_i18ngrep -q "diff\\.dirstat" actual_error
>>  '
>>  
>> +test_expect_success '--shortstat --dirstat should output only one dirstat' '
>> +	git diff --shortstat --dirstat=changes HEAD^..HEAD >actual_diff_shortstat_dirstat_changes &&
>> +	test $(grep -c " dst/copy/changed/$" actual_diff_shortstat_dirstat_changes) = 1 &&
> How portable is the "grep -c" usage ?
> (I don't now it either, do we have other opinions ?), but the following seems to be more "Git-style":
> 
> test_expect_success '--shortstat --dirstat should output only one dirstat' '
> 	git diff --shortstat --dirstat=changes HEAD^..HEAD >actual_diff_shortstat_dirstat_changes &&
> 	grep " dst/copy/changed/$" actual_diff_shortstat_dirstat_changes >actual &&
> 	test_line_count = 1 actual
>

If I would have had to guess from the documentation: What does "git diff --dirstat --shortstat" do? I would have answered: It displays both the dirstat and the shortstat.

So, is what you are trying to "fix" a peculiarity of "--dirstat=changes", or do you simplify prefer --dirstat and --shortstat to override each other?

Maybe I'm overlooking something (and that's not a rhetorical conditional), but if you specify both options when you want the output of only one them, the answer would be the obvious one, not a patch, wouldn't it?

If there is indeed a good reason to change the behavior it should be documented.

Michael