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

From: Sergey Organov <[email-masked]>
Date: Wed, 18 Aug 2021 11:56:14 +0300

Junio C Hamano <[email-masked]> writes:

> Sergey Organov <[email-masked]> writes:
>
>> 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.

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