Thread (1 message) 1 message, 1 author, 2023-09-14

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

From: Sergey Organov <hidden>
Date: 2023-09-14 23:56:40

Junio C Hamano [off-list ref] writes:
Sergey Organov [off-list ref] writes:
quoted
quoted
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.
quoted
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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help