patch, 2 partsdiff-merges: introduce '-d' option

This new convenience option requests full diff with respect to first parent, so that

  git log -d

will output diff with respect to first parent for every commit, universally, no matter how many parents the commit turns out to have.

It's implemented as pure synonym for

  --diff-merges=first-parent --patch

The first commit in the series tweaks diff-merges documentation a bit, and is valuable by itself. It's put here as '-d' implementation commit depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.

Sergey Organov (2):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '-d' option

 Documentation/diff-options.txt | 101 +++++++++++++++++++--------------
 Documentation/git-log.txt      |   4 +-
 diff-merges.c                  |   3 +
 t/t4013-diff-various.sh        |   8 +++
 4 files changed, 71 insertions(+), 45 deletions(-)

This option provides a shortcut to request diff with respect to first parent for any kind of commit, universally. It's implemented as pure synonym for "--diff-merges=first-parent --patch".

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 4 ++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 16 insertions(+), 1 deletion(-)

Sergey Organov <sorganov@gmail.com> writes:

Sounds very straight-forward.

Given that "--first-parent" in "git log --first-parent -p" already defeats "-m" and shows the diff against the first parent only, people may find it confusing if "git log -d" does not act as a shorthand for that. From the above and also from the documentation update, it is hard to tell if that is what you implemented, or it only affects the "diff-merges" part.

Other than that, the patch looks quite small and to the point.

Thanks.

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

It doesn't, and I believe it's a good thing, as primary function of --first-parent is to change history traversal rules, and if -d did that, it would be extremely confusing.

Also, --first-parent is correctly documented as implying --diff-merges=first-parent, not as defeating -m.

If we read resulting documentation with a fresh eye, -d is similar to --cc, and -c, just producing yet another kind of output, so I think all this fits together quite nicely and shouldn't cause confusion.

Thanks, -- Sergey Organov

Sergey Organov <sorganov@gmail.com> writes:

I am not sure about that.

Yes, exactly. That makes me even more convinced that the intuitive behaviour, when we say "we have this great short-hand option that lets your 'git log' to do the first-parent thing with patch output", is to do the first-parent traversal _and_ show first-parent patches.

"-d" is documented as a short-hand for "--diff-merges=first-parent --patch" and not for "--first-parent --patch", so the behaviour may correctly match documentation, but that does not make the documented behaviour an intuitive one. And a behaviour that is not intuitive is confusing.

Another thing is that showing first-parent patch for merges while letting the traversal also visit the second-parent chain is not as useful an option as it could be, even though it is not so bad as the original "-m -p" that also showed second-parent patch for merges as well. People would have to say "log --first-parent -p" to get the first-parent traversal with first-parent patch output, and they would not behefit from having "-d".

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

I think both behaviors make sense, provided they are correctly documented. I just prefer the one that is more basic, yet allows to achieve things that another one does not.

I don't see why desire to look at diff-to-first-parent on "side" branches is any different from desire to look at them on "primary" branch, sorry, so I still don't want "-d" to affect traversal or other commit filtering rules. We do have --first-parent as well as a few others for that.

Well, at least they can now say "log --first-parent -d" as well ;)

Honestly, the "log --first-parent -p" (without "-m") suddenly producing diffs for merge commits is already unnatural, needs yet another special-casing in documentation, and then, finally, this relatively new behavior was introduced exactly because there were no "-d" at that time, to save typing "-m". The latter is yet another example of why "-d" in its current form is a good idea.

That said, if you feel like there is place for a short-cut for this particular use-case, it'd be fine with me, say:

--fpd:
  short-cut for "--first-parent -d"

would fit quite nicely into the picture, I think.

Thanks, -- Sergey Organov

Sergey Organov <sorganov@gmail.com> writes:

Yeah, but that is not what I meant. The above argues for why "--diff-merges=first-parent" should exist independently from the "--first-parent" traversal *and* display option. I am not saying it should not exist.

But I view that the desire to look at any commits and its changes on the "side" branch at all *is* at odds with the wish to look at first-parent change for merge commits. Once you decide to look at first-parent change for a merge commit, then every change you see for each commit on the "side" branch, whether it is shown as first-parent diff or N pairwise diffs, is what you have already seen in the change in the merge commit, because "git log" goes newer to older, and the commits on the side branches appear after the merge that brings them to the mainline.

Making "log -d" mean "log --diff-merges=first-parent --patch" lets that less useful combination ("show first-parent patches but traverse side branches as well") squat on the short and sweet "-d" that could be used for more useful "log --first-parent --patch", which would also be more common and intuitive to users, and that is what I suspect will become problematic in the longer run.

Thanks.

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

I was not assuming you were saying this, as it has been discussed and agreed upon when --diff-merges=first-parent was introduced, though I think I now see your point more clearly.

I think I do now understand what you mean, yet I have alternative view on the issue.

Actually, this happens to be exactly one of intended use-cases for "-d". It's useful to see how some change introduced by the merge looked in the context of the original commit, or to figure where the change came from.

The exact order is orthogonal to the issue at hands, I think.

Sorry, "-d ≡ --first-parent --patch" you suggest contradicts my view on the whole scheme of things, for several reasons:

* I still find it problematic if -d, intended to fit nicely among --cc,
-c, -d, -m, -p, --remerge-diff options, suddenly implies --first-parent.
This would bring yet another inconsistency, and I don't want to be the
one who introduced it.

* In its current state -d conveniently means: "gimme simple diff output
for everything", where --first-parent you suggest doesn't fit at all.

* Current -d implementation is semantically as close to -p as possible,
tweaking exactly one thing compared to -p: the format of output for
merge commits, so is simpler than what you suggest from all angles, as
--first-parent tweaks more than one thing.

* To me what you argue for looks mostly like a desire to have a
short-cut for "--first-parent --patch", and my patch in question does
not seem to contradict this desire, as it'd be very surprising if
somebody came up with the name "-d" for such a short-cut. Definitely not
me.

