From: Junio C Hamano <gitster@pobox•com>
To: Michael J Gruber <git@drmicha•warpmail.net>
Cc: git@vger•kernel.org, Jeff King <peff@peff•net>
Subject: Re: [PATCH] combine-diff: use textconv for combined diff format
Date: Sat, 16 Apr 2011 10:19:48 -0700 [thread overview]
Message-ID: <7vei52azbf.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <4DA96E48.3050008@drmicha.warpmail.net> (Michael J. Gruber's message of "Sat, 16 Apr 2011 12:24:08 +0200")
Michael J Gruber <git@drmicha•warpmail.net> writes:
> Junio C Hamano venit, vidit, dixit 15.04.2011 20:34:
>> Michael J Gruber <git@drmicha•warpmail.net> writes:
>>
>>> git diff -m produces a combined diff!
>>
>> Hmm, what is the rest of your command line? I thought -m was a way to ask
>> pairwise diff with each parent.
>
> Sure, but it does not always work like that. Just look at the test from
> my patch, or do any "git merge --no-commit" and then "git diff -m". I
> would expect that to compare the worktree to each parent, but in fact it
> runs "diff --cc".
Thanks; it wasn't clear you are comparing stages with the working tree.
Asking for the rest of the command line paid off ;-)
And yes, comparing multiple entries with the worktree files is done in
run_diff_files() defined in diff-lib.c; it is unaware of the -m option
that was originally defined for diff-tree to show pairwise diff. That
codepath never cared about -m before nor after -c/--cc was invented for
diff-tree, and it only learned about -c/--cc when it was introduced. I
think diff-index is unaware of the -m option from the same historical
background (read: not "for the same reason or justification").
We may want to change that, but I am personally not very interested. We
can ask to diff against a specific stage, I know that is what I do, and I
think that is what most people do [*1*].
> "diff -m --oneline" says something like
>
> aa01ae1 (from 64c0923) Merge branch 'master' into somebranch
> diff --git a/a b/a
> index 72594ed..d8323da 100644
> Binary files a/a and b/a differ
> aa01ae1 (from e85049e) Merge branch 'master' into somebranch
> diff --git a/a b/a
> index 86e041d..d8323da 100644
> Binary files a/a and b/a differ
Yes this is the case for diff-tree running pair-wise comparison.
> so I'm wondering whether we shouldn't stay closer to that with "--cc
> also", e.g.:
>
> aa01ae1 Merge branch 'master' into somebranch
> diff --cc a
> index 72594ed,86e041d..d8323da
> Binary files a/a and b/a differ
The -c/--cc options are about presenting the pairwise -m output in a
different way by combining and condensing. So in that sense, if we really
want to combine and condense information, one possibility is to do:
Binary files a/a and c/a differ, b/a and c/a differ.
naming each parent as 'a', 'b', ... and giving the highest letter (in the
two-parent merge case, 'c') to the final result. By doing so, you can
express where in its tree each parent had the content when you are viewing
a renaming merge.
Bu that opens an old can of worms we should have opened and closed four
years ago.
The header shows "diff --cc a" followed by "--- a/a" followed by "+++ b/a"
before the hunk for a two-way merge. But if we are to "combine and
condense", another possibility is to show:
diff --cc a/a b/a c/a
index bf7c788,fa9d23a,5d24d9f..cc69134
--- a/a
--- b/a
+++ c/a
@@@@ -74,26 -74,6 -74,29 +74,50 @@@@
...
to keep the paths information. I do not think anybody cared so far, and
perhaps we should have done it when we introduced -c/--cc, but it is not
at all worth changing now.
That means that we are not all that worried about losing the rename
information when showing such a diff in --cc/-c form. After all, the
"diff --cc a" header is not "diff --cc a/a b/a c/a" that mentions all
paths, and "--- a/a" lines are not repeated for each parent. So while
showing the names like you suggested may be a possibility, I think an
approach that is more in line with the current output would be:
"Binary files a in different versions differ"
or something without naming them with a/, b/, ...
In short, it all depends on how much we condense when running -c/--cc. We
are inconsistent by showing both "--- a/a" and "+++ b/a" lines, but modulo
that we condense away the renamed path information in our current output.
And the final alternative in the previous paragraph would be more in line
with that design.
[Footnote]
*1* I also often use
diff HEAD...MERGE_HEAD $path
diff HEAD $path
diff MERGE_HEAD $path
during a conflicted merge when it is hard to read in the --cc form.
next prev parent reply other threads:[~2011-04-16 17:20 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-11 17:12 textconv not invoked when viewing merge commit Peter Oberndorfer
2011-04-12 9:04 ` Michael J Gruber
2011-04-14 19:09 ` Jeff King
2011-04-14 19:15 ` Jeff King
2011-04-14 19:26 ` Junio C Hamano
2011-04-14 19:28 ` Jeff King
2011-04-14 19:35 ` Michael J Gruber
2011-04-14 19:43 ` Junio C Hamano
2011-04-14 20:06 ` Junio C Hamano
2011-04-14 20:23 ` Jeff King
2011-04-14 21:05 ` Junio C Hamano
2011-04-14 21:30 ` Jeff King
2011-04-15 15:29 ` [PATCH] combine-diff: use textconv for combined diff format Michael J Gruber
2011-04-15 18:34 ` Junio C Hamano
2011-04-16 10:24 ` Michael J Gruber
2011-04-16 17:19 ` Junio C Hamano [this message]
2011-04-16 21:37 ` Jakub Narebski
2011-04-15 23:56 ` Jeff King
2011-04-21 16:08 ` Peter Oberndorfer
2011-04-15 6:54 ` textconv not invoked when viewing merge commit Matthieu Moy
2011-04-15 20:45 ` Junio C Hamano
2011-04-16 1:47 ` Jeff King
2011-04-16 6:10 ` Junio C Hamano
2011-04-16 6:33 ` Jeff King
2011-04-16 16:23 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vei52azbf.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox$(echo .)com \
--cc=git@drmicha$(echo .)warpmail.net \
--cc=git@vger$(echo .)kernel.org \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox