Re: [PATCH] bring description of git diff --cc up to date

Re: [PATCH] bring description of git diff --cc up to date

[Jonathan Nieder]

Junio C Hamano wrote:

> Jonathan Nieder <jrnieder@uchicago.edu> writes:
>
>> +	This flag implies the '-c' option and makes the patch output
>> +	even more compact by omitting uninteresting hunks.  A hunk is
>> +	considered interesting only if either (a) it shows changes from
>> +	all parents or (b) in an Octopus merge, it shows different changes
>> +	from at least three different parents.
>
> I am not sure where that "at lesta three different parents" comes from.
> It might be that what the logic does can be expressed that way, but that
> was not the guiding principle why the code does what it currently does.

Yes, exactly - this is what I meant in saying "my proposed text
does not suggest very strongly what --cc is intended to do".  I
haven't found any wording yet that both makes it clear what --cc
actually does and follows a thought process suggesting why.

Just to make sure I understand, here is what I think --cc does:

  - In a two-parent merge, it is exactly as Linus has been
    describing it.  A hunk is interesting if and only if
    it shows changes from both parents.

  - In a many-parent merge, the criterion is more stringent.
    As in the two-parent merge case, the hunk must show no
    changes from at least one of the parents, meaning that
    one of several alternatives for that portion of code
    was chosen by the code integrator (without mixing and
    matching or adding additional changes); but further, there
    must have been only two alternatives for that portion of
    code to choose between.  If there were three distinct
    alternatives, no matter what the code integrator does, the
    hunk will show up (because that is so rare and deserves
    attention).

Is that correct?

Thanks for the reading matter.  If it stimulates anyone reading
into coming up with a better description, they should please let
me know :)

Re: [PATCH] bring description of git diff --cc up to date

[Junio C Hamano]

Jonathan Nieder <jrnieder@uchicago.edu> writes:

> Just to make sure I understand, here is what I think --cc does:
>
>   - In a two-parent merge, it is exactly as Linus has been
>     ...
>   - In a many-parent merge, the criterion is more stringent.
>     ...
>
> Is that correct?

The logic in the code does not have separate criteria for two-parent and
Octopus cases.  Actually Linus talks about "when you have two versions to
choose from, and if the result matches one of them, then it is not
interesting".  In a two-parent merge, you cannot have three or more
possible versions to choose from by definition, can you?

Re: [PATCH] bring description of git diff --cc up to date

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jonathan Nieder <jrnieder@uchicago.edu> writes:
>
>> Just to make sure I understand, here is what I think --cc does:
>>
>>   - In a two-parent merge, it is exactly as Linus has been
>>     ...
>>   - In a many-parent merge, the criterion is more stringent.
>>     ...
>>
>> Is that correct?
>
> The logic in the code does not have separate criteria for two-parent and
> Octopus cases.  Actually Linus talks about "when you have two versions to
> choose from, and if the result matches one of them, then it is not
> interesting".  In a two-parent merge, you cannot have three or more
> possible versions to choose from by definition, can you?

To put it another way, I think what you wrote is correct, but two-parent
case is just a degenerated case of a more general rule, that is:

    A hunk is not interesting if the person who merged had only two
    choices offered by the parents to pick from, and the merge result
    exactly matched one of the choices.

You can come up with examples that do not match the above criteria; they
are all interesting.

For example, if all the parent of a tripus disagreed, the person had more
than two choices to pick from, so no matter what the resolution is, the
hunk is interesting.

On the other hand, if 4 parents in a dodecapus lack a line that all other
8 parents have (see the first example in [*1*]), then the choice for the
person who merges these 12 parents is either to include or not include
that line.  If the line was included, it is not interesting.  If the line
was deleted (which is different from what happened in *1*), it is not
interesting, either.

One thing to note is "have only two choices to pick from" does not have a
direct connection to two-parent-ness.  In a two-parent merge (di-pus?), by
definition you cannot have more than two choices, but that is not any
different from a Dodecapus that has only two groups of parents.  Most
octopus merges have only two groups of parents like the "merge from hell"
does when we talk about individual paths (otherwise it would be very
painful to resolve so it is not done in practice).

