Michal Privoznik | 12 Jan 17:02 2013
Picon

[PATCH 0/3] Rework git-diff algorithm selection

It's been a while I was trying to get this in. Recently, I realized how
important this is.

Please keep me CC'ed as I am not subscribed to the list.

Michal Privoznik (3):
  git-completion.bash: Autocomplete --minimal and --histogram for
    git-diff
  config: Introduce diff.algorithm variable
  diff: Introduce --diff-algorithm command line option

 Documentation/diff-config.txt          | 17 +++++++++++++++++
 Documentation/diff-options.txt         | 22 ++++++++++++++++++++++
 contrib/completion/git-completion.bash | 14 +++++++++++++-
 diff.c                                 | 33 +++++++++++++++++++++++++++++++++
 diff.h                                 |  2 ++
 merge-recursive.c                      |  6 ++++++
 6 files changed, 93 insertions(+), 1 deletion(-)

--

-- 
1.8.0.2

Michal Privoznik | 12 Jan 17:02 2013
Picon

[PATCH 2/3] config: Introduce diff.algorithm variable

Some users or projects prefer different algorithms over others, e.g.
patience over myers or similar. However, specifying appropriate
argument every time diff is to be used is impractical. Moreover,
creating an alias doesn't play nicely with other tools based on diff
(git-show for instance). Hence, a configuration variable which is able
to set specific algorithm is needed. For now, these four values are
accepted: 'myers' (which has the same effect as not setting the config
variable at all), 'minimal', 'patience' and 'histogram'.

Signed-off-by: Michal Privoznik <mprivozn <at> redhat.com>
---
 Documentation/diff-config.txt          | 17 +++++++++++++++++
 contrib/completion/git-completion.bash |  1 +
 diff.c                                 | 23 +++++++++++++++++++++++
 3 files changed, 41 insertions(+)

diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
index 4314ad0..be31276 100644
--- a/Documentation/diff-config.txt
+++ b/Documentation/diff-config.txt
 <at>  <at>  -155,3 +155,20  <at>  <at>  diff.tool::
 	"kompare".  Any other value is treated as a custom diff tool,
 	and there must be a corresponding `difftool.<tool>.cmd`
 	option.
+
+diff.algorithm::
+	Choose a diff algorithm.  The variants are as follows:
++
+--
+`myers`;;
(Continue reading)

Junio C Hamano | 14 Jan 19:33 2013
Picon
Picon

Re: [PATCH 2/3] config: Introduce diff.algorithm variable

Michal Privoznik <mprivozn <at> redhat.com> writes:

> Some users or projects prefer different algorithms over others, e.g.
> patience over myers or similar. However, specifying appropriate
> argument every time diff is to be used is impractical. Moreover,
> creating an alias doesn't play nicely with other tools based on diff
> (git-show for instance). Hence, a configuration variable which is able
> to set specific algorithm is needed. For now, these four values are
> accepted: 'myers' (which has the same effect as not setting the config
> variable at all), 'minimal', 'patience' and 'histogram'.
>
> Signed-off-by: Michal Privoznik <mprivozn <at> redhat.com>
> ---
>  Documentation/diff-config.txt          | 17 +++++++++++++++++
>  contrib/completion/git-completion.bash |  1 +
>  diff.c                                 | 23 +++++++++++++++++++++++
>  3 files changed, 41 insertions(+)
>
> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt
> index 4314ad0..be31276 100644
> --- a/Documentation/diff-config.txt
> +++ b/Documentation/diff-config.txt
>  <at>  <at>  -155,3 +155,20  <at>  <at>  diff.tool::
>  	"kompare".  Any other value is treated as a custom diff tool,
>  	and there must be a corresponding `difftool.<tool>.cmd`
>  	option.
> +
> +diff.algorithm::
> +	Choose a diff algorithm.  The variants are as follows:
> ++
(Continue reading)

Michal Privoznik | 12 Jan 17:02 2013
Picon

[PATCH 3/3] diff: Introduce --diff-algorithm command line option

Since command line options have higher priority than config file
variables and taking previous commit into account, we need a way
how to specify myers algorithm on command line. However,
inventing `--myers` is not the right answer. We need far more
general option, and that is `--diff-algorithm`. The older options
(`--minimal`, `--patience` and `--histogram`) are kept for
backward compatibility.

