Thread (1 message) 1 message, 1 author, 2021-08-18

Re: [PATCH] Revert 'diff-merges: let "-m" imply "-p"'

From: Sergey Organov <hidden>
Date: 2021-08-18 08:56:20

Junio C Hamano [off-list ref] writes:
Sergey Organov [off-list ref] writes:
quoted
So, do I get it right that there is actually no reason to use "log
--first-parent -m" anymore, since the time the much older commit made
--first-parent imply -m?
It was necessary for scripts to say

    git log --first-parent -m "$@"

if they wanted to optionally show a first-parent diff for a merge
when the user of the script passes "-p" in "$@" (and not to show
patch if the user does not pass "-p").  

That changed with 9ab89a24 (log: enable "-m" automatically with
"--first-parent", 2020-07-29).
Yes, and since then it's no more needed to say "--first-parent -m" in
this case, as "--fist-parent" will do.
After that commit, it no longer was needed, but it still was correct
to expect that no patch will be shown with "--first-parent -m",
unless you give "-p" at the same time.  The original change that the
patch under discussion reverted broke that expectation.
We need to note that the "-m" implied by "--first-parent" is "if we
were to show some comparison, do so also for merge commits", not the
"if the user says '-m', it must mean that the user wants to see
comparison, period, so make it imply '-p'".  The latter is what was
reverted.
Yes, there is minor backward incompatibility indeed, and that was
expected. This could be seen from the patch in the same series that
fixes "git stash" by removing unneeded -m.

The fix for the scripts is as simple as removing -m from "--first-parent
-m". It's a one-time change.
quoted
If so, I'd object against this particular patch as the pros of patch
being reverted outweighs its cons, and the original patch never meant to
be entirely backward compatible in the first place, when it was
accepted.
I agree that we both (and if there were other reviewers, they too)
mistakenly thought that the change in behaviour was innocuous enough
when we queued the patch, but our mistakes were caught while the
topic was still cooking in 'next', and I have Jonathan to thank for
being extra careful.
So, what would be the procedure to get this change back, as this minor
backward incompatibility shouldn't be the show-stopper for the change
that otherwise is an improvement?

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