git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v1 0/9] diff-merges: let -m imply -p

From
Sergey Organov <sorganov@gmail.com>
Date
May 17, 2021, 15:58 UTC
Message-ID
<20210517155818.32224-1-sorganov@gmail.com>
In-Reply-To
<CAMMLpeR-W35Qq6a343ifrxJ=mwBc_VcXZtVrBYDpJTySNBroFw@mail.gmail.com>

Fix long standing inconsistency between -c/--cc that do imply -p, on one side, and -m that did not imply -p, on the other side.

Only the last patch is the actual functional change. The reset of patches are additional tests and refactorings that are not expected to alter existing behaviors.

After these patches
  git log -m

will start to produce diffs without need to provide -p as well, that improves both consistency and usability. It gets even more useful if one sets "log.diffMerges" configuration variable to "first-parent" to force -m produce usual diff with respect to first parent only.

These patches don't change behavior when specific diff format is explicitly provided on the command-line, so that commands like

  git log -m --raw
  git log -m --stat
are not affected.

Exact historical semantics of -m is still provided by --diff-merges=separate.

Updates in v1:
  * Stop parsing distinct diff-index options beforehand, as it could
    cause unexpected behaviors. Implement different strategy to avoid
    clash of diff-index "-m" and diff-merges "-m".
  * Added tests for "git log -m --raw" and "git log -m".
Sergey Organov (9):
  t4013: test that "-m" alone has no effect in "git log"
  t4013: test "git -m --raw"
  t4013: test "git -m --stat"
  t4013: test "git diff-index -m"
  diff-merges: move specific diff-index "-m" handling to diff-index
  git-svn: stop passing "-m" to "git rev-list"
  stash list: stop passing "-m" to "git list"
  diff-merges: rename "combined_imply_patch" to "merges_imply_patch"
  diff-merges: let "-m" imply "-p"
 Documentation/diff-options.txt    |  8 ++--
 builtin/diff-index.c              |  9 +++++
 builtin/stash.c                   |  2 +-
 diff-merges.c                     | 36 +++++++++--------
 diff-merges.h                     |  2 +
 perl/Git/SVN.pm                   |  2 +-
 revision.h                        |  2 +-
 t/t3903-stash.sh                  |  2 +-
 t/t4013-diff-various.sh           | 23 +++++++++++
 t/t4013/diff.log_-m_--raw_master  | 61 ++++++++++++++++++++++++++++
 t/t4013/diff.log_-m_--stat_master | 66 +++++++++++++++++++++++++++++++
 11 files changed, 188 insertions(+), 25 deletions(-)
 create mode 100644 t/t4013/diff.log_-m_--raw_master
 create mode 100644 t/t4013/diff.log_-m_--stat_master
Interdiff against v0:
diff --git a/builtin/diff-index.c b/builtin/diff-index.c
index 28bc51d0d8f4..cf09559e422d 100644
--- a/builtin/diff-index.c
+++ b/builtin/diff-index.c
@@ -2,6 +2,7 @@
 #include "cache.h"
 #include "config.h"
 #include "diff.h"
+#include "diff-merges.h"
 #include "commit.h"
 #include "revision.h"
 #include "builtin.h"
@@ -12,37 +13,11 @@ static const char diff_cache_usage[] =
 "[<common-diff-options>] <tree-ish> [<path>...]"
 COMMON_DIFF_OPTIONS_HELP;
 
