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

From: Junio C Hamano <gitster@pobox.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: "Sergey Organov" <sorganov@gmail.com>,
	"Jeff King" <peff@peff.net>,
	"Philip Oakley" <philipoakley@iee.email>,
	"Elijah Newren" <newren@gmail.com>,
	"Felipe Contreras" <felipe.contreras@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Alex Henrie" <alexhenrie24@gmail.com>,
	"huang guanlong" <gl041188@gmail.com>,
	git@vger.kernel.org, "Hudson Ayers" <hudsonayers@google.com>,
	"Taylor Yu" <tlyu@mit.edu>, "Joshua Nelson" <jyn514@gmail.com>
Subject: Re: [PATCH] Revert 'diff-merges: let "-m" imply "-p"'
Date: Fri, 06 Aug 2021 10:55:23 -0700	[thread overview]
Message-ID: <xmqqczqqihkk.fsf@gitster.g> (raw)
In-Reply-To: <xmqqh7g2ij5q.fsf@gitster.g> (Junio C. Hamano's message of "Fri, 06 Aug 2021 10:21:05 -0700")

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

>> Similarly, a script author could include
>>
>> 	git log -m --first-parent --format=%H -- README
>>
>> to filter the first-parent history for commits that modified README.
>> The -m is a no-op but it reflects the script author's intent.
>
> So the expectation is with "-m" we'd give single parent commits on
> the fp chain, and merges from side branches that change README, in
> addition to merges from side branches that was forked way before the
> README was updated on the trunk (hence had ancient README but the
> merge kept the version from the trunk)?
>
>> For
>> example, until 1e20a407fe2 (stash list: stop passing "-m" to "git
>> log", 2021-05-21), "git stash list" did this.
>
> This is not a example that supports your conclusion, though.  The
> reason why 288c67ca (stash: default listing to working-tree diff,
> 2014-08-06) added "-m" on the command line to make it:
>
>   git log --format="%gd: %gs" -g --first-parent -m "$@" $ref_stash --
>
> is to prepare for the users who may pass "-p" as part of the "$@";
> they wil get no patches out of these merge commits that represent
> stash entries otherwise, and they'd have to pass "-m -p" instead,
> without the change.
>
>> As a result, we can't safely change "-m" to imply "-p" without fear of
>> breaking such scripts.  Restore the previous behavior.
>
> So the above is *not* an example of a script that would have been
> broken with this change.

Sorry, I have to take 70% of the above back.  While it is *not* an
example that shows an author's intent that changes from not just
trunk but all side branches to README are to be shown, the original
left after 288c67ca (stash: default listing to working-tree diff,
2014-08-06) would have been broken by the change you are proposing
to revert.  It used to be just "if you give -p we'll show a patch"
but if we make "-m" to mean "-m -p", it does get broken.

Which is BAD.

I am inclined to take the revert, but I do not think reverting it
alone will break more things than it fixes.

For example, 1e20a407 (stash list: stop passing "-m" to "git log",
2021-05-21) that dropped "-m" must be reverted as well, no?