[Reference]

*1* http://article.gmane.org/gmane.comp.version-control.git/15487

Re: [PATCH] bring description of git diff --cc up to date

[Jonathan Nieder]

Junio C Hamano wrote:

> Actually Linus talks about "when you have two versions to
> choose from, and if the result matches one of them, then it is not
> interesting".

That is much better - thanks.  (The description by Linus I was referring
to was from
<http://thread.gmane.org/gmane.comp.version-control.git/89415>:
"So "--cc" only shows output if: the merge itself actually changed
something from _all_ parents" - which is not too misleading since the
two-parent case is the usual one.)

How about this, then?
--- %< ---
Subject: document diff --cc's long-ago-changed semantics

In February 2006 [1], the definition of "interesting hunk" for
git's "compact combined diff" format changed, without any
corresponding change in documentation.  This patch brings the
documentation up to date.

[1] commit bf1c32bdec8223785c779779d0a660a099f69a63
    combine-diff: update --cc "uninteresting hunks" logic
---
 Documentation/git-diff-tree.txt    |   12 +++++++-----
 Documentation/rev-list-options.txt |    9 +++++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 0e45b58..14dc70d 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -92,12 +92,14 @@ include::pretty-options.txt[]
 --cc::
 	This flag changes the way a merge commit patch is displayed,
 	in a similar way to the '-c' option. It implies the '-c'
-	and '-p' options and further compresses the patch output
-	by omitting hunks that show differences from only one
-	parent, or show the same change from all but one parent
-	for an Octopus merge.  When this optimization makes all
+	and '-p' options and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered uninteresting if the person merging had two versions
+	to choose between among all of the parents and the result shows
+	no changes from one of those versions.
+	When this optimization makes all
 	hunks disappear, the commit itself and the commit log
-	message is not shown, just like in any other "empty diff" case.
+	message are not shown, just like in any other "empty diff" case.
 
 --always::
 	Show the commit itself and the commit log message even
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b6f5d87..c61d05d 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -111,10 +111,11 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 
 --cc::
 
-	This flag implies the '-c' options and further compresses the
-	patch output by omitting hunks that show differences from only
-	one parent, or show the same change from all but one parent for
-	an Octopus merge.
+	This flag implies the '-c' option and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered uninteresting if the person merging had two versions
+	to choose between among all of the parents and the result shows
+	no changes from one of those versions.
 
 -r::
 
-- 
1.5.6.3.549.g8ca11

Re: [PATCH] bring description of git diff --cc up to date

[Junio C Hamano]

Jonathan Nieder <jrnieder@uchicago.edu> writes:

> +	This flag implies the '-c' option and makes the patch output
> +	even more compact by omitting uninteresting hunks.  A hunk is
> +	considered uninteresting if the person merging had two versions
> +	to choose between among all of the parents and the result shows

Hmm, I am not a native speaker, but the above makes me confused into
thinking that even if there are 47 parent versions, it is Ok if I looked
at only two versions and picked from one of them -- the description does
not seem to make it clear that it is required that the other 45 agree with
one of the two I looked at and picked from.

    ... if the contents in the parents had only two variants and the merge
    result picked one of them without modification.

would be succinct enough, perhaps?

If we want to further elaborate, we could say something like:

    In a two-parent merge, by definition, there can only be two variants
    to choose from.  Even in a merge with more than two parents, if some
    parents share the same content, and all the other parents share
    another, same content, there are only two variants to choose from.

but I think that is too verbose...

Re: [PATCH] bring description of git diff --cc up to date

[Jonathan Nieder]

On Tue, 22 Jul 2008, Junio C Hamano wrote:

>> +	This flag implies the '-c' option and makes the patch output
>> +	even more compact by omitting uninteresting hunks.  A hunk is
>> +	considered uninteresting if the person merging had two versions
>> +	to choose between among all of the parents and the result shows
>
> the above makes me confused into
> thinking that even if there are 47 parent versions, it is Ok if I looked
> at only two versions and picked from one of them

