Re: [PATCH 2/2] diff-merges: introduce '-d' option

From: Junio C Hamano <gitster@pobox.com>
To: Sergey Organov <sorganov@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 2/2] diff-merges: introduce '-d' option
Date: Thu, 14 Sep 2023 15:17:31 -0700	[thread overview]
Message-ID: <xmqqled8h01w.fsf@gitster.g> (raw)
In-Reply-To: <87o7i7hler.fsf@osv.gnss.ru> (Sergey Organov's message of "Tue, 12 Sep 2023 10:59:24 +0300")

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.
>
> 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.

I am not sure about that.

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

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.

> 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.

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".