-static int parse_distinct_options(int argc, const char **argv,
-				  struct rev_info *revs, unsigned int *options)
-{
-	int i, left;
-
-	for (i = left = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		int leave = 0;
-
-		if (!strcmp(arg, "--cached"))
-			*options |= DIFF_INDEX_CACHED;
-		else if (!strcmp(arg, "--merge-base"))
-			*options |= DIFF_INDEX_MERGE_BASE;
-		else if (!strcmp(arg, "-m"))
-			revs->match_missing = 1;
-		else
-			leave = 1;
-
-		if (leave)
-			argv[left++] = arg;
-	}
-
-	argv[left] = NULL;
-
-	return left;
-}
-
 int cmd_diff_index(int argc, const char **argv, const char *prefix)
 {
 	struct rev_info rev;
 	unsigned int option = 0;
+	int i;
 	int result;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -54,15 +29,24 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
 	prefix = precompose_argv_prefix(argc, argv, prefix);
 
 	/*
-	 * It's essential to parse our distinct options before calling
-	 * setup_revisions(), for the latter not to see "-m".
+	 * We need no diff for merges options, and we need to avoid conflict
+	 * with our own meaning of "-m".
 	 */
-	argc = parse_distinct_options(argc, argv, &rev, &option);
+	diff_merges_suppress_options_parsing();
+
 	argc = setup_revisions(argc, argv, &rev, NULL);
+	for (i = 1; i < argc; i++) {
+		const char *arg = argv[i];
 
-	if (argc > 1)
-		usage(diff_cache_usage);
-
+		if (!strcmp(arg, "--cached"))
+			option |= DIFF_INDEX_CACHED;
+		else if (!strcmp(arg, "--merge-base"))
+			option |= DIFF_INDEX_MERGE_BASE;
+		else if (!strcmp(arg, "-m"))
+			rev.match_missing = 1;
+		else
+			usage(diff_cache_usage);
+	}
 	if (!rev.diffopt.output_format)
 		rev.diffopt.output_format = DIFF_FORMAT_RAW;
 
diff --git a/diff-merges.c b/diff-merges.c
index a827482a97ff..0dfcaa1b11b0 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -6,6 +6,7 @@ typedef void (*diff_merges_setup_func_t)(struct rev_info *);
 static void set_separate(struct rev_info *revs);
 
 static diff_merges_setup_func_t set_to_default = set_separate;
+static int suppress_parsing;
 
 static void suppress(struct rev_info *revs)
 {
@@ -90,12 +91,20 @@ int diff_merges_config(const char *value)
 	return 0;
 }
 
+void diff_merges_suppress_options_parsing(void)
+{
+	suppress_parsing = 1;
+}
+
 int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 {
 	int argcount = 1;
 	const char *optarg;
 	const char *arg = argv[0];
 
+	if (suppress_parsing)
+		return 0;
+
 	if (!strcmp(arg, "-m")) {
 		set_to_default(revs);
 		revs->merges_imply_patch = 1;
@@ -145,6 +154,9 @@ void diff_merges_set_dense_combined_if_unset(struct rev_info *revs)
 
 void diff_merges_setup_revs(struct rev_info *revs)
 {
+	if (suppress_parsing)
+		return;
+
 	if (revs->combine_merges == 0)
 		revs->dense_combined_merges = 0;
 	if (revs->separate_merges == 0)
diff --git a/diff-merges.h b/diff-merges.h
index 09d9a6c9a4fb..b5d57f6563e3 100644
--- a/diff-merges.h
+++ b/diff-merges.h
@@ -11,6 +11,8 @@ struct rev_info;
 
 int diff_merges_config(const char *value);
 
+void diff_merges_suppress_options_parsing(void);
+
 int diff_merges_parse_opts(struct rev_info *revs, const char **argv);
 
 void diff_merges_suppress(struct rev_info *revs);
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index bd16d4885cfd..15ca3c75bff7 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -337,6 +337,8 @@ log -m -p --first-parent master
 log -m -p master
 log --cc -m -p master
 log -c -m -p master
+log -m --raw master
+log -m --stat master
 log -SF master
 log -S F master
 log -SF -p master
@@ -452,6 +454,14 @@ 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 &&
+	process_diffs result >expected &&
+	git log -m >result &&
+	process_diffs result >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
 	git log -p --diff-merges=separate master >result &&
 	process_diffs result >expected &&
diff --git a/t/t4013/diff.log_-m_--raw_master b/t/t4013/diff.log_-m_--raw_master
new file mode 100644
index 000000000000..cd2ecc462832
--- /dev/null
+++ b/t/t4013/diff.log_-m_--raw_master
@@ -0,0 +1,61 @@
+$ git log -m --raw master
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (from 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+:100644 100644 cead32e... 992913c... M	dir/sub
+:100644 100644 b414108... 10a8a9f... M	file0
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (from c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+:100644 100644 7289e35... 992913c... M	dir/sub
+:100644 100644 f4615da... 10a8a9f... M	file0
+:000000 100644 0000000... b1e6722... A	file1
+:100644 000000 01e79c3... 0000000... D	file2
+:100644 000000 7289e35... 0000000... D	file3
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+:100644 100644 35d242b... 7289e35... M	dir/sub
+:100644 100644 01e79c3... f4615da... M	file0
+:000000 100644 0000000... 7289e35... A	file3
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+:100644 100644 8422d40... cead32e... M	dir/sub
+:000000 100644 0000000... b1e6722... A	file1
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+:100644 100644 35d242b... 8422d40... M	dir/sub
+:100644 100644 01e79c3... b414108... M	file0
+:100644 000000 01e79c3... 0000000... D	file2
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$
diff --git a/t/t4013/diff.log_-m_--stat_master b/t/t4013/diff.log_-m_--stat_master
new file mode 100644
index 000000000000..c7db084fd905
--- /dev/null
+++ b/t/t4013/diff.log_-m_--stat_master
@@ -0,0 +1,66 @@
+$ git log -m --stat master
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (from 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+ dir/sub | 2 ++
+ file0   | 3 +++
+ 2 files changed, 5 insertions(+)
+
+commit 59d314ad6f356dd08601a4cd5e530381da3e3c64 (from c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a)
+Merge: 9a6d494 c7a2ab9
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:04:00 2006 +0000
+
+    Merge branch 'side'
+
+ dir/sub | 4 ++++
+ file0   | 3 +++
+ file1   | 3 +++
+ file2   | 3 ---
+ file3   | 4 ----
+ 5 files changed, 10 insertions(+), 7 deletions(-)
+
+commit c7a2ab9e8eac7b117442a607d5a9b3950ae34d5a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:03:00 2006 +0000
+
+    Side
+
+ dir/sub | 2 ++
+ file0   | 3 +++
+ file3   | 4 ++++
+ 3 files changed, 9 insertions(+)
+
+commit 9a6d4949b6b76956d9d5e26f2791ec2ceff5fdc0
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:02:00 2006 +0000
+
+    Third
+
+ dir/sub | 2 ++
+ file1   | 3 +++
+ 2 files changed, 5 insertions(+)
+
+commit 1bde4ae5f36c8d9abe3a0fce0c6aab3c4a12fe44
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:01:00 2006 +0000
+
+    Second
+    
+    This is the second commit.
+
+ dir/sub | 2 ++
+ file0   | 3 +++
+ file2   | 3 ---
+ 3 files changed, 5 insertions(+), 3 deletions(-)
+
+commit 444ac553ac7612cc88969031b02b3767fb8a353a
+Author: A U Thor <author@example.com>
+Date:   Mon Jun 26 00:00:00 2006 +0000
+
+    Initial
+$

-- 
2.25.1
Previous: Sergey OrganovNext: Sergey Organov
Message 66 of 129 in “Why doesn't `git log -m` imply `-p`?”
  1. Alex HenrieApr 29, 2021
  2. Junio C HamanoApr 29, 2021
  3. Sergey OrganovApr 29, 2021
  4. Alex HenrieApr 29, 2021
  5. Sergey OrganovApr 29, 2021
  6. Alex HenrieApr 29, 2021
  7. Sergey OrganovApr 29, 2021
  8. Felipe ContrerasMay 4, 2021
  9. Sergey OrganovMay 4, 2021
  10. Junio C HamanoApr 29, 2021
  11. Junio C HamanoApr 30, 2021
  12. Sergey OrganovApr 30, 2021
  13. Junio C HamanoMay 1, 2021
  14. Sergey OrganovMay 3, 2021
  15. Junio C HamanoMay 4, 2021
  16. Sergey OrganovMay 4, 2021
  17. Junio C HamanoMay 4, 2021
  18. Sergey OrganovMay 4, 2021
  19. Junio C HamanoMay 5, 2021
  20. Sergey OrganovMay 5, 2021
  21. Junio C HamanoMay 6, 2021
  22. Sergey OrganovMay 6, 2021
  23. Junio C HamanoMay 6, 2021
  24. Sergey OrganovMay 6, 2021
  25. Alex HenrieMay 7, 2021
  26. Sergey OrganovMay 10, 2021
  27. Alex HenrieMay 10, 2021
  28. 0/6 diff-merges: let -m imply -pSergey Organov, May 10, 2021
  29. 1/6 t4013: add test for "git diff-index -m"Sergey Organov, May 10, 2021
  30. 2/6 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 10, 2021
  31. Junio C HamanoMay 11, 2021
  32. Junio C HamanoMay 11, 2021
  33. Junio C HamanoMay 11, 2021
  34. Sergey OrganovMay 11, 2021
  35. 3/6 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 10, 2021
  36. 4/6 stash list: stop passing "-m" to "git list"Sergey Organov, May 10, 2021
  37. 5/6 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 10, 2021
  38. 6/6 diff-merges: let -m imply -pSergey Organov, May 10, 2021
  39. Junio C HamanoMay 11, 2021
  40. Junio C HamanoMay 11, 2021
  41. Sergey OrganovMay 11, 2021
  42. Alex HenrieMay 11, 2021
  43. Sergey OrganovMay 11, 2021
  44. Alex HenrieMay 11, 2021
  45. Sergey OrganovMay 11, 2021
  46. Felipe ContrerasMay 12, 2021
  47. Elijah NewrenMay 11, 2021
  48. Sergey OrganovMay 11, 2021
  49. Elijah NewrenMay 11, 2021
  50. Sergey OrganovMay 11, 2021
  51. Junio C HamanoMay 11, 2021
  52. Sergey OrganovMay 11, 2021
  53. Junio C HamanoMay 11, 2021
  54. Jonathan NiederMay 19, 2021
  55. Sergey OrganovMay 20, 2021
  56. Felipe ContrerasMay 21, 2021
  57. Sergey OrganovMay 11, 2021
  58. Sergey OrganovMay 17, 2021
  59. Sergey OrganovMay 11, 2021
  60. Jonathan NiederMay 19, 2021
  61. Sergey OrganovMay 19, 2021
  62. Junio C HamanoMay 19, 2021
  63. Sergey OrganovMay 20, 2021
  64. Jonathan NiederMay 20, 2021
  65. Sergey OrganovMay 20, 2021
  66. 0/9 diff-merges: let -m imply -pSergey Organov, May 17, 2021
  67. 1/9 t4013: test that "-m" alone has no effect in "git log"Sergey Organov, May 17, 2021
  68. 4/9 t4013: test "git diff-index -m"Sergey Organov, May 17, 2021
  69. 3/9 t4013: test "git -m --stat"Sergey Organov, May 17, 2021
  70. 2/9 t4013: test "git -m --raw"Sergey Organov, May 17, 2021
  71. Bagas SanjayaMay 18, 2021
  72. Sergey OrganovMay 18, 2021
  73. 6/9 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 17, 2021
  74. 8/9 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 17, 2021
  75. 5/9 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 17, 2021
  76. Junio C HamanoMay 17, 2021
  77. Sergey OrganovMay 17, 2021
  78. Junio C HamanoMay 17, 2021
  79. Sergey OrganovMay 17, 2021
  80. 9/9 diff-merges: let "-m" imply "-p"Sergey Organov, May 17, 2021
  81. 7/9 stash list: stop passing "-m" to "git list"Sergey Organov, May 17, 2021
  82. Junio C HamanoMay 17, 2021
  83. Sergey OrganovMay 17, 2021
  84. Bagas SanjayaMay 18, 2021
  85. Sergey OrganovMay 18, 2021
  86. Sergey OrganovMay 18, 2021
  87. Junio C HamanoMay 18, 2021
  88. Sergey OrganovMay 18, 2021
  89. 0/9 diff-merges: let -m imply -pSergey Organov, May 19, 2021
  90. 1/9 t4013: test that "-m" alone has no effect in "git log"Sergey Organov, May 19, 2021
  91. 2/9 t4013: test "git log -m --raw"Sergey Organov, May 19, 2021
  92. 3/9 t4013: test "git log -m --stat"Sergey Organov, May 19, 2021
  93. 4/9 t4013: test "git diff-index -m"Sergey Organov, May 19, 2021
  94. 5/9 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 19, 2021
  95. 6/9 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 19, 2021
  96. 7/9 stash list: stop passing "-m" to "git log"Sergey Organov, May 19, 2021
  97. 8/9 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 19, 2021
  98. 9/9 diff-merges: let "-m" imply "-p"Sergey Organov, May 19, 2021
  99. 00/10 diff-merges: let -m imply -pSergey Organov, May 20, 2021
  100. 02/10 t4013: test "git log -m --raw"Sergey Organov, May 20, 2021
  101. 01/10 t4013: test that "-m" alone has no effect in "git log"Sergey Organov, May 20, 2021
  102. 03/10 t4013: test "git log -m --stat"Sergey Organov, May 20, 2021
  103. 04/10 t4013: test "git diff-tree -m"Sergey Organov, May 20, 2021
  104. 05/10 t4013: test "git diff-index -m"Sergey Organov, May 20, 2021
  105. 06/10 diff-merges: move specific diff-index "-m" handling to diff-indexSergey Organov, May 20, 2021
  106. 07/10 git-svn: stop passing "-m" to "git rev-list"Sergey Organov, May 20, 2021
  107. 08/10 stash list: stop passing "-m" to "git log"Sergey Organov, May 20, 2021
  108. 09/10 diff-merges: rename "combined_imply_patch" to "merges_imply_patch"Sergey Organov, May 20, 2021
  109. 10/10 diff-merges: let "-m" imply "-p"Sergey Organov, May 20, 2021
  110. Jonathan NiederAug 5, 2021
  111. Revert 'diff-merges: let "-m" imply "-p"'Jonathan Nieder, Aug 6, 2021
  112. Junio C HamanoAug 6, 2021
  113. Junio C HamanoAug 6, 2021
  114. Jonathan NiederAug 6, 2021
  115. Junio C HamanoAug 8, 2021
  116. Sergey OrganovAug 17, 2021
  117. Junio C HamanoAug 17, 2021
  118. Sergey OrganovAug 18, 2021
  119. Junio C HamanoAug 19, 2021
  120. Junio C HamanoAug 19, 2021
  121. Sergey OrganovAug 20, 2021
  122. Jonathan NiederAug 7, 2021
  123. Johannes SixtAug 7, 2021
  124. Jonathan NiederAug 7, 2021
  125. Junio C HamanoAug 7, 2021
  126. Jonathan NiederAug 7, 2021
  127. Junio C HamanoAug 8, 2021
  128. Sergey OrganovAug 17, 2021
  129. Sergey OrganovAug 16, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.