diff: consolidate diff algorithm option parsing

A subsequent commit will need the ability to tell if the diff algorithm
was set through the command line through setting a new member of
diff_options. While this logic can be added to the
diff_opt_diff_algorithm() callback, the `--minimal` and `--histogram`
options are handled via OPT_BIT without a callback.

Remedy this by consolidating the options parsing logic for --minimal and
--histogram into one callback. This way we can modify `diff_options` in
that function.

As an additional refactor, the logic that sets the diff algorithm in
diff_opt_diff_algorithm() can be refactored into a helper that will
allow multiple callsites to set the diff algorithm.

Signed-off-by: John Cai <johncai86@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 1054a4b7329d0b70ed62311a728b5257df3a8143..3b97f5eca2f4cc7886929ff2bf6106bcb1663f14 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -3437,6 +3437,22 @@ static int diff_filepair_is_phoney(struct diff_filespec *one,
        return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);
 }
 
+static int set_diff_algorithm(struct diff_options *opts,
+                             const char *alg)
+{
+       long value = parse_algorithm_value(alg);
+
+       if (value < 0)
+               return -1;
+
+       /* clear out previous settings */
+       DIFF_XDL_CLR(opts, NEED_MINIMAL);
+       opts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
+       opts->xdl_opts |= value;
+
+       return 0;
+}
+
 static void builtin_diff(const char *name_a,
                         const char *name_b,
                         struct diff_filespec *one,
@@ -5117,17 +5133,28 @@ static int diff_opt_diff_algorithm(const struct option *opt,
                                   const char *arg, int unset)
 {
        struct diff_options *options = opt->value;
-       long value = parse_algorithm_value(arg);
 
        BUG_ON_OPT_NEG(unset);
-       if (value < 0)
+
+       if (set_diff_algorithm(options, arg))
                return error(_("option diff-algorithm accepts \"myers\", "
                               "\"minimal\", \"patience\" and \"histogram\""));
 
-       /* clear out previous settings */
-       DIFF_XDL_CLR(options, NEED_MINIMAL);
-       options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
-       options->xdl_opts |= value;
+       return 0;
+}
+
+static int diff_opt_diff_algorithm_no_arg(const struct option *opt,
+                                  const char *arg, int unset)
+{
+       struct diff_options *options = opt->value;
+
+       BUG_ON_OPT_NEG(unset);
+       BUG_ON_OPT_ARG(arg);
+
+       if (set_diff_algorithm(options, opt->long_name))
+               BUG("available diff algorithms include \"myers\", "
+                              "\"minimal\", \"patience\" and \"histogram\"");
+
        return 0;
 }
 
@@ -5260,7 +5287,6 @@ static int diff_opt_patience(const struct option *opt,
 
        BUG_ON_OPT_NEG(unset);
        BUG_ON_OPT_ARG(arg);
-       options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
        /*
         * Both --patience and --anchored use PATIENCE_DIFF
         * internally, so remove any anchors previously
@@ -5269,7 +5295,8 @@ static int diff_opt_patience(const struct option *opt,
        for (i = 0; i < options->anchors_nr; i++)
                free(options->anchors[i]);
        options->anchors_nr = 0;
-       return 0;
+
+       return set_diff_algorithm(options, "patience");
 }
 
 static int diff_opt_ignore_regex(const struct option *opt,
@@ -5571,9 +5598,10 @@ static void prep_parse_options(struct diff_options *options)
                            N_("prevent rename/copy detection if the number of rename/copy targets exceeds given limit")),
 
                OPT_GROUP(N_("Diff algorithm options")),
-               OPT_BIT(0, "minimal", &options->xdl_opts,
-                       N_("produce the smallest possible diff"),
-                       XDF_NEED_MINIMAL),
+               OPT_CALLBACK_F(0, "minimal", options, NULL,
+                              N_("produce the smallest possible diff"),
+                              PARSE_OPT_NONEG | PARSE_OPT_NOARG,
+                              diff_opt_diff_algorithm_no_arg),
                OPT_BIT_F('w', "ignore-all-space", &options->xdl_opts,
                          N_("ignore whitespace when comparing lines"),
                          XDF_IGNORE_WHITESPACE, PARSE_OPT_NONEG),
@@ -5599,9 +5627,10 @@ static void prep_parse_options(struct diff_options *options)
                               N_("generate diff using the \"patience diff\" algorithm"),
                               PARSE_OPT_NONEG | PARSE_OPT_NOARG,
                               diff_opt_patience),
-               OPT_BITOP(0, "histogram", &options->xdl_opts,
-                         N_("generate diff using the \"histogram diff\" algorithm"),
-                         XDF_HISTOGRAM_DIFF, XDF_DIFF_ALGORITHM_MASK),
+               OPT_CALLBACK_F(0, "histogram", options, NULL,
+                              N_("generate diff using the \"histogram diff\" algorithm"),
+                              PARSE_OPT_NONEG | PARSE_OPT_NOARG,
+                              diff_opt_diff_algorithm_no_arg),
                OPT_CALLBACK_F(0, "diff-algorithm", options, N_("<algorithm>"),
                               N_("choose a diff algorithm"),
                               PARSE_OPT_NONEG, diff_opt_diff_algorithm),