Re: [PATCH/RFC] Do not show "diff --git" metainfo with --no-prefix

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 16, 2008, 01:46 UTC
Message-ID: <7vir1ubl0g.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LSU.1.00.0801160133150.17650@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 27 quoted lines

>> diff --git a/diff.c b/diff.c
>> index b18c140..8126a74 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -1246,30 +1258,46 @@ static void builtin_diff(const char *name_a,
>>  	char *a_one, *b_two;
>>  	const char *set = diff_get_color_opt(o, DIFF_METAINFO);
>>  	const char *reset = diff_get_color_opt(o, DIFF_RESET);
>> +	int is_git_diff = with_standard_prefix(o);
>>  
>>  	a_one = quote_two(o->a_prefix, name_a + (*name_a == '/'));
>>  	b_two = quote_two(o->b_prefix, name_b + (*name_b == '/'));
>>  	lbl[0] = DIFF_FILE_VALID(one) ? a_one : "/dev/null";
>>  	lbl[1] = DIFF_FILE_VALID(two) ? b_two : "/dev/null";
>> -	printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
>> +
>> +	if (!is_git_diff)
>> +		printf("%sIndex: %s%s\n", set, b_two, reset);
>> +	else
>> +		printf("%sdiff --git %s %s%s\n", set, a_one, b_two, reset);
>> +
>
> Hmm.  AFAICT plain diff outputs "diff ...", not "Index: ...".  IMHO doing 
> half of what SVN does, and half what GNU diff does, but not completely 
> what something else does, does not help anybody.
>
> So I'm mildly negative on this hunk.

You misread the intention of the patch.

This whole point of this RFC patch is about not labelling a non-git patch that results from --no-prefix with "diff --git". As I said in my reply to Daniel, I do not like "Index:" myself, and doing printf("diff %s %s\n", a_one, b_two) instead would be perfectly fine by me.

I do not mind keeping the metainformation such as rename/deleted labels in the output of non-git case (iow, dropping all the other hunks that pay attention to is_git_diff in the RFC patch). As long as the patch is not labelled with "diff --git", stricter checks in git-apply will not trigger, and it knows to skip these non-patch lines. Also a plain GNU patch would ignore those metainformation lines, so there is no strong reason to remove them from the output, unless somebody wants to use non patch non git tool that is stricter for no good reason (and I'd agree with you that the solution to such a tool is a postprocessing filter outside of git).