Thread (1 message) 1 message, 1 author, 2020-12-09

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

From: Sergey Organov <hidden>
Date: 2020-12-09 13:35:49

Elijah Newren [off-list ref] writes:
On Tue, Dec 8, 2020 at 8:18 PM Junio C Hamano [off-list ref] wrote:
quoted
Elijah Newren [off-list ref] writes:
quoted
quoted
If I can run "git log --some-options master..next" (or more
realistically, over the range ko/next..next) to get individual
commits (without patch) and merges (only when --cc gives some
interesting nearby-changes), I would be very happy.  But is there a
set of options that lets me do so?
So, you're saying you changed your mind since five years ago?[1]  Or
that what you said five years ago is still valid, but you'd appreciate
more/different options that allow this new thing?

[1]
https://lore.kernel.org/git/1440110591-12941-1-git-send-email-gitster@pobox.com/ (local)
Sorry, but I am not seeing in [1] anything that relates to the above
"want to see --cc patch for merge but just log message for single
parent commit". 5 years is a long time even in Git timescale, so I
would not be surprised if I changed my mind over time, but I am not
sure what opinion on the matter you think I expressed back then.

"git log --cc master..next" shows all commits' log messages, patch
for each single-parent commit, and combined-dense patch for each
merge.  There is no option to squelch the patch for only single
parent commits.  It may not be such a bad thing to have as an extra
option.

So, I think what I am saying is that ...
quoted
quoted
...  As I said, I'm not sure why
anyone would ever want to see diffs for merges and not for normal
commits, the closest useful thing I can imagine is commit messages +
diffs for just merges, stripping the normal commits.
... I see use for such a feature (assuming that you didn't mean by
"diffs for merges" a regular "--first-parent -p" patch, but meant to
say "--cc" patch) in my workflow.  I'd review "log ko/next..next"
before deciding to push out the day's integration of 'next', and at
that point, I trust individual commits that came from contributors
well enough (otherwise I wouldn't be merging them to 'next'), but I
would appreciate the last chance to re-examine conflict resolutions
in merges.

It does not mean that I do not like the current behaviour that
"--cc" always implies "-p"; it is convenient.  It's just I find the
lack of feature slightly less than ideal, but I do not care deeply
enough to design how to express such a feature from the command
line.
Okay, thanks for clarifying.  It sounds like you were focusing on the
tangentially related comment I made (diffs for merges and not for
normal commits) while I was focusing on Sergey's question (should we
revert --cc implies -p).  I was having a hard time understanding if
you were answering his question or not.  This last paragraph of yours
acknowledges the question, though you still avoid answering it.  :-)

However, even my focus was on a secondary question.  His real original
question is: -m and --cc are inconsistent -- one requires -p, while
the other doesn't.  Should that be fixed...and which option(s) should
change?  He gave two possibilities I didn't like.  I added a third
that you didn't like.  So...
I believe you've misunderstood me slightly.

I didn't suggest bare reverting of the "-c/-cc imply -p" commit. I
rather suggested to modify current behavior to "-c/--cc enable diff
output for merge commits", then add "-m" to the mix, so that we finally
get:

  "-m/-c/--c enable diff output for merge commits".

And I should emphasize that what I mean differs from "-m/-c/--cc imply -p
for merge commits only" as Junio has put it in this discussion, even if
slightly, -- it won't imply -p, to avoid messing with --first-parent
that would imply -p through -m and enable diff for merges, that is not
what we want.

This is what I'd like to see, as it finally makes -m/-c/--cc (as well as
other --diff-merges options) focus on merge commits only and never
affect regular commits, -- the way it should be.

My alternative suggestion was to rather add "-m implies -p" to the
current state, resulting in:

  "-m/-c/--c imply -p".

However, the latter one has additional complication in handling of
"--first-parent" that currently implies -m that'd then imply -p and
suddenly give lengthy output on bare "git log --first-parent".

Fixing the latter is still possible by complicating of options handling
by specifying that implied options don't imply other options, only
explicit options do, but somehow I don't like this, -- too complex for
the job at hand. I'm still on position that option management in Git
could rather be done in much simpler manner, without need for such
complexities.

Thanks,
-- Sergey
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help