Re: [PATCH 00/26] git-log: implement new --diff-merge options

Junio C Hamano [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

A clarification and a correction.

quoted

I suspect that the real reason why "-m" does not imply "-p" was
merely a historical implementation detail...

Now I remember better.  The reason was pure oversight.

In the beginning, there was no patch output for merges.  As most
merges just resolve cleanly, and back then the first-parent chains
were treated as much much less special than we treat them today,
"git log -p" showed only patches for single-parent commits and
everybody was happy.  It could have been a possible alternative
design to show first-parent diff for a merge instead of showing no
patch, but because the traversal went to side branches, the changes
made by the merge to the mainline as a single big patch would have
been redundant---we would be seeing individual patches from the side
branch anyway.

Then later we introduced "-m -p"; since the first-parent chain was
not considered all that special, we treated each parent equally.
Nobody, not even Linus and I, thought it was useful by itself even
back then, but we didn't have anything better.

I think it was Paul Mackerras's "gitk" that invented the concept of
combined merges.  We liked it quite a lot, and added "-c" and "--cc"
soon after that, to the core git and kept polishing, until "gitk"
stopped combining the patches with each parent in tcl/tk script and
instead started telling "git" to show with "--cc".

By the time the change to make "--cc" imply "-p" was introduced, it
was pretty much given that "-m -p" was useful to anybody, unless you
are consuming these individual patches in a script or something like
that.  So simply I didn't even think of making "-m" imply "-p".  It
would be logical to make it so, but it would not add much practical
value, I would have to say.

... and then later the "--first-parent implies -m" change has been made,
that would't work as expected if -m implied -p in the fist place, as
it'd break "git log --first-parent".

quoted

If I were to decide now with hindsight, perhaps I'd make "--cc" and
"-m" imply "-p" only for merge commits, and the user can explicitly
give "--cc -p" and "-m -p" to ask patches for single-parent commits
to be shown as well.

After "now with hindsight", I need to add "and without having to
worry about backward compatibility issues" here.  IOW, the above is
not my recommendation.  It would be the other way around: "--cc"
implies "-p" for both merges and non-merges, "-m" implies "-p" for
both merges and non-merges.  It is acceptable to add a new option
"--no-patch-for-non-merge" so that the user can ask to see only the
combined diff for merges and no patches for individual commits.

OK, so, do we decide that -c/--cc must continue to imply -p and thus
request diffs for everything?

If so, I can rather change --diff-merges=combined/dense-combined
behavior to /not/ imply -p, thus effectively making --cc a synonym for
"--diff-merges=dense-combined --patch", that will have zero backward
compatibility issues.

Looks like it'd have everything covered. Old options won't change their
behavior at all, and the new set of options will behave differently,
exactly if designed from scratch, providing new functionality.

Both "--no-patch-for-non-merge" option, and making "-m" imply "-p"
are very low priority from my point of view, though, since our users
(including me) lived without the former and have been happily using
"log --cc" for a long time, and we've written off the latter as
pretty much useless combination unless you are a script.

I think my above suggestion covers all the worries without need for this
nasty "--no-patch-for-non-merge" option.

That said, -m is useless, period. It'd likely have some merit in
plumbing, but definitely not in porcelain. So I'm inclined to let it
rest in peace indeed, dying.

Thanks,
-- Sergey