Hi,

On Tue, 15 Jan 2008, Junio C Hamano wrote:

quoted text
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> 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.

Well, I commented on this hunk specifically, and think that the intention 
of the patch would be better served by just conditionally omitting 
"--git", and nothing else.

quoted text
> 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).

Fair enough.

Ciao,
Dscho

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html