Re: [PATCH/RFC] Do not show "diff --git" metainfo with --no-prefix
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jan 16, 2008, 01:53 UTC
- Message-ID
- <alpine.LSU.1.00.0801160152540.17650@racer.site>
- In-Reply-To
- <7vir1ubl0g.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 15 Jan 2008, Junio C Hamano wrote:
Show 36 quoted lines
> 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.
Show 10 quoted lines
> 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