* Finally, if -d becomes "--patch --first-parent", how do I get back
useful "--patch --diff-merges=first-parent" part of it, provided
--first-parent is unreversable? And even if it were reversable, then

   git log -d --no-first-parent =
   git log --patch --first-parent --no-first-parent =
   git log --patch

is definitely not what is needed, nor frequent demand to revert implied things indicates optimal design. Compare this to

   git log -d --first-parent

that current -d provides for you to get what you need, and that unambiguously reads: "gimme *d*iff for all commits while following *first parent* through the history" (while, unlike, -p not requiring --first-parent to implicitly tweak diff for merges output).

Overall, after considering your concern, I'd still prefer to leave "-d" semantics as implemented, consistent with the rest of similar options, and let somebody else define more shortcuts for their frequent use-cases if they feel like it.

Thanks, -- Sergey Organov

P.S. I also figure that maybe our divergence comes from the fact that I consider merge commits to be primarily commits (introducing particular set of changes, and then having reference to the source of the changes), whereas you consider them primarily merges (joining two histories, and then maybe some artificial changes that make merges "evil"). That's why we often end up agreeing to disagree, as both these points of view seem pretty valid.

Sergey Organov <sorganov@gmail.com> writes:

It rarely is the case that two opposing world views are equally valid, though.

If there were an option that forbids any comparison output from a single parent commit (say --ndfnm "no-diff-for-non-merge"), then those with "merges are the primary thing, single-parent commits on the merged branches are implementation details" worldview would be commonly using "--diff-merges=first-parent --patch --ndfnm" and (1) viewing only the combined effect of merging side branches without seeing noise from individual commits whose effects are already shown in these merges, and (2) traversing the side branches as well, so that merges from side-side branches into the side branches are viewable the same way as merges into the mainline.

But because no such option exists and nobody asked for such an option during the whole lifetime of the project, I highly doubt that it is a valid world view with wide backing from the users.

Even if it were a valid world view with wide backing, the combination "--diff-merges=first-parent --patch" would be less than ideal presentation for them (due to lack of "--ndfnm"). And as I already said, it would not be useful without --first-parent traversal for the worldview git has supported.

That is why I find it of dubious value to let short-and-sweet '-d' be squatted by less than ideal "--diff-merges=first-parent --patch" combination. Shorthands are scarse resources, and we want to be careful before handing them out.

Thanks.

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

Yes. In this particular case the two are not opposing though, rather orthogonal, as they reflect the intrinsic dualism of the concept of "merge commit". Merge commit is both a new state, and history junction, neither of which is more or less valid or essential, and I use both views myself, depending on situation.

An electron is both a particle and a wave, and one just uses its side that is more convenient for explanation of the case in hand.

I promote features that I routinely need in my workflows, yet respecting the other side of the coin as well, even though I may rarely find this other side useful. I mean, for me, this -c/--cc (let alone -m) output is only confusing, yet I won't be saying that it's somehow less valid than proposed -d.

No need to ask for a new option, as the behavior you describe is already there, and is spelled "git log --diff-merges=first-parent" (--diff-merges=1 for short).

Your concern above seems to be void, so this doesn't hold either.

As a side-note though, something like this has been asked recently, as what you describe by --ndfnm should in fact have been what --no-patch does, but surprisingly does not (please recall recent discussion of this issue).

Apparently it is valid, otherwise there would be no need for diff to first parent at all, let alone "git log --first-parent -p" have used it by default.

First, as we figured above, --ndfnm is not needed, and second, me, being one of "them", tries hard to convince you it is the best presentation "them" can get, while "ideal" simply never exists.

Yes, you said it earlier in this thread, and as well I already explained how it is useful without --first-parent.

Hopefully I do understand your concerns, yet I believe "--diff-merges=first-parent --patch" is way better for "-d" shortcut than "--first-parent --patch", for the reasons I already explained earlier in this thread.

Yep, agreed.

I believe I carefully thought it over though, weighing all pros and cons, and thus -d fits nicely among -c and --cc, being yet another frequently desired format for merges, plays nice with -p as well, and is very mnemonic, giving us convenient, user-friendly, and consistent user interface overall.

Thanks, -- Sergey Organov

Sergey Organov <sorganov@gmail.com> writes:

Ah, that changes things.  

Making "--diff-merges=<how>" only about the presentation of merge commits, requiring a separate "-p" for single-parent commits [*], does make the life for those in the "merges are the only interesting things" camp a lot easier, exactly because the lack of "-p" can be used to say "I am not interested in chanages by single-parent commits".

	Side note: I personally think it is a design mistake of
	--diff-merges=<how> (e.g., --cc and --diff-merges=cc do not
	behave the same way) but that is a different story, and it
	is way too late now anyway to "fix" or change.

So "-d" that stands for "--diff-merges=first-parent -p" makes the more useful (to those who think "merges are the only interesting things", which I do not belong to) "--diff-merges=first-parent" (without "-p") less useful. And the combination is not useful for those of us who find individual patches plus tweaks by merges (either --cc or --remerge-diff) are the way to look at the history.

I still do not think that we want to give a short-and-sweet single letter option for such a combination.

Thanks for clarification.

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

Only a tiny bit, unfortunately, as I'm still struggling to finally convince you (((

        Side note: This has been considered and agreed upon when
        --diff-merges= options were introduced, and as far as I recall,
        at that time you explicitly agreed it might be useful to be able
        to get output only for merge commits.

        --cc is a simple alias for "--diff-merges=cc --patch" nowadays,
        so yes, they do behave differently, and that's by design. Dunno
        see any design mistake here, as we get all useful variations of
        behavior with a straightforward design, more frequent use-cases
        served by shorter options. Looks fine.

