Hi,
On Tue, 15 Jan 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> 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.
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
