On Wed, 16 Jan 2008, Johannes Schindelin wrote:
> 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.
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*
-
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