Yes, you have your --cc, -c, and --remerge-diff (that I'd call something like --rd probably, but anyway). Could I please have my simple, straightforward, mnemonic, and terribly useful "-d" as well?

In other words, will I finally be faced with "if you need it, do it yourself" argument? ;)

I have very simple desire: convenient way to tell Git to show me diff to the first parent for merge commits, as that's the thing I need 99% of times when I do request diff output at all. That's exactly what I'd have seen as changes when I was about to commit the merge as well, similar to any other commit. It's so natural that I can't figure why it looks so damn rare or unusual to you, and that it makes you argue so hard against -d, especially when -p, -c, --cc, or even -m, are already there?

I do sympathize your desire to be careful about short options, but what reservation for "-d" do you still have in mind? It seems that it was just waiting for me to come and finally bring it to life with the best meaning possible. How long should I wait for it to remain unused to finally be able to make use of it?

Thanks, -- Sergey Organov

* Put descriptions of convenience shortcuts first, so they are the
  first things reader observes, not lengthy stuff.

* Add explanation note on '-m' not implying '-p' unlike similar
  options.

* Get rid of very long line containing all the --diff-merges formats
  by replacing them with <format>, and putting each supported format
  on its own line.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 97 +++++++++++++++++++---------------
 Documentation/git-log.txt      |  2 +-
 2 files changed, 55 insertions(+), 44 deletions(-)

Sergey Organov <sorganov@gmail.com> writes:

It is more like that `-p` does not imply `-m` (which used to mean "consider showing the comparison between parent(s) and the child, even for merge commits"), even though newer options like `-c`, `--cc` and others do imply `-m` (simply because they do not make much sense if they are not allowed to work on merges) that may make new people confused. If `-p` implied `-m` (or if `-m` implied `-p`), it would also have been utterly confusing and useless for human consumption.

In either case, unless the reason why `-p` does not imply `-m` unlike others is explained, I do not think the note adds that much value. I'd suggest dropping it.

Looks reasonable, even though I didn't quite see much problem with the original. If we were shuffling the sections like this patch, I wonder if moving combined/dense-combined a bit higher (perhaps before the "remerge") may make more sense, though (the ordering would simply become "simpler to more involved").

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

No, neither --cc nor -c imply -m.

-m is documented to produce very specific output that is neither -c nor --cc, and it's indeed how it works.

-c and --cc imply -p, not -m, and it has been documented for ages already, and it's indeed how it works, and that's what corresponding commits that added the features claim.

Overall, --cc, -c, and --remerge-diff all imply -p, whereas -m does not. This is simple fact.

So I feel we need to document why -m doesn't imply -p as other similar options do.

Fortunately, -p does not imply -m, but if -m implied -p, similar to --cc and -c, it'd be rather very natural, and thus people keep asking why it's not the case.

-p does not imply others. It's others (--cc, etc.) that imply -p.

The problem being solved is that we periodically get (valid) questions why -m does not behave similar to -c and --cc, and now --remerge-diff.

The original --diff-merge=... line was so long it didn't fit, especially after "remerge" has been added, and also was hard to grok.

I kept original order, but I agree combined/dense-combined fit better above remerge.

I'll change the order in re-roll.

Thanks, -- Sergey Organov

Sergey Organov <sorganov@gmail.com> writes:

I was only trying to help you polish the text you added to explain what you called the "legacy feature" to reflect the reason behind that legacy. As you obviously were not there back then when I made "--cc" imply "-m" while keeping "-p" not to imply "-m".

Our "git log [--diff-output-options]" logic was (and still is) not to show the comparison between parents and the child by default for merge commits even with -p/--raw/--stat (these were what existed and were common back then) and "git log --raw/--stat/-p" showed the raw/diffstat/patch after the log message for one-parent commits but only the log message for merges.

