Thread (3 messages) flat view 3 messages, 3 authors, 2016-06-15

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:44:05

Hi,

On Tue, 15 Jan 2008, Junio C Hamano wrote:
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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help