Thread (4 messages) flat view 4 messages, 3 authors, 2021-08-07

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

From: Junio C Hamano <hidden>
Date: 2021-08-06 17:21:11

Jonathan Nieder [off-list ref] writes:
The problem is that although diff generation options are only relevant
for the displayed diff, a script author can imagine them affecting
path limiting.  For example, I might run
I am somewhat puzzled.  What does "can imagine" exactly mean and
justify this change?  A script author may imagine "git cat-file" can
be expected to meow, but the command actually does not meow and end
up disappointing the author, but that wouldn't justify a rename of
"cat-file" to something else.
	git log -w --format=%H -- README

hoping to list commits that edited README, excluding whitespace-only
changes.  In fact, a whitespace-only change is not TREESAME so the use
of -w here has no effect (since we don't apply these diff generation
flags to the diff_options struct rev_info::pruning used for this
purpose), but the documentation suggests that it should work

	Suppose you specified foo as the <paths>. We shall call
	commits that modify foo !TREESAME, and the rest TREESAME. (In
	a diff filtered for foo, they look different and equal,
	respectively.)

and a script author who has not tested whitespace-only changes
wouldn't notice.
It would need to be corrected by a bugfix of either TREESAME
computation, or a documentation fix, I would think.  I fail to see
the similarity you perceive to the "-m" issue at hand, though.
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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help