Here's another attempt.  I grimace at the sound of it, but it might be
more clear.

--- snipsnip ---
Subject: document diff --cc's long-ago-changed semantics

In February 2006 [1], the definition of "interesting hunk" for
git's compact-combined diff format changed without a
corresponding change in documentation.  This patch brings the
documentation up to date.

[1] commit bf1c32bdec8223785c779779d0a660a099f69a63
    combine-diff: update --cc "uninteresting hunks" logic

Signed-off-by: Jonathan Nieder <jrnieder@uchicago.edu>
---
 Documentation/git-diff-tree.txt    |   12 +++++++-----
 Documentation/rev-list-options.txt |    9 +++++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 0e45b58..7f8dc5b 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -92,12 +92,14 @@ include::pretty-options.txt[]
 --cc::
 	This flag changes the way a merge commit patch is displayed,
 	in a similar way to the '-c' option. It implies the '-c'
-	and '-p' options and further compresses the patch output
-	by omitting hunks that show differences from only one
-	parent, or show the same change from all but one parent
-	for an Octopus merge.  When this optimization makes all
+	and '-p' options and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered uninteresting if it shows no changes from at least
+	one of the parents and shows the same changes from each of the
+	parents from which it shows changes.
+	When this optimization makes all
 	hunks disappear, the commit itself and the commit log
-	message is not shown, just like in any other "empty diff" case.
+	message are not shown, just like in any other "empty diff" case.
 
 --always::
 	Show the commit itself and the commit log message even
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b6f5d87..d75de78 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -111,10 +111,11 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 
 --cc::
 
-	This flag implies the '-c' options and further compresses the
-	patch output by omitting hunks that show differences from only
-	one parent, or show the same change from all but one parent for
-	an Octopus merge.
+	This flag implies the '-c' option and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered uninteresting if it shows no changes from at least
+	one of the parents and shows the same changes from each of the
+	parents from which it shows changes.
 
 -r::
 
-- 
1.5.6.3.549.g8ca11

Re: [PATCH] bring description of git diff --cc up to date

[Junio C Hamano]

Jonathan Nieder <jrnieder@uchicago.edu> writes:

> On Tue, 22 Jul 2008, Junio C Hamano wrote:
>
> Here's another attempt.  I grimace at the sound of it, but it might be
> more clear.

Let's just do this.  I think it is clear enough.

 Documentation/git-diff-tree.txt    |   10 +++++-----
 Documentation/rev-list-options.txt |    6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 0e45b58..8c8f35b 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -93,11 +93,11 @@ include::pretty-options.txt[]
 	This flag changes the way a merge commit patch is displayed,
 	in a similar way to the '-c' option. It implies the '-c'
 	and '-p' options and further compresses the patch output
-	by omitting hunks that show differences from only one
-	parent, or show the same change from all but one parent
-	for an Octopus merge.  When this optimization makes all
-	hunks disappear, the commit itself and the commit log
-	message is not shown, just like in any other "empty diff" case.
+	by omitting uninteresting hunks whose the contents in the parents
+	have only two variants and the merge result picks one of them
+	without modification.  When all hunks are uninteresting, the commit
+	itself and the commit log message is not shown, just like in any other
+	"empty diff" case.
 
 --always::
 	Show the commit itself and the commit log message even
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b6f5d87..3aa3809 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -112,9 +112,9 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 --cc::
 
 	This flag implies the '-c' options and further compresses the
-	patch output by omitting hunks that show differences from only
-	one parent, or show the same change from all but one parent for
-	an Octopus merge.
+	patch output by omitting uninteresting hunks whose contents in
+	the parents have only two variants and the merge result picks
+	one of them without modification.
 
 -r::
 

Re: [PATCH] bring description of git diff --cc up to date

[Jonathan Nieder]

Junio C Hamano wrote:

> Let's just do this.  I think it is clear enough.

Works for me.  Thanks, and sorry I dragged this on so.

[PATCH] bring description of git diff --cc up to date

[Jonathan Nieder]

In February 2006 [1], the behavior of diff --cc was changed to
fit a more appropriate notion of "interesting hunks" in Octopus
merges.  This patch updates documentation accordingly.

