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

From: Sergey Organov <sorganov@gmail.com>
Date: Sep 16, 2023, 18:37 UTC
Message-ID: <87ttrudkw9.fsf@osv.gnss.ru>
In-Reply-To: <xmqqzg1nfixw.fsf@gitster.g>

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

Show 10 quoted lines

> Sergey Organov <sorganov@gmail.com> writes:
>
>> 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
>
> Yeah, but that is not what I meant.  The above argues for why
> "--diff-merges=first-parent" should exist independently from the
> "--first-parent" traversal *and* display option.  I am not saying
> it should not exist.

I was not assuming you were saying this, as it has been discussed and agreed upon when --diff-merges=first-parent was introduced, though I think I now see your point more clearly.

>
> But I view that the desire to look at any commits and its changes on
> the "side" branch at all *is* at odds with the wish to look at
> first-parent change for merge commits.

I think I do now understand what you mean, yet I have alternative view on the issue.

> Once you decide to look at first-parent change for a merge commit,
> then every change you see for each commit on the "side" branch,
> whether it is shown as first-parent diff or N pairwise diffs, is what
> you have already seen in the change in the merge commit,

Actually, this happens to be exactly one of intended use-cases for "-d". It's useful to see how some change introduced by the merge looked in the context of the original commit, or to figure where the change came from.

> because "git log" goes newer to older, and the commits on the side
> branches appear after the merge that brings them to the mainline.

The exact order is orthogonal to the issue at hands, I think.

Show 6 quoted lines

> Making "log -d" mean "log --diff-merges=first-parent --patch" lets
> that less useful combination ("show first-parent patches but
> traverse side branches as well") squat on the short and sweet "-d"
> that could be used for more useful "log --first-parent --patch",
> which would also be more common and intuitive to users, and that is
> what I suspect will become problematic in the longer run.

Sorry, "-d ≡ --first-parent --patch" you suggest contradicts my view on the whole scheme of things, for several reasons:

* I still find it problematic if -d, intended to fit nicely among --cc,
-c, -d, -m, -p, --remerge-diff options, suddenly implies --first-parent.
This would bring yet another inconsistency, and I don't want to be the
one who introduced it.

* In its current state -d conveniently means: "gimme simple diff output
for everything", where --first-parent you suggest doesn't fit at all.

* Current -d implementation is semantically as close to -p as possible,
tweaking exactly one thing compared to -p: the format of output for
merge commits, so is simpler than what you suggest from all angles, as
--first-parent tweaks more than one thing.

* To me what you argue for looks mostly like a desire to have a
short-cut for "--first-parent --patch", and my patch in question does
not seem to contradict this desire, as it'd be very surprising if
somebody came up with the name "-d" for such a short-cut. Definitely not
me.

* Finally, if -d becomes "--patch --first-parent", how do I get back
useful "--patch --diff-merges=first-parent" part of it, provided
--first-parent is unreversable? And even if it were reversable, then

   git log -d --no-first-parent =
   git log --patch --first-parent --no-first-parent =
   git log --patch

is definitely not what is needed, nor frequent demand to revert implied things indicates optimal design. Compare this to

   git log -d --first-parent

that current -d provides for you to get what you need, and that unambiguously reads: "gimme *d*iff for all commits while following *first parent* through the history" (while, unlike, -p not requiring --first-parent to implicitly tweak diff for merges output).

Overall, after considering your concern, I'd still prefer to leave "-d" semantics as implemented, consistent with the rest of similar options, and let somebody else define more shortcuts for their frequent use-cases if they feel like it.

Thanks, -- Sergey Organov

P.S. I also figure that maybe our divergence comes from the fact that I consider merge commits to be primarily commits (introducing particular set of changes, and then having reference to the source of the changes), whereas you consider them primarily merges (joining two histories, and then maybe some artificial changes that make merges "evil"). That's why we often end up agreeing to disagree, as both these points of view seem pretty valid.