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

From: Sergey Organov <[email-masked]>
Date: Wed, 09 Dec 2020 16:34:29 +0300

Elijah Newren <[email-masked]> writes:

> On Tue, Dec 8, 2020 at 8:18 PM Junio C Hamano <[email-masked]> wrote:
>>
>> Elijah Newren <[email-masked]> writes:
>>
>> >> 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/[email-masked]/
>>
>> 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 ...
>>
>> > > ...  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