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

From: Sergey Organov <sorganov@gmail.com>
Date: Sep 14, 2023, 23:56 UTC
Message-ID: <87y1h8wbpo.fsf@osv.gnss.ru>
In-Reply-To: <xmqqled8h01w.fsf@gitster.g>

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

Show 28 quoted lines

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

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.

Show 10 quoted lines

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

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.

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

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