On 14.01.2013 22:06, Junio C Hamano wrote:
> Michal Privoznik <mprivozn@xxxxxxxxxx> 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).
>
>> 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?
>
Not really. In my approach, --minimal looks at yet another algorithm.
However, current code sees it as orthogonal to the myers algorithm. The
flag doesn't have any effect on --patience or --histogram at all.
So I think we need to clear the flag only when using --diff-algorithm.
Michal
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html