Thread (1 message) 1 message, 1 author, 2020-12-03

Re: [PATCH v1 05/27] revision: move diff merges functions to its own diff-merges.c

From: Junio C Hamano <hidden>
Date: 2020-12-03 00:59:36

Sergey Organov [off-list ref] writes:
quoted hunk
diff --git a/diff-merges.h b/diff-merges.h
new file mode 100644
index 000000000000..e0cca3d935d3
--- /dev/null
+++ b/diff-merges.h
@@ -0,0 +1,12 @@
+#ifndef DIFF_MERGES_H
+#define DIFF_MERGES_H
Describe what "diff-merges" module is about in a comment here.

This matters because in 07/27 the log message complains that the
first-parent traversal option is checked in the module but it is out
of place.  Without defining what the "module" is about, the readers
would have a hard time agreeing with the justification.

My guess is that this is about deciding how a merge commit is shown
in 'log -p' and 'show' output, comparing the commit with its
parent(s) in patch output.  With that explained, it may make sense
for 07/27 to complain that --first-parent that is primarily a
traversal option does also affect how a merge is shown (I do not
necessarily agree with the complaint, though)
quoted hunk
+
+struct rev_info;
+
+void init_diff_merge_revs(struct rev_info *revs);
+int parse_diff_merge_opts(struct rev_info *revs, const char **argv);
+void setup_diff_merges_revs(struct rev_info *revs);
+void rev_diff_merges_default_to_dense_combined(struct rev_info *revs);
+void rev_diff_merges_first_parent_defaults_to_enable(struct rev_info *revs);
+
+#endif
diff --git a/revision.c b/revision.c
index ce90c2991657..4bc14a08a624 100644
--- a/revision.c
+++ b/revision.c
@@ -5,6 +5,7 @@
 #include "tree.h"
 #include "commit.h"
 #include "diff.h"
+#include "diff-merges.h"
 #include "refs.h"
 #include "revision.h"
 #include "repository.h"
@@ -1805,8 +1806,6 @@ static int add_parents_only(struct rev_info *revs, const char *arg_, int flags,
 	return 1;
 }
 
-static void init_diff_merge_revs(struct rev_info *revs);
-
 void repo_init_revisions(struct repository *r,
 			 struct rev_info *revs,
 			 const char *prefix)
@@ -2155,75 +2154,6 @@ static void add_message_grep(struct rev_info *revs, const char *pattern)
 	add_grep(revs, pattern, GREP_PATTERN_BODY);
 }
 
-static void init_diff_merge_revs(struct rev_info *revs) {
-	revs->ignore_merges = -1;
-}
-
-static int parse_diff_merge_opts(struct rev_info *revs, const char **argv) {
-	int argcount;
-	const char *optarg;
-	const char *arg = argv[0];
-
-	if (!strcmp(arg, "-m")) {
-		/*
-		 * To "diff-index", "-m" means "match missing", and to the "log"
-		 * family of commands, it means "show full diff for merges". Set
-		 * both fields appropriately.
-		 */
-		revs->ignore_merges = 0;
-		revs->match_missing = 1;
-	} else if (!strcmp(arg, "-c")) {
-		revs->diff = 1;
-		revs->dense_combined_merges = 0;
-		revs->combine_merges = 1;
-	} else if (!strcmp(arg, "--cc")) {
-		revs->diff = 1;
-		revs->dense_combined_merges = 1;
-		revs->combine_merges = 1;
-	} else if (!strcmp(arg, "--no-diff-merges")) {
-		revs->ignore_merges = 1;
-	} else if (!strcmp(arg, "--combined-all-paths")) {
-		revs->diff = 1;
-		revs->combined_all_paths = 1;
-	} else if ((argcount = parse_long_opt("diff-merges", argv, &optarg))) {
-		if (!strcmp(optarg, "off")) {
-			revs->ignore_merges = 1;
-		} else {
-			die(_("unknown value for --diff-merges: %s"), optarg);
-		}
-	} else
-		return 0;
-
-	return 1;
-}
-
-static void setup_diff_merges_revs(struct rev_info *revs)
-{
-	if (revs->combine_merges && revs->ignore_merges < 0)
-		revs->ignore_merges = 0;
-	if (revs->ignore_merges < 0)
-		revs->ignore_merges = 1;
-	if (revs->combined_all_paths && !revs->combine_merges)
-		die("--combined-all-paths makes no sense without -c or --cc");
-}
-
-void rev_diff_merges_first_parent_defaults_to_enable(struct rev_info *revs) {
-	if (revs->first_parent_only && revs->ignore_merges < 0)
-		revs->ignore_merges = 0;
-}
-
-void rev_diff_merges_default_to_dense_combined(struct rev_info *revs) {
-	if (revs->ignore_merges < 0) {
-		/* There was no "-m" variant on the command line */
-		revs->ignore_merges = 0;
-		if (!revs->first_parent_only && !revs->combine_merges) {
-			/* No "--first-parent", "-c", or "--cc" */
-			revs->combine_merges = 1;
-			revs->dense_combined_merges = 1;
-		}
-	}
-}
-
 static int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
 			       int *unkc, const char **unkv,
 			       const struct setup_revision_opt* opt)
diff --git a/revision.h b/revision.h
index 3dd0229f4edc..f6bf860d19e5 100644
--- a/revision.h
+++ b/revision.h
@@ -456,7 +456,4 @@ int rewrite_parents(struct rev_info *revs,
  */
 struct commit_list *get_saved_parents(struct rev_info *revs, const struct commit *commit);
 
-void rev_diff_merges_default_to_dense_combined(struct rev_info *revs);
-void rev_diff_merges_first_parent_defaults_to_enable(struct rev_info *revs);
-
 #endif
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help