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>

John Cai committed Feb 20, 2023 at 21:04 UTC 11e95e16e8ef70c6c757d50a99f9b310e6c795f4
1 file changed +43 -14
diff.c
+43 -14
@@ -3437,6 +3437,22 @@ static int diff_filepair_is_phoney(struct diff_filespec *one,
3437 return !DIFF_FILE_VALID(one) && !DIFF_FILE_VALID(two);
3438 }
3439
3440 +static int set_diff_algorithm(struct diff_options *opts,
3441 + const char *alg)
3442 +{
3443 + long value = parse_algorithm_value(alg);
3444 +
3445 + if (value < 0)
3446 + return -1;
3447 +
3448 + /* clear out previous settings */
3449 + DIFF_XDL_CLR(opts, NEED_MINIMAL);
3450 + opts->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
3451 + opts->xdl_opts |= value;
3452 +
3453 + return 0;
3454 +}
3455 +
3456 static void builtin_diff(const char *name_a,
3457 const char *name_b,
3458 struct diff_filespec *one,
@@ -5117,17 +5133,28 @@ static int diff_opt_diff_algorithm(const struct option *opt,
5133 const char *arg, int unset)
5134 {
5135 struct diff_options *options = opt->value;
5120 - long value = parse_algorithm_value(arg);
5136
5137 BUG_ON_OPT_NEG(unset);
5123 - if (value < 0)
5138 +
5139 + if (set_diff_algorithm(options, arg))
5140 return error(_("option diff-algorithm accepts \"myers\", "
5141 "\"minimal\", \"patience\" and \"histogram\""));
5142
5127 - /* clear out previous settings */
5128 - DIFF_XDL_CLR(options, NEED_MINIMAL);
5129 - options->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;
5130 - options->xdl_opts |= value;
5143 + return 0;
5144 +}
5145 +
5146 +static int diff_opt_diff_algorithm_no_arg(const struct option *opt,
5147 + const char *arg, int unset)
5148 +{
5149 + struct diff_options *options = opt->value;
5150 +
5151 + BUG_ON_OPT_NEG(unset);
5152 + BUG_ON_OPT_ARG(arg);
5153 +
5154 + if (set_diff_algorithm(options, opt->long_name))
5155 + BUG("available diff algorithms include \"myers\", "
5156 + "\"minimal\", \"patience\" and \"histogram\"");
5157 +
5158 return 0;
5159 }
5160
@@ -5260,7 +5287,6 @@ static int diff_opt_patience(const struct option *opt,
5287
5288 BUG_ON_OPT_NEG(unset);
5289 BUG_ON_OPT_ARG(arg);
5263 - options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
5290 /*
5291 * Both --patience and --anchored use PATIENCE_DIFF
5292 * internally, so remove any anchors previously
@@ -5269,7 +5295,8 @@ static int diff_opt_patience(const struct option *opt,
5295 for (i = 0; i < options->anchors_nr; i++)
5296 free(options->anchors[i]);
5297 options->anchors_nr = 0;
5272 - return 0;
5298 +
5299 + return set_diff_algorithm(options, "patience");
5300 }
5301
5302 static int diff_opt_ignore_regex(const struct option *opt,
@@ -5571,9 +5598,10 @@ static void prep_parse_options(struct diff_options *options)
5598 N_("prevent rename/copy detection if the number of rename/copy targets exceeds given limit")),
5599
5600 OPT_GROUP(N_("Diff algorithm options")),
5574 - OPT_BIT(0, "minimal", &options->xdl_opts,
5575 - N_("produce the smallest possible diff"),
5576 - XDF_NEED_MINIMAL),
5601 + OPT_CALLBACK_F(0, "minimal", options, NULL,
5602 + N_("produce the smallest possible diff"),
5603 + PARSE_OPT_NONEG | PARSE_OPT_NOARG,
5604 + diff_opt_diff_algorithm_no_arg),
5605 OPT_BIT_F('w', "ignore-all-space", &options->xdl_opts,
5606 N_("ignore whitespace when comparing lines"),
5607 XDF_IGNORE_WHITESPACE, PARSE_OPT_NONEG),
@@ -5599,9 +5627,10 @@ static void prep_parse_options(struct diff_options *options)
5627 N_("generate diff using the \"patience diff\" algorithm"),
5628 PARSE_OPT_NONEG | PARSE_OPT_NOARG,
5629 diff_opt_patience),
5602 - OPT_BITOP(0, "histogram", &options->xdl_opts,
5603 - N_("generate diff using the \"histogram diff\" algorithm"),
5604 - XDF_HISTOGRAM_DIFF, XDF_DIFF_ALGORITHM_MASK),
5630 + OPT_CALLBACK_F(0, "histogram", options, NULL,
5631 + N_("generate diff using the \"histogram diff\" algorithm"),
5632 + PARSE_OPT_NONEG | PARSE_OPT_NOARG,
5633 + diff_opt_diff_algorithm_no_arg),
5634 OPT_CALLBACK_F(0, "diff-algorithm", options, N_("<algorithm>"),
5635 N_("choose a diff algorithm"),
5636 PARSE_OPT_NONEG, diff_opt_diff_algorithm),