Signed-off-by: Michal Privoznik <mprivozn <at> redhat.com>
---
 Documentation/diff-options.txt         | 22 ++++++++++++++++++++++
 contrib/completion/git-completion.bash | 11 +++++++++++
 diff.c                                 | 12 +++++++++++-
 diff.h                                 |  2 ++
 merge-recursive.c                      |  6 ++++++
 5 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 39f2c50..4091f52 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
 <at>  <at>  -45,6 +45,9  <at>  <at>  ifndef::git-format-patch[]
 	Synonym for `-p --raw`.
 endif::git-format-patch[]

+The next three options are kept for compatibility reason. You should use the
+`--diff-algorithm` option instead.
+
 --minimal::
 	Spend extra time to make sure the smallest possible
(Continue reading)

Junio C Hamano | 14 Jan 19:44 2013
Picon
Picon

Re: [PATCH 3/3] diff: Introduce --diff-algorithm command line option

Michal Privoznik <mprivozn <at> redhat.com> writes:

> Since command line options have higher priority than config file
> variables and taking previous commit into account, we need a way
> how to specify myers algorithm on command line.

Yes.  We cannot stop at [2/3] without this patch.

> However,
> inventing `--myers` is not the right answer. We need far more
> general option, and that is `--diff-algorithm`.

Yes, --diff-algorithm=default would let people defeat a configured
algo without knowing how exactly to spell myers.

> The older options
> (`--minimal`, `--patience` and `--histogram`) are kept for
> backward compatibility.

That is phrasing it too strongly to be acceptable.

People who do not care any configuration can keep using --histogram
without having to retrain their fingers to type a much longer form
you introduced (i.e. --diff-algorithm=histogram).  It is and will
always be just as valid a way to ask for --histogram as the new
longhand is.

> Signed-off-by: Michal Privoznik <mprivozn <at> redhat.com>
> ---
>  Documentation/diff-options.txt         | 22 ++++++++++++++++++++++
(Continue reading)

Junio C Hamano | 14 Jan 20:12 2013
Picon
Picon

Re: [PATCH 3/3] diff: Introduce --diff-algorithm command line option

Junio C Hamano <gitster <at> pobox.com> writes:

> Michal Privoznik <mprivozn <at> redhat.com> writes:
> ...
>> diff --git a/merge-recursive.c b/merge-recursive.c
>> index d882060..53d8feb 100644
>> --- a/merge-recursive.c
>> +++ b/merge-recursive.c
>>  <at>  <at>  -2068,6 +2068,12  <at>  <at>  int parse_merge_opt(struct merge_options *o, const char *s)
>>  		o->xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);
>>  	else if (!strcmp(s, "histogram"))
>>  		o->xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);
>> +	else if (!strcmp(s, "diff-algorithm=")) {
>> +		long value = parse_algorithm_value(s+15);
>> +		if (value < 0)
>> +			return -1;
>> +		o->xdl_opts |= value;
>
> Isn't this new hunk wrong?  DIFF_WITH_ALG() removes the previously
> chosen algorithm choice before OR'ing the new one in, so that
>
> 	diff --histogram --patience
>
> would not end up with a value PATIENCE|HISTOGRAM OR'ed together in
> the algo field.

I misspoke; this is not "diff", but "merge-recursive".  The issue
may be the same here, though.
Michal Privoznik | 12 Jan 17:02 2013
Picon

[PATCH 1/3] git-completion.bash: Autocomplete --minimal and --histogram for git-diff

Even though --patience was already there, we missed --minimal and
--histogram for some reason.

Signed-off-by: Michal Privoznik <mprivozn <at> redhat.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a4c48e1..383518c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
 <at>  <at>  -1031,7 +1031,7  <at>  <at>  __git_diff_common_options="--stat --numstat --shortstat --summary
 			--no-ext-diff
 			--no-prefix --src-prefix= --dst-prefix=
 			--inter-hunk-context=
-			--patience
+			--patience --histogram --minimal
 			--raw
 			--dirstat --dirstat= --dirstat-by-file
 			--dirstat-by-file= --cumulative
--

-- 
1.8.0.2


Gmane