]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Revert 'diff-merges: let "-m" imply "-p"'
authorJonathan Nieder <jrnieder@gmail.com>
Fri, 6 Aug 2021 01:45:23 +0000 (18:45 -0700)
committerJunio C Hamano <gitster@pobox.com>
Mon, 9 Aug 2021 20:52:01 +0000 (13:52 -0700)
This reverts commit f5bfcc823ba242a46e20fb6f71c9fbf7ebb222fe, which
made "git log -m" imply "--patch" by default.  The logic was that
"-m", which makes diff generation for merges perform a diff against
each parent, has no use unless I am viewing the diff, so we could save
the user some typing by turning on display of the resulting diff
automatically.  That wasn't expected to adversely affect scripts
because scripts would either be using a command like "git diff-tree"
that already emits diffs by default or would be combining -m with a
diff generation option such as --name-status.  By saving typing for
interactive use without adversely affecting scripts in the wild, it
would be a pure improvement.

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

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.

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.  For
example, until 1e20a407fe2 (stash list: stop passing "-m" to "git
log", 2021-05-21), "git stash list" did this.

As a result, we can't safely change "-m" to imply "-p" without fear of
breaking such scripts.  Restore the previous behavior.

Noticed because Rust's src/bootstrap/bootstrap.py made use of this
same construct: https://github.com/rust-lang/rust/pull/87513.  That
script has been updated to omit the unnecessary "-m" option, but we
can expect other scripts in the wild to have similar expectations.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/diff-options.txt
diff-merges.c
t/t4013-diff-various.sh

index 2825783049c51019bab9d5d20507705d331684cc..6d968b9012dcbe56b616f9edd66b1e869d09e2ae 100644 (file)
@@ -49,9 +49,10 @@ ifdef::git-log[]
 --diff-merges=m:::
 -m:::
        This option makes diff output for merge commits to be shown in
-       the default format. The default format could be changed using
+       the default format. `-m` will produce the output only if `-p`
+       is given as well. The default format could be changed using
        `log.diffMerges` configuration parameter, which default value
-       is `separate`. `-m` implies `-p`.
+       is `separate`.
 +
 --diff-merges=first-parent:::
 --diff-merges=1:::
@@ -61,8 +62,7 @@ ifdef::git-log[]
 --diff-merges=separate:::
        This makes merge commits show the full diff with respect to
        each of the parents. Separate log entry and diff is generated
-       for each parent. This is the format that `-m` produced
-       historically.
+       for each parent.
 +
 --diff-merges=combined:::
 --diff-merges=c:::
index 0dfcaa1b11b06c456b9b2349859255602df647e8..d897fd8a293319b34172c5c816d28ffb37ad7ce9 100644 (file)
@@ -107,7 +107,6 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 
        if (!strcmp(arg, "-m")) {
                set_to_default(revs);
-               revs->merges_imply_patch = 1;
        } else if (!strcmp(arg, "-c")) {
                set_combined(revs);
                revs->merges_imply_patch = 1;
index 7fadc985cccd0558be4e4114434f143605e710a4..e561a8e48521a022ea90891f96d1054cf1ecc68d 100755 (executable)
@@ -455,8 +455,8 @@ diff-tree --stat --compact-summary initial mode
 diff-tree -R --stat --compact-summary initial mode
 EOF
 
-test_expect_success 'log -m matches log -m -p' '
-       git log -m -p master >result &&
+test_expect_success 'log -m matches pure log' '
+       git log master >result &&
        process_diffs result >expected &&
        git log -m >result &&
        process_diffs result >actual &&