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

On Wed, 16 Jan 2008, Johannes Schindelin wrote:

Hi,

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

quoted

Johannes Schindelin [off-list ref] writes:

quoted

quoted

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.

At most, I think, if a_one and b_two are identical, we could use the 
"Index:" form, since "diff -ur something something" is weird (how can 
"something" be different from itself?). If they're different, definitely 
use "diff %s %s".

	-Daniel
*This .sig left intentionally blank*