Rework git-diff algorithm selection

diff.c +18 -0

+	else if (!strcasecmp(value, "minimal"))
+		return XDF_NEED_MINIMAL;
+	else if (!strcasecmp(value, "patience"))
+		return XDF_PATIENCE_DIFF;
+	else if (!strcasecmp(value, "histogram"))
+		return XDF_HISTOGRAM_DIFF;
+	else
+		return -1;
+}
+
 /*
  * These are to give UI layer defaults.
  * The core-level commands such as git-diff-files should

@@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
 		return 0;
 	}
 
+	if (!strcmp(var, "diff.algorithm")) {
+		diff_algorithm = parse_algorithm_value(value);
+		if (diff_algorithm < 0)
+			return -1;
+		return 0;
+	}
+
 	if (git_color_config(var, value, cb) < 0)
 		return -1;
 

@@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)
 	options->add_remove = diff_addremove;
 	options->use_color = diff_use_color_default;
 	options->detect_rename = diff_detect_rename_default;
+	options->xdl_opts |= diff_algorithm;
 
 	if (diff_no_prefix) {
 		options->a_prefix = options->b_prefix = "";

diff.c static int git_config_rename(const char *var, const char *value) · line 150

Quoted 4 lines · Michal Privoznik

diff.c +4 -0

+static long parse_algorithm_value(const char *value)
+{
+	if (!value || !strcasecmp(value, "myers"))
+		return 0;

[diff]
algorithm

should probably error out.

Also it is rather unusual to parse the keyword values case insensitively.

Quoted 1 line

Also it is rather unusual to parse the keyword values case insensitively.

Is it? "git grep strcasecmp" shows that we already do so in many cases (e.g., any bool option, core.autocrlf, receive.deny*, etc).

+--diff-algorithm={patience|minimal|histogram|myers}::

Documentation/diff-options.txt +11 -0

+	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`;;

diff.c +1 -1

index e9a7e4d..3e021d5 100644
@@ -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"))

Quoted 12 lines · Michal Privoznik

+--diff-algorithm={patience|minimal|histogram|myers}::

Documentation/diff-options.txt +11 -0

+	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`;;

Documentation/diff-options.txt endif::git-format-patch[] · line 79

Quoted 10 lines

Documentation/diff-options.txt +10 -0

+	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 21 lines

diff.c +5 -1

index e9a7e4d..3e021d5 100644
@@ -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\", "

diff.c int diff_opt_parse(struct diff_options *options, const char **av, int ac) · line 3644

Quoted 5 lines

diff.c +5 -0

+				     "\"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?