[1] commit bf1c32bdec8223785c779779d0a660a099f69a63
    combine-diff: update --cc "uninteresting hunks" logic

Signed-off-by: Jonathan Nieder <jrnieder@uchicago.edu>
---
	Hello, all.  A while ago when someone was asking about the diff
	--cc output, I noticed some mistakes in the documentation but
	never got around to fixing them.  So here's a small fix.  After
	this patch, many issues remain:

	 - The user would usually look to git-diff(1) or git-log(1) for
	   an explanation of the --cc option, not git-diff-tree(1) or
	   git-rev-list(1).
	 - diff-format.txt contains the cryptic text "Note that
	   'combined diff' lists only files which were modified from
	   all parents."  I have no idea what that means.
	 - There is some duplication of text between git-diff-tree(1)
	   and git-rev-list(1) here.  I don't think that's such a big
	   deal, though.
	 - My proposed text here is not very intuitive or clear, and it
	   does not suggest very strongly what --cc is intended to do
	   (to point out "evil merges" and other merges where the
	   integrator is likely to have made a mistake).  Alternate
	   wordings welcome!

	At least this patch would not makes matters worse.

 Documentation/git-diff-tree.txt    |   12 +++++++-----
 Documentation/rev-list-options.txt |    9 +++++----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-diff-tree.txt b/Documentation/git-diff-tree.txt
index 0e45b58..5a81d5d 100644
--- a/Documentation/git-diff-tree.txt
+++ b/Documentation/git-diff-tree.txt
@@ -92,12 +92,14 @@ include::pretty-options.txt[]
 --cc::
 	This flag changes the way a merge commit patch is displayed,
 	in a similar way to the '-c' option. It implies the '-c'
-	and '-p' options and further compresses the patch output
-	by omitting hunks that show differences from only one
-	parent, or show the same change from all but one parent
-	for an Octopus merge.  When this optimization makes all
+	and '-p' options and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered interesting only if either (a) it shows changes from
+	all parents or (b) in an Octopus merge, it shows different changes
+	from at least three different parents.
+	When this optimization makes all
 	hunks disappear, the commit itself and the commit log
-	message is not shown, just like in any other "empty diff" case.
+	message are not shown, just like in any other "empty diff" case.
 
 --always::
 	Show the commit itself and the commit log message even
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index b6f5d87..a399e2b 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -111,10 +111,11 @@ options may be given. See linkgit:git-diff-files[1] for more options.
 
 --cc::
 
-	This flag implies the '-c' options and further compresses the
-	patch output by omitting hunks that show differences from only
-	one parent, or show the same change from all but one parent for
-	an Octopus merge.
+	This flag implies the '-c' option and makes the patch output
+	even more compact by omitting uninteresting hunks.  A hunk is
+	considered interesting only if either (a) it shows changes from
+	all parents or (b) in an Octopus merge, it shows different changes
+	from at least three different parents.
 
 -r::
 
-- 
1.5.6.3.549.g8ca11

Re: [PATCH] bring description of git diff --cc up to date

[Junio C Hamano]

Jonathan Nieder <jrnieder@uchicago.edu> writes:

> +	This flag implies the '-c' option and makes the patch output
> +	even more compact by omitting uninteresting hunks.  A hunk is
> +	considered interesting only if either (a) it shows changes from
> +	all parents or (b) in an Octopus merge, it shows different changes
> +	from at least three different parents.

I am not sure where that "at lesta three different parents" comes from.
It might be that what the logic does can be expressed that way, but that
was not the guiding principle why the code does what it currently does.

These two might make it clearer what hunks are considered interesting:

    http://thread.gmane.org/gmane.comp.version-control.git/15098/focus=15109
    http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15491

Especially the latter thread, not just the focused one but the whole
thing, is quite amusing.

This one gives an insight into the logical consequence of the hunk
selection criteria:

    http://thread.gmane.org/gmane.comp.version-control.git/15486/focus=15600

it is worth a read after understanding why some hunks are omitted and some
hunks are included by reading other articles in the thread.