The reason behind that design choice is that Linus (and I and others) did not find that the patches for merges are as useful as patches for regular commits. We made "git log -p" to omit patches for merges that tend to become large.

	Side note: the first-parent patch is sort of readable, but
	if you are not doing the "--first-parent" traversal (which
	is a much later invention, so it wasn't even an option), you
	are showing individual commits and their patches while
	traversing the side branch, then the first-parent patch of a
	merge amounts to the squash of individual changes on the
	side branch that got merged.  It was deemed redundant
	presentation that is just wasteful and harder to grok than
	reading individual commits.  Worse, the patch against second
	and later parent(s) have no real value (it shows how behind
	the fork point of the side branch was relative to the tip of
	the trunk, which is rarely useful).

But we also wanted to have a mode of "git log -p" that spews everything to the output that could be used to reconstruct the history, hence we added "-m" to tell "git log":

	By default, you are designed not to show comparison between
	parents and the child for merge commit.  But when "-m" is
	given, do show the comparison for merge commit in the format
	that other options given to you, like --raw, --patch,
	specifies.

We however didn't have a good idea how to represent such a comparison between parents and the child, so we chose the most redundant, verbose, and obvious, which is N pairwise patches with each of N parents to the child (for a N-parent patch).

Later "--cc" and "-c" came as an alternative way to represent comparison between parents and the child.

Given that I, together with Linus, invented "--cc" and "-c", taking inspiration from how Paul Mackerras showed a merge in his 'gitk' tool, and made the design decision not to require "-m" to get the output in the format they specify when the "git log" traversal shows merge commits, I do not know what to say when you repeat that "--cc" does not imply "-m". It simply is not true.

I think this is the second time you claimed the above after I explained the same to you, if I am not mistaken. If you do not want to be corrected, that is fine, and I'll stop wasting my time trying to correct you.

But I still have to make sure that you (or anybody else) do not spread misinformation to other users by writing incorrect statements in documentation patches.

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

Your help is appreciated, yet unfortunately I still can't figure how to improve the text based on your advice.

Your "I made --cc imply -m" does not explain why later, when you made --cc imply -p (did you, or was it somebody else?), you didn't make -m imply -p at the same time, and then "while keeping -p not to imply -m" sounds out of place as we rather try to figure why "-m not implies -p".

The "--c imply -m" part of the help raises yet another question: if --cc implied -m, why it was not -m that was made to imply -p instead of --cc (and -c)? Then both --cc and -c would imply -p automatically as a side-effect of implication of -p by -m (do not confuse with agreed non-implication of -m by -p), and then all the relevant options were consistent. This consideration renders current situation more surprising instead of clarifying it, I'm afraid.

"-p does not imply -m" fact is fine with me and is not the cause of user confusion I'm trying to address. How does it help us to explain why "-m does not imply -p" though?

[...]

I keep saying "--cc does not imply -m" because it does not seem to, unless you either use some vague meaning of "imply", or mean some other "-m", not the one used in "git log". Please check:

$ cd src/git $ git --version git version 2.42.0.111.gd814540bb75b $ git describe v2.42.0-111-gd814540bb75b $ git log 74a2e88700efc -n1 -p --cc > diff.actual $ git log 74a2e88700efc -n1 -p --cc -m > diff.expected $ cmp diff.expected diff.actual diff.expected diff.actual differ: byte 706, line 18 $

This test tells us that "--c" is not the same as "--cc -m", that for me in turn reads "--cc does not imply -m", and that's what I continue to say.

I'd love to be corrected, but I think I carefully checked my grounds before saying that --cc does not imply -m, please consider:

1. "--cc implies -m" is not documented. Please point to the
   documentation in case I missed it.

2. Git does not behave as if "--cc implied -m", see the test-case above.

If it's neither documented nor matches actual behavior, it's not there, at least from the POV of random user, to whom my original clarification of "why -m does not imply -p?" has been addressed.

On top of that, I even can't figure why we argue about it in the first place, as it seems to be irrelevant to the issue at hand: explain why -m does not imply -p?

I'm all against spreading misinformation, and try my best to avoid it myself. I still fail to see what misinformation, exactly, you find in this particular explanation by me:

" Note: This option [`-m`] not implying `-p` is legacy feature that is
  preserved for the sake of backward compatibility. "

That's exactly what I figured out from a lot of discussions over my multiple attempts to make `-m` behave more usefully. Is it that "legacy feature" somehow sounds offensive, or what?

As, despite your help, I fail to come up with better edition of the note, please, if you feel like it, suggest your own variant of explanation to the user why `-m` is left inconsistent with the rest of diff for merges options, provided current matching documentation reads roughly like this (from more recent options to oldest):

   --remrege-diff: produces "remerge" output. Implies -p.
   --cc: produces dense combined output. Implies -p.
   -c: produces combined output. Implies -p.
   -m: produces separate output, provided -p is given as well (?!).

and so why

  git log -m

surprisingly has no visible effect, and then the user needs to type:

   git log -m -p

That's all I wanted to explain to the user in a few words with the note you argue against.

Thanks, -- Sergey Organov

Sergey Organov <sorganov@gmail.com> writes:

If I were doing this patch, I would start from something like this:

-m::
	By default, comparisons between parent commits and the child
	commit are not shown for merge commits, but with the `-m`
	option, `git log` can be told to show comparisons for merges
	in chosen formats (e.g. `--raw`, `-p`, `--stat`).  When
	output formats (e.g. `--cc`) that are specifically designed
	to show better comparisons for merges are given, this option
	is implied; in other words, you do not have to say e.g. `git
	log -m --cc`.  `git log --cc` suffices.

The rest is a tangent that is not related to the above. I suspect that this also applies to newer `--remerge-diff`, as it also targets to show merges better than the original "pairwise patches" that were largely useless, but the right way to view what `--cc` and other formats do for non-merge commits is *not* to think that they "imply" `-p`. It is more like that the output from these formats on non-merge commits happen to be identical to what `-p` would produce. You could say that the "magic" these options know to show merge commits better degenerates to what `-p` gives when applied to non-merge commits.

Another way to look at it is that `--cc` and friends, even though they are meant as improvements for showing merges over "-m -p" that gives human-unreadable pair-wise diffs, do not imply "--merges" (i.e. show only merge commits)---hence they have to show something for non-merge commits. Because output formats for all of them were modeled loosely [*] after "-p" output, we happened to pick it as the format they fall back to when they are not showing comparisons for merge commits.

[Footnote]

 * Here, `-p` roughly means "what GNU patch and `git apply` take".
   Output from `-c` and `--cc` on merge commits do not qualify, but
   they are loosely modeled after it.

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

Well, to me this piece looks much harder to understand than current Git documentation, and then seemingly contradicts current Git behavior and implementation, as "log --cc -m" is not the same as "log --cc" in the current Git (so we can't say that --cc implies -m), and "log -m --cc" is the same as "log --cc" due to absolutely different reason: -m and --cc are mutually exclusive options, so the last one simply takes precedence.

In the current Git, as documented, -m just produces separate diff with respect to every parent. Simple and straightforward. Users don't need to learn about --cc, -c, --raw, --stat... to figure what -m does and if it's what they need. Unfortunately they still need to learn about -p, but I'm already done trying to promote this simple change.

I admit you are very creative producing these views,, but currently these options just imply -p. Simple to understand, useful, works.

Overall, as you don't like my simple clarification, and I don't like the direction(s) you propose, I figure I rather withdraw the part of patch causing contention in the re-roll.

Thanks, -- Sergey Organov

This new convenience option requests full diff with respect to first parent, so that

  git log -d

will output diff with respect to first parent for every commit, universally, no matter how many parents the commit turns out to have.

It's implemented as pure synonym for

  --diff-merges=first-parent --patch

The first commit in the series tweaks diff-merges documentation a bit, and is valuable by itself. It's put here as '-d' implementation commit depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.

Updates in v2:

  * Reordered documentation for diff-merges formats in accordance with
    Junio recommendation.

  * Removed clarification of surprising -m behavior due to controversy
    with Junio on how exactly it should look like.

Sergey Organov (2):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '-d' option

 Documentation/diff-options.txt | 102 ++++++++++++++++++---------------
 Documentation/git-log.txt      |   4 +-
 diff-merges.c                  |   3 +
 t/t4013-diff-various.sh        |   8 +++
 4 files changed, 70 insertions(+), 47 deletions(-)

Interdiff against v1:

This option provides a shortcut to request diff with respect to first parent for any kind of commit, universally. It's implemented as pure synonym for "--diff-merges=first-parent --patch".

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 4 ++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 16 insertions(+), 1 deletion(-)

* Put descriptions of convenience shortcuts first, so they are the
  first things reader observes rather than lengthy detailed stuff.

* Get rid of very long line containing all the --diff-merges formats
  by replacing them with <format>, and putting each supported format
  on its own line.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 98 ++++++++++++++++++----------------
 Documentation/git-log.txt      |  2 +-
 2 files changed, 54 insertions(+), 46 deletions(-)

This new convenience option requests full diff with respect to first parent, so that

  git log --dd

will output diff with respect to first parent for every commit, universally, no matter how many parents the commit turns out to have.

'--dd' is implemented as pure synonym for "--diff-merges=first-parent --patch".

The first commit in the series tweaks diff-merges documentation a bit, and is valuable by itself. It's put here as '--dd' implementation commit depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do. So, basically,
'--dd' is what '-m' should have been to be more useful.

Updates in v3:

  * Option renamed from '-d' to '--dd' due to Junio overpowering
    request to keep short-and-sweet '-d' reserved for another (yet
    unspecified) use.

  * Added completion of '--dd' to git-completion.bash.

Updates in v2:

  * Reordered documentation for diff-merges formats in accordance with
    Junio recommendation.

  * Removed clarification of surprising -m behavior due to controversy
    with Junio on how exactly it should look like.

Sergey Organov (3):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '--dd' option
  completion: complete '--dd'

 Documentation/diff-options.txt         | 103 ++++++++++++++-----------
 Documentation/git-log.txt              |   4 +-
 contrib/completion/git-completion.bash |   2 +-
 diff-merges.c                          |   3 +
 t/t4013-diff-various.sh                |   8 ++
 5 files changed, 72 insertions(+), 48 deletions(-)

Interdiff against v2:

This option provides a shortcut to request diff with respect to first parent for any kind of commit, universally. It's implemented as pure synonym for "--diff-merges=first-parent --patch".

NOTE: originally proposed as '-d', and renamed to '--dd' due to Junio
request to keep "short-and-sweet" '-d' reserved for other uses.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 5 +++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 17 insertions(+), 1 deletion(-)

Sergey Organov <sorganov@gmail.com> writes:

That explains what the patch does, but it does not tell us why it is useful [*].

The note is not grammatical, and more importantly, readers of "git log" 6 months down the road would not care. I'd rather not see it in the proposed log message. It is suitable material to place after the three-dash line, or in the cover letter for the iteration.

Quite straight-forward as expected. I do not think "--dd" clicks for many people as "first parent diffs all over", though.

Looking good.

Thanks.

[Footnote]

* As I said elsewhere, I do not think it is a good idea to encourage
  users' to adopt a screwed-up worldview in which first parent is
  special but not special, and does the wrong thing for reverse
  merges.  If the option were short-hand for "--first-parent -p",
  at least I would be more sympathetic.

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

OK, will get rid of it.

Thanks, -- Sergey Organov

* Put descriptions of convenience shortcuts first, so they are the
  first things reader observes rather than lengthy detailed stuff.

* Get rid of very long line containing all the --diff-merges formats
  by replacing them with <format>, and putting each supported format
  on its own line.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 98 ++++++++++++++++++----------------
 Documentation/git-log.txt      |  2 +-
 2 files changed, 54 insertions(+), 46 deletions(-)

On Wed, Oct 4, 2023 at 5:51 PM Sergey Organov <sorganov@gmail.com> wrote:

I'm having difficulty grasping the parenthetical "(which see)" comment.

Eric Sunshine <sunshine@sunshineco.com> writes:

I believe it's translated full form of q.v., see:

https://en.wikipedia.org/wiki/List_of_Latin_abbreviations

"q.v.
 quod vide
 "which see"

Imperative, used after a term or phrase that should be looked up elsewhere in the current document or book."

HTH, -- Sergey Organov

Eric Sunshine <sunshine@sunshineco.com> writes:

I am, too. I know what it means when written in the more common Latin abbreviation (q.v.), but I suspect it may be rare to spell it in English like this. I found

https://writingcenter.unc.edu/tips-and-tools/latin-terms-and-abbreviations/

that starts its explanation with this:

     The abbreviation q.v. stands for quod vide, which translates
     literally as “which see,” although in practice it mea
     something more like “for which see elsewhere.

and it goes on to say:

     The reader is expected to know how to locate this information
     without further assistance. Since there is always the
     possibility that the reader won’t be able to find the
     information cited by q.v., it’s better to use a simple English
     phrase such as “for more on this topic, see pages 72-3” or
     “a detailed definition appears on page 16.” Such phrases are
     immediately comprehensible to the reader (who may not even know
     what q.v. means) and remove any ambiguity about where
     additional information is located.

which only applies halfway to this example, as with the text before it makes it very clear for readers that they need to learn about "--diff-merges=on". It is so clear to the point that the only effect "(which see)" here has is to waste bytes and confuses readers, I am afraid.

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

Well, I didn't invent it, and didn't lookup it until asked, it just popped-up out of my head somehow.

I'll remove it as causes confusion.

Thanks, -- Sergey Organov

Sergey Organov <sorganov@gmail.com> writes:

I think the sentence reads better without the translated (q.v.) that confused Eric.

Good.

I suspect that many people do not get what "re-merge" in "against re-merge" really is. As "combined diff" and "dense combined diff" are not explained in the previous two entries either, and expect the readers to read the real description (which more or less matches what the original description for "-c" and "--cc" had, which is good), it would be better to say "Produce remerge-diff output for merge commits." here, too. It makes it consistent, and "for merge commits" makes it clear the "magic" does not apply to regular commits (which the above entries for "-c" and "--cc" do, which is very good).

This reads well.

In the longer term, "--diff-merge=first-parent" that is used without first-parent traversal should be discouraged and be deprecated, I think, but that is a separate story [*].

The original is already wrong so these are not problems this patch introduces, but

 - "configuration variable" is how we refer to these entities.
 - "which default value" -> "whose default value".

Yes, this is the same output as `-p`, as if parents other than the first parent of the merge commit did not exist.

This was inherited from the original elsewhere, but it makes it unnecessary confusing to say "full diff" here and in the next one.

    Show `--patch` output with respoect to the first parent for a
    merge commit, as if the other parents did not exist.

perhaps?

In the early days of Git before -c/--cc were invented, we explained this mode as "pairwise comparison", and the phrase "pairwise" still may be the best one to describe the behaviour here. In fact, we see in the updated description of combined below the exact phrase is used to refer to this oldest output format.

    Show the `--patch` output pairwise, together with the commit
    header, repeated for each parent for a merge commit.

or something, perhaps. I added "repeated" here to make the contrast with "simultaneously" stand out.

The original says "two-parent merge comimts are remerged" so it is not a failure of this patch, but the first verb "Remerge" sounds unnecessarily unfriendly to the readers.

	For a two-parent merge commit, a merge of these two commits
	is retried to create a temporary tree object, potentially
	containing files with conflict markers.  A `--patch` output
	then is shown between ...

would be easier to follow and more faithful to the original description added by db757e8b (show, log: provide a --remerge-diff capability, 2022-02-02).

Either way, it makes readers wonder what happens to merges with more than 2 parents (octopus merges). It is not a new problem and this topic should not attempt to fix it.

Looks very good otherwise.  Let me read on.

Thanks.

[Footnote]

* When a project allows fast-forward merges, something like this can
  happen (and Git was _designed_ to allow and even encourage it)

  - Linus pulls from Sergey and sees merge conflicts that are very
    messy.  Sergey is asked to resolve the conflict, as Linus knows
    Sergey understands the changes he is asking Linus to pull much
    better than Linus does.

  - Sergey does "git pull origin" that would give the same set of
    conflicts Linus saw, perhaps ours/theirs sides swapped, resolves
    the conflicts, and comits the merge result.  He may even add a
    few other improvements on top (or may not).  He tells Linus that
    his tree is ready to be pulled again.

  - Linus pulls from Sergey again.  This time it is fast-forward,
    without an extra merge commit that records the Linus's previous
    tip as the first parent and Sergey's work as the second parent.

  - Linus continues working from here.

  In such a workflow, merges are nothing more than "combining
  multiple histories together" and the first parenthood is NOT
  inherently special among parents at all.  The original "-m -p"
  (aka "pairwise diff") output reflects this world view and ensures
  that all parents are shown more or less as equals (yes, the first
  parent diff is shown first before the other parents, but you
  cannot avoid it when outputting to a single dimension medium).

  This world view was the only world view Git supported, until I
  added the "--first-parent" traversal in 0053e902 (git-log
  --first-parent: show only the first parent log, 2007-03-13).

  With the "--first-parent", with "--no-ff" option to "git merge", a
  different world view becomes possible.  A merge is not merely
  combining multiple histories, which are equals.  It is bringing
  work done on a side branch into the trunk.  To see the overview of
  the history, "git log --first-parent" would give the outline,
  which would be full of merges from side branches, each of which
  can be seen as summarizing the work done on the side branch that
  was merged, and it may occasionally have single-parent commits
  that are hotfixes or trivial clean-ups or project administrivia
  commits.  With "-p", "git log" would show the changes the work
  done on a side branch as a single unit for a merge, and individual
  commits if they are single-parent.  The life is good.

  It all breaks down if the "diff against the first parent" is done
  on a merge that is not bringing the work on a side branch in to
  the trunk.  The merge done in the second step Sergey did for Linus
  in the above example will have his work on the history leading to
  its first parent, and from the overall project's point of view,
  the second parent is the tip of the history of the trunk.  Showing
  first-parent diff for a merge that was *not* discovered via the
  first-parent traversal would show such a meaningless patch.  This
  is an illustration of the fallout from mixing two incompatible
  world views together, "--diff-merges=first-parent" wants to work
  in a world where the first-parent is special among parents, but
  traversal without "--first-parent" wants to treat all the branches
  equally.

  All the other <format>s accepted by the "--diff-merges=<format>"
  option are symmetrical and they work equally well when in a
  history of a project that considers the first-parenthood special
  (i.e. work on a side branch is brought into the trunk history) or
  in a history with merges whose parent order should not matter, so
  unlike "--diff-merges=first-parent", it makes sense to apply them
  with or without first-parent traversal.  It however is not true
  for the "--diff-merges=first-parent" variant, which is asymmetric.

  And that is why I think use of "--diff-merges=first-parent"
  without "--first-parent" traversal is a bad thing to teach users
  to use.

Hi,

On Thu, Oct 5, 2023 at 2:24 PM Junio C Hamano <gitster@pobox.com> wrote:

Agreed; confused me too.

Perhaps:

Produce remerge-diff output for merge commits, in order to show how conflicts were resolved.

I like this.

I like it. Perhaps it may also benefit from explaining why this mode is useful as well:

    For a two-parent merge commit, a merge of these two commits is
    retried to create a temporary tree object, potentially containing
    files with conflict markers.  A diff is then shown between that
    temporary tree and the actual merge commit.  This has the effect
    of showing whether and how both semantic and textual conflicts
    were resolved by the user (i.e. what changes the user made after
    running 'git merge' and before finally committing).

We could add:

    For octopus merges (merges with more than two parents), currently
only shows a warning about skipping such commits.

if wanted.

But perhaps I've distracted too much from Sergey's topic, and I should submit these wording tweaks as a patch on top? I'm fine either way.

Thanks for writing this up. In the past, I didn't know how to put into words why I didn't particularly care for this mode. You explain it rather well.

Elijah Newren <newren@gmail.com> writes:

Will remove, no problem.

Thanks, -- Sergey Organov

Elijah Newren <newren@gmail.com> writes:

Will use this description in re-roll.

Please let's left it for some follow-up, as this patch does not rephrase original, just changes the presentation.

Elijah Newren <newren@gmail.com> writes:

I do not mind it, but then I'd prefer to see ", in order to show how" also in the description of "--cc" and "-c" for consistency.

A succinct way to say what they do may be hard to come by, but I think of them showing places that did not have obvious natural resolution.

Yes, and because we do not have a back reference from here to the description for "--remerge-diff" we saw earlier, we'd need the "in order to" you suggested earlier there, too.

The primary purpose of polishing during a review cycle should be to help the original contributor to express what they wanted to express better, so talking about octopus behaviour, which wasn't covered in the original nor the patch under review, can be left out to avoid extending the scope of the topic further.

But everything else you said in the message I am responding to falls into the scope of the "improving existing documentation for various merge presentation modes" topic, I would think, and they are more or less usable verbatim, so it would not be too much of a burden to make sure they are used in the next iteration.

Thanks for a review, and thanks Sergey for streamlining the documentation around here.

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

So, is it OK with both of you if I leave it as:

"Produce remerge-diff output for merge commits."

for now, and let you tweak the descriptions later on, if needed?

Thanks, -- Sergey Organov

Sergey Organov <sorganov@gmail.com> writes:

I do not know what Elijah would say, but in one of iterations of my draft response to him indeed suggested that "in order to" here is not necessary if it is described for the "--diff-merges=remerge" option, because those who know enough to skip referring to the other entry are expected to know why it exists. So I think I am OK with that.

Thanks.

On Fri, Oct 6, 2023 at 11:01 AM Junio C Hamano <gitster@pobox.com> wrote:

The problem is it's really hard for me to come up with an answer to that, in part because...

In my opinion, --remerge-diff does this better; wouldn't we want a rationale where these particular modes shine? Is that a non-empty set? (It may well be, but to me, --cc was never worse than -c while often being better, and likewise, --remerge-diff is never worse than --cc while often being better, at least on anything I had thought to use any of these for. Maybe there are other usecases for -c and --cc I'm just not thinking of?)

Elijah Newren <newren@gmail.com> writes:

Between -c and --cc, I do not think there is anything that makes us favor -c over --cc. While the algorithm to decide which hunks out of -c's output to omit was being polished, comparison with -c served a good way to give baseline, but once --cc has become solid, I do not think I've used -c myself.

I personally find that a very trivial merge resolution is far easier to read with --cc than --remerge-diff, the latter being way too verbose.

Also, --cc and -c should work inside a read-only repository where you only have read access to. If remerge needs to write some objects to the repository, then you'd need some hack to give a writable object store overlay via the alternate odb mechanism, or something, right?

$ git show --oneline --cc -U1 9fde277c338 9fde277c33 Merge branch 'cc/git-replay' into seen

diff --cc Makefile index cf60c16deb,05a504dc28..c581c1ddba

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

Well, the above did not come out as well as I intended, as I forgot to prefix it with something I thought was obvious from what I said in the recent discussion in the earlier iteration of this topic, where I said that it would be "--remerge-diff", if I were to pick an option that is so useful that it deserves short and sweet single letter. Narutally, it came after we gained experience with "--cc", so it would be surprising if it did worse. Just like it is natural to expect that "--cc" would give more useful output than "-m -p" that predates everybody else.

In short, I would say "--remerge-diff" would give output that is the easiest to grok among the three modern variants to show the changes a merge introduces.

The above two cases, where I said cc does better than remerge-diff, were meant as _exceptions_ for that general sentiment.

On Fri, Oct 6, 2023 at 11:49 PM Junio C Hamano <gitster@pobox.com> wrote:

Perhaps, then, the user manual should either omit -c, or recommend users use --cc instead?

Ah, indeed, for those that know the --cc output format well (it takes a bit to figure out for newcomers), your example demonstrates this nicely. Thanks.

Well, it does use a temporary object store with the alternate odb mechanism already, but I don't think there's any code to allow the user to input the location for the temporary store, and thus we'd probably attempt to write it underneath the same read-only directory. So, yes, read-only repositories would likely be problematic for --remerge-diff.

However, are read-only repositories worth mentioning in the documentation here?

Thanks, this is useful. This does make me wonder, though: Should we perhaps guide users as to what we recommend (and recommend against) in this documentation?

If we have lots of options and they all shine on different usecases, it makes sense to just provide a long list of possibilities for users. But if we generally feel that one is entirely supplanted by another (e.g. -c by --cc) it seems beneficial to mention that, and if we generally feel that one will often be clearer or more useful than the others (e.g. --remerge-diff), it seems beneficial to recommend it. Thoughts?

Also, perhaps this would be best to include in a follow-up series (as it appears from Sergey's latest iteration that we are leaving other tweaks for a later series anyway), if we do decide we want to do it...

Elijah Newren <newren@gmail.com> writes:

I do not think I'd miss "-c", but I do not know about others.

Yup. And newcomers would take a bit to figure out remerge-diff output, too, so my answers were written from the "nobody will stay newcomer forever. now once they get proficient enough, which ones are good for them" viewpoint.

Elijah Newren <newren@gmail.com> writes:

I am glad it helped non-zero number of people.

It probably owes big to my failing, but because we strongly view it a virtue not to be opinionated, we have many discrete tools and features that can be used in combination to support a workflow well, even when some of these tools and features are not useful for a different workflow. It indeed is a good thing to be flexible and to support different workflows well, and we tend not to single out a workflow among many and advocate it, but because our documentation lacks description of major possible workflows, what their underlying philosophies and their strengths are, how some of our tools and features support them, and why some others are not good fit. Being given a toolbox with too many tools without being taught how they are to be used together and for what purpose may be a fun puzzle to figure out for tinkerers, but when you have a problem to solve and tinkering is not your main focus, which is true for most people, it is not fun.

In short, in pursuit of not to be opinionated, we fail to give the readers best current practices. The first place to start rectifying it might be to have some write-ups for various major workflows and the worldview behind them. The importance given to first-parenthood offers two quite different worldviews that affects the choice of tools (e.g. "merge --no-ff", "checkout origin/master && merge mine && branch -f mine" aka "reverse merge").

I suspect that this also relates to your "would --cc be totally unnecessary now we have --remerge-diff?" as well. What kind of conflicts are interesting highly depends on what you are looking for, which in turn is influenced by the workflow employed by the project and what role you are playing in it.

On Mon, Oct 9, 2023 at 7:44 PM Junio C Hamano <gitster@pobox.com> wrote:

I completely agree with you. The #1 conversation I have with friends who are new to Git is figuring out which of the major workflows they should use, and what are the drawbacks and benefits. And how to diagnose based on the existing commit history, review tool in use, etc which one is used by their peers. Having this documented somewhere - maybe in Pro Git book, maybe in manpage - would be hugely useful. I could envision a diagram of a sample commit history, and "if it already looks like this or you want it to look like this, your workflow should be blah" and finally a list of some drawbacks, benefits, and warnings about what to avoid in that workflow. Plus, perhaps, many shout-outs to `git reflog` for fixing when you misunderstood and made a mess. (that's the #2 conversation I have :) )

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

[...]

I fail to see why useful harmless feature is to be deprecated. I believe users are pretty capable to decide if they need it by themselves, without our guidance.

Thanks, -- Sergey Organov

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

[...]

Added this amendment to the patch.

Thanks, -- Sergey Organov

'--dd' only makes sense for 'git log' and 'git show', so add it to __git_log_show_options which is referenced in the completion for these two commands.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Sergey Organov <sorganov@gmail.com> writes:

Quite straight-forward. I am kind of surprised that we do not have to list "--cc" here. Perhaps it is so short and common that people do not need completion help?

But that is not a new problem caused by this series, so it is OK.

Unless "--cc" gets completed without being listed here, using some automation like the "--git-completion-helper" option, in which case we may want to see if we can remove all of the above and complete them the same way as "--cc" gets completed. I didn't check.

Thanks.

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

I checked, though with rather old 2.25.1 running on my Ubuntu, and it is not completed.

I think that it's still a good idea to add --cc to completions, so that it's there in the suggested completion list, for the sake of discoverability. That's why I bothered to add --dd to the completions.

Thanks, -- Sergey Organov

This new convenience option requests full diff with respect to first parent, so that

  git log --dd

will output diff with respect to first parent for every commit, universally, no matter how many parents the commit turns out to have.

Gives user quick and universal way to see what changes, exactly, were brought to a branch by merges as well as by regular commits.

'--dd' is implemented as pure synonym for "--diff-merges=first-parent --patch".

The first commit in the series tweaks diff-merges documentation a bit, and is valuable by itself. It's put here as '--dd' implementation commit depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do. So, basically,
'--dd' is what '-m' should have been to be more useful.

Updates in v4:

  * Removed "(which see)" reference from documentation that caused
    confusion.

  * Removed explanation why it's --dd and not simply -d from commit
    message.

  * Refined --remerge-diff short description according to Junio and
    Elijah comments.

  * Added explanation of --dd purpose.

  * Fixed style and syntax of "on,m::" description.

Updates in v3:

  * Option renamed from '-d' to '--dd' due to Junio overpowering
    request to keep short-and-sweet '-d' reserved for another (yet
    unspecified) use.

  * Added completion of '--dd' to git-completion.bash.

Updates in v2:

  * Reordered documentation for diff-merges formats in accordance with
    Junio recommendation.

  * Removed clarification of surprising -m behavior due to controversy
    with Junio on how exactly it should look like.

Sergey Organov (3):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '--dd' option
  completion: complete '--dd'

 Documentation/diff-options.txt         | 105 ++++++++++++++-----------
 Documentation/git-log.txt              |   4 +-
 contrib/completion/git-completion.bash |   2 +-
 diff-merges.c                          |   3 +
 t/t4013-diff-various.sh                |   8 ++
 5 files changed, 73 insertions(+), 49 deletions(-)

Interdiff against v3:

This option provides a shortcut to request diff with respect to first parent for any kind of commit, universally. It's implemented as pure synonym for "--diff-merges=first-parent --patch".

Gives user quick and universal way to see what changes, exactly, were brought to a branch by merges as well as by regular commits.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 5 +++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 17 insertions(+), 1 deletion(-)

* Put descriptions of convenience shortcuts first, so they are the
  first things reader observes rather than lengthy detailed stuff.

* Get rid of very long line containing all the --diff-merges formats
  by replacing them with <format>, and putting each supported format
  on its own line.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 100 ++++++++++++++++++---------------
 Documentation/git-log.txt      |   2 +-
 2 files changed, 55 insertions(+), 47 deletions(-)

'--dd' only makes sense for 'git log' and 'git show', so add it to __git_log_show_options which is referenced in the completion for these two commands.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Sergey Organov <sorganov@gmail.com> writes:

Will replace.  Thanks.