Re: [PATCH v1 05/27] revision: move diff merges functions to its own diff-merges.c
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Dec 3, 2020, 00:58 UTC
- Message-ID
- <xmqqa6uvk8mv.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20201108213838.4880-6-sorganov@gmail.com>
Sergey Organov <sorganov@gmail.com> writes:
Show 8 quoted lines
> 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)
Show 119 quoted lines
> +
> +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