Re: [PATCH v2 3/3] diff: Introduce --diff-algorithm command line option
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:55:45
Michal Privoznik [off-list ref] writes:
+--diff-algorithm={patience|minimal|histogram|myers}::
+ Choose a diff algorithm. The variants are as follows:
++
+--
+`myers`;;
+ The basic greedy diff algorithm.
+`minimal`;;
+ Spend extra time to make sure the smallest possible diff is
+ produced.
+`patience`;;
+ Use "patience diff" algorithm when generating patches.
+`histogram`;;
+ This algorithm extends the patience algorithm to "support
+ low-occurrence common elements".
+--
++
+For instance, if you configured diff.algorithm variable to a
+non-default value and want to use the default one, then you
+have to use `--diff-algorithm=myers` option.
+
+You should prefer this option over the `--minimal`, `--patience` and
+`--histogram` which are kept just for backwards compatibility.Much better; I'd drop the last paragraph, though. I also think we really should consider "default" synonym for whichever happens to be the built-in default (currently myers).
quoted hunk ↗ jump to hunk
diff --git a/diff.c b/diff.c index e9a7e4d..3e021d5 100644 --- a/diff.c +++ b/diff.c@@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char *value) return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0; } -static long parse_algorithm_value(const char *value) +long parse_algorithm_value(const char *value) { if (!value || !strcasecmp(value, "myers")) return 0;@@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac) options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF); else if (!strcmp(arg, "--histogram")) options->xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF); + else if (!prefixcmp(arg, "--diff-algorithm=")) { + long value = parse_algorithm_value(arg+17); + if (value < 0) + 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;
This makes me wonder if other places that use DIFF_WITH_ALG() also need to worry about clearing NEED_MINIMAL?