{"thread":{"id":"32607","subject":"[PATCH 2/3] config: Introduce diff.algorithm variable","startedAt":"2013-01-12T16:02:12Z","lastAt":"2013-01-14T19:12:18Z","messageCount":7,"participants":["Michal Privoznik","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"206608","messageId":"cover.1358006339.git.mprivozn@redhat.com","threadId":"32607","inReplyTo":null,"subject":"[PATCH 0/3] Rework git-diff algorithm selection","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-12T16:02:12Z","receivedAt":"2013-01-12T16:02:12Z","isPatch":true,"sender":{"key":"mprivozn@redhat.com","avatar":"https://avatars.githubusercontent.com/u/12559331?v=4"},"body":"It's been a while I was trying to get this in. Recently, I realized how\nimportant this is.\n\nPlease keep me CC'ed as I am not subscribed to the list.\n\nMichal Privoznik (3):\n  git-completion.bash: Autocomplete --minimal and --histogram for\n    git-diff\n  config: Introduce diff.algorithm variable\n  diff: Introduce --diff-algorithm command line option\n\n Documentation/diff-config.txt          | 17 +++++++++++++++++\n Documentation/diff-options.txt         | 22 ++++++++++++++++++++++\n contrib/completion/git-completion.bash | 14 +++++++++++++-\n diff.c                                 | 33 +++++++++++++++++++++++++++++++++\n diff.h                                 |  2 ++\n merge-recursive.c                      |  6 ++++++\n 6 files changed, 93 insertions(+), 1 deletion(-)\n\n-- \n1.8.0.2\n"},{"id":"206607","messageId":"362b9aec3329d669d910dd4081e12c21ea552b2f.1358006339.git.mprivozn@redhat.com","threadId":"32607","inReplyTo":"cover.1358006339.git.mprivozn@redhat.com","subject":"[PATCH 1/3] git-completion.bash: Autocomplete --minimal and --histogram for git-diff","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-12T16:02:13Z","receivedAt":"2013-01-12T16:02:13Z","isPatch":true,"sender":{"key":"mprivozn@redhat.com","avatar":"https://avatars.githubusercontent.com/u/12559331?v=4"},"body":"Even though --patience was already there, we missed --minimal and\n--histogram for some reason.\n\nSigned-off-by: Michal Privoznik <mprivozn@redhat.com>\n---\n contrib/completion/git-completion.bash | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex a4c48e1..383518c 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1031,7 +1031,7 @@ __git_diff_common_options=\"--stat --numstat --shortstat --summary\n \t\t\t--no-ext-diff\n \t\t\t--no-prefix --src-prefix= --dst-prefix=\n \t\t\t--inter-hunk-context=\n-\t\t\t--patience\n+\t\t\t--patience --histogram --minimal\n \t\t\t--raw\n \t\t\t--dirstat --dirstat= --dirstat-by-file\n \t\t\t--dirstat-by-file= --cumulative\n-- \n1.8.0.2\n"},{"id":"206604","messageId":"72370b372a56cc5bfaa9741eae62eae2854964b2.1358006339.git.mprivozn@redhat.com","threadId":"32607","inReplyTo":"cover.1358006339.git.mprivozn@redhat.com","subject":"[PATCH 2/3] config: Introduce diff.algorithm variable","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-12T16:02:14Z","receivedAt":"2013-01-12T16:02:14Z","isPatch":true,"sender":{"key":"mprivozn@redhat.com","avatar":"https://avatars.githubusercontent.com/u/12559331?v=4"},"body":"Some users or projects prefer different algorithms over others, e.g.\npatience over myers or similar. However, specifying appropriate\nargument every time diff is to be used is impractical. Moreover,\ncreating an alias doesn't play nicely with other tools based on diff\n(git-show for instance). Hence, a configuration variable which is able\nto set specific algorithm is needed. For now, these four values are\naccepted: 'myers' (which has the same effect as not setting the config\nvariable at all), 'minimal', 'patience' and 'histogram'.\n\nSigned-off-by: Michal Privoznik <mprivozn@redhat.com>\n---\n Documentation/diff-config.txt          | 17 +++++++++++++++++\n contrib/completion/git-completion.bash |  1 +\n diff.c                                 | 23 +++++++++++++++++++++++\n 3 files changed, 41 insertions(+)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex 4314ad0..be31276 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -155,3 +155,20 @@ diff.tool::\n \t\"kompare\".  Any other value is treated as a custom diff tool,\n \tand there must be a corresponding `difftool.<tool>.cmd`\n \toption.\n+\n+diff.algorithm::\n+\tChoose a diff algorithm.  The variants are as follows:\n++\n+--\n+`myers`;;\n+\tThe basic greedy diff algorithm.\n+`minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+`patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+`histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 383518c..33e25dc 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1839,6 +1839,7 @@ _git_config ()\n \t\tdiff.suppressBlankEmpty\n \t\tdiff.tool\n \t\tdiff.wordRegex\n+\t\tdiff.algorithm\n \t\tdifftool.\n \t\tdifftool.prompt\n \t\tfetch.recurseSubmodules\ndiff --git a/diff.c b/diff.c\nindex 732d4c2..ddae5c4 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -36,6 +36,7 @@ static int diff_no_prefix;\n static int diff_stat_graph_width;\n static int diff_dirstat_permille_default = 30;\n static struct diff_options default_diff_options;\n+static long diff_algorithm = 0;\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_RESET,\n@@ -143,6 +144,20 @@ static int git_config_rename(const char *var, const char *value)\n \treturn git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;\n }\n \n+static long parse_algorithm_value(const char *value)\n+{\n+\tif (!value || !strcasecmp(value, \"myers\"))\n+\t\treturn 0;\n+\telse if (!strcasecmp(value, \"minimal\"))\n+\t\treturn XDF_NEED_MINIMAL;\n+\telse if (!strcasecmp(value, \"patience\"))\n+\t\treturn XDF_PATIENCE_DIFF;\n+\telse if (!strcasecmp(value, \"histogram\"))\n+\t\treturn XDF_HISTOGRAM_DIFF;\n+\telse\n+\t\treturn -1;\n+}\n+\n /*\n  * These are to give UI layer defaults.\n  * The core-level commands such as git-diff-files should\n@@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"diff.algorithm\")) {\n+\t\tdiff_algorithm = parse_algorithm_value(value);\n+\t\tif (diff_algorithm < 0)\n+\t\t\treturn -1;\n+\t\treturn 0;\n+\t}\n+\n \tif (git_color_config(var, value, cb) < 0)\n \t\treturn -1;\n \n@@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)\n \toptions->add_remove = diff_addremove;\n \toptions->use_color = diff_use_color_default;\n \toptions->detect_rename = diff_detect_rename_default;\n+\toptions->xdl_opts |= diff_algorithm;\n \n \tif (diff_no_prefix) {\n \t\toptions->a_prefix = options->b_prefix = \"\";\n-- \n1.8.0.2\n"},{"id":"206605","messageId":"6c058f13c6bb83985e8651a8dde99e7b17879d4e.1358006339.git.mprivozn@redhat.com","threadId":"32607","inReplyTo":"cover.1358006339.git.mprivozn@redhat.com","subject":"[PATCH 3/3] diff: Introduce --diff-algorithm command line option","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-12T16:02:15Z","receivedAt":"2013-01-12T16:02:15Z","isPatch":true,"sender":{"key":"mprivozn@redhat.com","avatar":"https://avatars.githubusercontent.com/u/12559331?v=4"},"body":"Since command line options have higher priority than config file\nvariables and taking previous commit into account, we need a way\nhow to specify myers algorithm on command line. However,\ninventing `--myers` is not the right answer. We need far more\ngeneral option, and that is `--diff-algorithm`. The older options\n(`--minimal`, `--patience` and `--histogram`) are kept for\nbackward compatibility.\n\nSigned-off-by: Michal Privoznik <mprivozn@redhat.com>\n---\n Documentation/diff-options.txt         | 22 ++++++++++++++++++++++\n contrib/completion/git-completion.bash | 11 +++++++++++\n diff.c                                 | 12 +++++++++++-\n diff.h                                 |  2 ++\n merge-recursive.c                      |  6 ++++++\n 5 files changed, 52 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 39f2c50..4091f52 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -45,6 +45,9 @@ ifndef::git-format-patch[]\n \tSynonym for `-p --raw`.\n endif::git-format-patch[]\n \n+The next three options are kept for compatibility reason. You should use the\n+`--diff-algorithm` option instead.\n+\n --minimal::\n \tSpend extra time to make sure the smallest possible\n \tdiff is produced.\n@@ -55,6 +58,25 @@ endif::git-format-patch[]\n --histogram::\n \tGenerate a diff using the \"histogram diff\" algorithm.\n \n+--diff-algorithm={patience|minimal|histogram|myers}::\n+\tChoose a diff algorithm. The variants are as follows:\n++\n+--\n+`myers`;;\n+\tThe basic greedy diff algorithm.\n+`minimal`;;\n+\tSpend extra time to make sure the smallest possible diff is\n+\tproduced.\n+`patience`;;\n+\tUse \"patience diff\" algorithm when generating patches.\n+`histogram`;;\n+\tThis algorithm extends the patience algorithm to \"support\n+\tlow-occurrence common elements\".\n+--\n++\n+You should prefer this option over the `--minimal`, `--patience` and\n+`--histogram` which are kept just for backwards compatibility.\n+\n --stat[=<width>[,<name-width>[,<count>]]]::\n \tGenerate a diffstat. By default, as much space as necessary\n \twill be used for the filename part, and the rest for the graph\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 33e25dc..d592cf9 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1021,6 +1021,8 @@ _git_describe ()\n \t__gitcomp_nl \"$(__git_refs)\"\n }\n \n+__git_diff_algorithms=\"myers minimal patience histogram\"\n+\n __git_diff_common_options=\"--stat --numstat --shortstat --summary\n \t\t\t--patch-with-stat --name-only --name-status --color\n \t\t\t--no-color --color-words --no-renames --check\n@@ -1035,6 +1037,7 @@ __git_diff_common_options=\"--stat --numstat --shortstat --summary\n \t\t\t--raw\n \t\t\t--dirstat --dirstat= --dirstat-by-file\n \t\t\t--dirstat-by-file= --cumulative\n+\t\t\t--diff-algorithm=\n \"\n \n _git_diff ()\n@@ -1042,6 +1045,10 @@ _git_diff ()\n \t__git_has_doubledash && return\n \n \tcase \"$cur\" in\n+\t--diff-algorithm=*)\n+\t\t__gitcomp \"$__git_diff_algorithms\" \"\" \"${cur##--diff-algorithm=}\"\n+\t\treturn\n+\t\t;;\n \t--*)\n \t\t__gitcomp \"--cached --staged --pickaxe-all --pickaxe-regex\n \t\t\t--base --ours --theirs --no-index\n@@ -2114,6 +2121,10 @@ _git_show ()\n \t\t\t\" \"\" \"${cur#*=}\"\n \t\treturn\n \t\t;;\n+\t--diff-algorithm=*)\n+\t\t__gitcomp \"$__git_diff_algorithms\" \"\" \"${cur##--diff-algorithm=}\"\n+\t\treturn\n+\t\t;;\n \t--*)\n \t\t__gitcomp \"--pretty= --format= --abbrev-commit --oneline\n \t\t\t$__git_diff_common_options\ndiff --git a/diff.c b/diff.c\nindex ddae5c4..6418076 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char *value)\n \treturn git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;\n }\n \n-static long parse_algorithm_value(const char *value)\n+long parse_algorithm_value(const char *value)\n {\n \tif (!value || !strcasecmp(value, \"myers\"))\n \t\treturn 0;\n@@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);\n \telse if (!strcmp(arg, \"--histogram\"))\n \t\toptions->xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);\n+\telse if (!prefixcmp(arg, \"--diff-algorithm=\")) {\n+\t\tlong value = parse_algorithm_value(arg+17);\n+\t\tif (value < 0)\n+\t\t\treturn error(\"option diff-algorithm accepts \\\"myers\\\", \"\n+\t\t\t\t     \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\");\n+\t\t/* clear out previous settings */\n+\t\tDIFF_XDL_CLR(options, NEED_MINIMAL);\n+\t\toptions->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n+\t\toptions->xdl_opts |= value;\n+\t}\n \n \t/* flags options */\n \telse if (!strcmp(arg, \"--binary\")) {\ndiff --git a/diff.h b/diff.h\nindex a47bae4..54c2590 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -333,6 +333,8 @@ extern struct userdiff_driver *get_textconv(struct diff_filespec *one);\n \n extern int parse_rename_score(const char **cp_p);\n \n+extern long parse_algorithm_value(const char *value);\n+\n extern int print_stat_summary(FILE *fp, int files,\n \t\t\t      int insertions, int deletions);\n extern void setup_diff_pager(struct diff_options *);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d882060..53d8feb 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -2068,6 +2068,12 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);\n \telse if (!strcmp(s, \"histogram\"))\n \t\to->xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);\n+\telse if (!strcmp(s, \"diff-algorithm=\")) {\n+\t\tlong value = parse_algorithm_value(s+15);\n+\t\tif (value < 0)\n+\t\t\treturn -1;\n+\t\to->xdl_opts |= value;\n+\t}\n \telse if (!strcmp(s, \"ignore-space-change\"))\n \t\to->xdl_opts |= XDF_IGNORE_WHITESPACE_CHANGE;\n \telse if (!strcmp(s, \"ignore-all-space\"))\n-- \n1.8.0.2\n"},{"id":"206822","messageId":"7vwqvfbomm.fsf@alter.siamese.dyndns.org","threadId":"32607","inReplyTo":"72370b372a56cc5bfaa9741eae62eae2854964b2.1358006339.git.mprivozn@redhat.com","subject":"Re: [PATCH 2/3] config: Introduce diff.algorithm variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-14T18:33:53Z","receivedAt":"2013-01-14T18:33:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Privoznik <mprivozn@redhat.com> writes:\n\n> Some users or projects prefer different algorithms over others, e.g.\n> patience over myers or similar. However, specifying appropriate\n> argument every time diff is to be used is impractical. Moreover,\n> creating an alias doesn't play nicely with other tools based on diff\n> (git-show for instance). Hence, a configuration variable which is able\n> to set specific algorithm is needed. For now, these four values are\n> accepted: 'myers' (which has the same effect as not setting the config\n> variable at all), 'minimal', 'patience' and 'histogram'.\n>\n> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n> ---\n>  Documentation/diff-config.txt          | 17 +++++++++++++++++\n>  contrib/completion/git-completion.bash |  1 +\n>  diff.c                                 | 23 +++++++++++++++++++++++\n>  3 files changed, 41 insertions(+)\n>\n> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\n> index 4314ad0..be31276 100644\n> --- a/Documentation/diff-config.txt\n> +++ b/Documentation/diff-config.txt\n> @@ -155,3 +155,20 @@ diff.tool::\n>  \t\"kompare\".  Any other value is treated as a custom diff tool,\n>  \tand there must be a corresponding `difftool.<tool>.cmd`\n>  \toption.\n> +\n> +diff.algorithm::\n> +\tChoose a diff algorithm.  The variants are as follows:\n> ++\n> +--\n> +`myers`;;\n> +\tThe basic greedy diff algorithm.\n> +`minimal`;;\n> +\tSpend extra time to make sure the smallest possible diff is\n> +\tproduced.\n> +`patience`;;\n> +\tUse \"patience diff\" algorithm when generating patches.\n> +`histogram`;;\n> +\tThis algorithm extends the patience algorithm to \"support\n> +\tlow-occurrence common elements\".\n> +--\n> ++\n\nSounds sensible.\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 383518c..33e25dc 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1839,6 +1839,7 @@ _git_config ()\n>  \t\tdiff.suppressBlankEmpty\n>  \t\tdiff.tool\n>  \t\tdiff.wordRegex\n> +\t\tdiff.algorithm\n>  \t\tdifftool.\n>  \t\tdifftool.prompt\n>  \t\tfetch.recurseSubmodules\n> diff --git a/diff.c b/diff.c\n> index 732d4c2..ddae5c4 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -36,6 +36,7 @@ static int diff_no_prefix;\n>  static int diff_stat_graph_width;\n>  static int diff_dirstat_permille_default = 30;\n>  static struct diff_options default_diff_options;\n> +static long diff_algorithm = 0;\n\nPlease do not initialize a static explicitly to a zero and instead\nlet BSS do its thing.\n\n>  static char diff_colors[][COLOR_MAXLEN] = {\n>  \tGIT_COLOR_RESET,\n> @@ -143,6 +144,20 @@ static int git_config_rename(const char *var, const char *value)\n>  \treturn git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;\n>  }\n>  \n> +static long parse_algorithm_value(const char *value)\n> +{\n> +\tif (!value || !strcasecmp(value, \"myers\"))\n> +\t\treturn 0;\n> +\telse if (!strcasecmp(value, \"minimal\"))\n> +\t\treturn XDF_NEED_MINIMAL;\n> +\telse if (!strcasecmp(value, \"patience\"))\n> +\t\treturn XDF_PATIENCE_DIFF;\n> +\telse if (!strcasecmp(value, \"histogram\"))\n> +\t\treturn XDF_HISTOGRAM_DIFF;\n> +\telse\n> +\t\treturn -1;\n> +}\n> +\n>  /*\n>   * These are to give UI layer defaults.\n>   * The core-level commands such as git-diff-files should\n> @@ -196,6 +211,13 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)\n>  \t\treturn 0;\n>  \t}\n>  \n> +\tif (!strcmp(var, \"diff.algorithm\")) {\n> +\t\tdiff_algorithm = parse_algorithm_value(value);\n> +\t\tif (diff_algorithm < 0)\n> +\t\t\treturn -1;\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tif (git_color_config(var, value, cb) < 0)\n>  \t\treturn -1;\n>  \n> @@ -3213,6 +3235,7 @@ void diff_setup(struct diff_options *options)\n>  \toptions->add_remove = diff_addremove;\n>  \toptions->use_color = diff_use_color_default;\n>  \toptions->detect_rename = diff_detect_rename_default;\n> +\toptions->xdl_opts |= diff_algorithm;\n>  \n>  \tif (diff_no_prefix) {\n>  \t\toptions->a_prefix = options->b_prefix = \"\";\n"},{"id":"206823","messageId":"7vsj63bo4n.fsf@alter.siamese.dyndns.org","threadId":"32607","inReplyTo":"6c058f13c6bb83985e8651a8dde99e7b17879d4e.1358006339.git.mprivozn@redhat.com","subject":"Re: [PATCH 3/3] diff: Introduce --diff-algorithm command line option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-14T18:44:40Z","receivedAt":"2013-01-14T18:44:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michal Privoznik <mprivozn@redhat.com> writes:\n\n> Since command line options have higher priority than config file\n> variables and taking previous commit into account, we need a way\n> how to specify myers algorithm on command line.\n\nYes.  We cannot stop at [2/3] without this patch.\n\n> However,\n> inventing `--myers` is not the right answer. We need far more\n> general option, and that is `--diff-algorithm`.\n\nYes, --diff-algorithm=default would let people defeat a configured\nalgo without knowing how exactly to spell myers.\n\n> The older options\n> (`--minimal`, `--patience` and `--histogram`) are kept for\n> backward compatibility.\n\nThat is phrasing it too strongly to be acceptable.\n\nPeople who do not care any configuration can keep using --histogram\nwithout having to retrain their fingers to type a much longer form\nyou introduced (i.e. --diff-algorithm=histogram).  It is and will\nalways be just as valid a way to ask for --histogram as the new\nlonghand is.\n\n> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>\n> ---\n>  Documentation/diff-options.txt         | 22 ++++++++++++++++++++++\n>  contrib/completion/git-completion.bash | 11 +++++++++++\n>  diff.c                                 | 12 +++++++++++-\n>  diff.h                                 |  2 ++\n>  merge-recursive.c                      |  6 ++++++\n>  5 files changed, 52 insertions(+), 1 deletion(-)\n>\n> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\n> index 39f2c50..4091f52 100644\n> --- a/Documentation/diff-options.txt\n> +++ b/Documentation/diff-options.txt\n> @@ -45,6 +45,9 @@ ifndef::git-format-patch[]\n>  \tSynonym for `-p --raw`.\n>  endif::git-format-patch[]\n>  \n> +The next three options are kept for compatibility reason. You should use the\n> +`--diff-algorithm` option instead.\n> +\n\nDrop this.\n\n> @@ -55,6 +58,25 @@ endif::git-format-patch[]\n>  --histogram::\n>  \tGenerate a diff using the \"histogram diff\" algorithm.\n>  \n> +--diff-algorithm={patience|minimal|histogram|myers}::\n> +\tChoose a diff algorithm. The variants are as follows:\n> ++\n> +--\n> +`myers`;;\n> +\tThe basic greedy diff algorithm.\n> +`minimal`;;\n> +\tSpend extra time to make sure the smallest possible diff is\n> +\tproduced.\n> +`patience`;;\n> +\tUse \"patience diff\" algorithm when generating patches.\n> +`histogram`;;\n> +\tThis algorithm extends the patience algorithm to \"support\n> +\tlow-occurrence common elements\".\n> +--\n> ++\n> +You should prefer this option over the `--minimal`, `--patience` and\n> +`--histogram` which are kept just for backwards compatibility.\n\nDrop the last one, and replace it with something like:\n\n\tIf you configured diff.algorithm to a non-default value and\n\twant to use the default one, you have to use this form and\n\tchoose myers, i.e. --diff-algorithm=myers, as we do not have\n\ta short-and-sweet --myers option.\n\n(but the above is a bit too verbose, so please shorten it appropriately).\n\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 33e25dc..d592cf9 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -1021,6 +1021,8 @@ _git_describe ()\n>  \t__gitcomp_nl \"$(__git_refs)\"\n>  }\n>  \n> +__git_diff_algorithms=\"myers minimal patience histogram\"\n> +\n>  __git_diff_common_options=\"--stat --numstat --shortstat --summary\n>  \t\t\t--patch-with-stat --name-only --name-status --color\n>  \t\t\t--no-color --color-words --no-renames --check\n> @@ -1035,6 +1037,7 @@ __git_diff_common_options=\"--stat --numstat --shortstat --summary\n>  \t\t\t--raw\n>  \t\t\t--dirstat --dirstat= --dirstat-by-file\n>  \t\t\t--dirstat-by-file= --cumulative\n> +\t\t\t--diff-algorithm=\n>  \"\n>  \n>  _git_diff ()\n> @@ -1042,6 +1045,10 @@ _git_diff ()\n>  \t__git_has_doubledash && return\n>  \n>  \tcase \"$cur\" in\n> +\t--diff-algorithm=*)\n> +\t\t__gitcomp \"$__git_diff_algorithms\" \"\" \"${cur##--diff-algorithm=}\"\n> +\t\treturn\n> +\t\t;;\n>  \t--*)\n>  \t\t__gitcomp \"--cached --staged --pickaxe-all --pickaxe-regex\n>  \t\t\t--base --ours --theirs --no-index\n> @@ -2114,6 +2121,10 @@ _git_show ()\n>  \t\t\t\" \"\" \"${cur#*=}\"\n>  \t\treturn\n>  \t\t;;\n> +\t--diff-algorithm=*)\n> +\t\t__gitcomp \"$__git_diff_algorithms\" \"\" \"${cur##--diff-algorithm=}\"\n> +\t\treturn\n> +\t\t;;\n>  \t--*)\n>  \t\t__gitcomp \"--pretty= --format= --abbrev-commit --oneline\n>  \t\t\t$__git_diff_common_options\n> diff --git a/diff.c b/diff.c\n> index ddae5c4..6418076 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -144,7 +144,7 @@ static int git_config_rename(const char *var, const char *value)\n>  \treturn git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;\n>  }\n>  \n> -static long parse_algorithm_value(const char *value)\n> +long parse_algorithm_value(const char *value)\n>  {\n>  \tif (!value || !strcasecmp(value, \"myers\"))\n>  \t\treturn 0;\n> @@ -3633,6 +3633,16 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n>  \t\toptions->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);\n>  \telse if (!strcmp(arg, \"--histogram\"))\n>  \t\toptions->xdl_opts = DIFF_WITH_ALG(options, HISTOGRAM_DIFF);\n> +\telse if (!prefixcmp(arg, \"--diff-algorithm=\")) {\n> +\t\tlong value = parse_algorithm_value(arg+17);\n> +\t\tif (value < 0)\n> +\t\t\treturn error(\"option diff-algorithm accepts \\\"myers\\\", \"\n> +\t\t\t\t     \"\\\"minimal\\\", \\\"patience\\\" and \\\"histogram\\\"\");\n> +\t\t/* clear out previous settings */\n> +\t\tDIFF_XDL_CLR(options, NEED_MINIMAL);\n> +\t\toptions->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\n> +\t\toptions->xdl_opts |= value;\n> +\t}\n>  \n>  \t/* flags options */\n>  \telse if (!strcmp(arg, \"--binary\")) {\n> diff --git a/diff.h b/diff.h\n> index a47bae4..54c2590 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -333,6 +333,8 @@ extern struct userdiff_driver *get_textconv(struct diff_filespec *one);\n>  \n>  extern int parse_rename_score(const char **cp_p);\n>  \n> +extern long parse_algorithm_value(const char *value);\n> +\n>  extern int print_stat_summary(FILE *fp, int files,\n>  \t\t\t      int insertions, int deletions);\n>  extern void setup_diff_pager(struct diff_options *);\n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index d882060..53d8feb 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -2068,6 +2068,12 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n>  \t\to->xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);\n>  \telse if (!strcmp(s, \"histogram\"))\n>  \t\to->xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);\n> +\telse if (!strcmp(s, \"diff-algorithm=\")) {\n> +\t\tlong value = parse_algorithm_value(s+15);\n> +\t\tif (value < 0)\n> +\t\t\treturn -1;\n> +\t\to->xdl_opts |= value;\n\nIsn't this new hunk wrong?  DIFF_WITH_ALG() removes the previously\nchosen algorithm choice before OR'ing the new one in, so that\n\n\tdiff --histogram --patience\n\nwould not end up with a value PATIENCE|HISTOGRAM OR'ed together in\nthe algo field.\n"},{"id":"206827","messageId":"7vtxqja8a5.fsf@alter.siamese.dyndns.org","threadId":"32607","inReplyTo":"7vsj63bo4n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] diff: Introduce --diff-algorithm command line option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-14T19:12:18Z","receivedAt":"2013-01-14T19:12:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Michal Privoznik <mprivozn@redhat.com> writes:\n> ...\n>> diff --git a/merge-recursive.c b/merge-recursive.c\n>> index d882060..53d8feb 100644\n>> --- a/merge-recursive.c\n>> +++ b/merge-recursive.c\n>> @@ -2068,6 +2068,12 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n>>  \t\to->xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);\n>>  \telse if (!strcmp(s, \"histogram\"))\n>>  \t\to->xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);\n>> +\telse if (!strcmp(s, \"diff-algorithm=\")) {\n>> +\t\tlong value = parse_algorithm_value(s+15);\n>> +\t\tif (value < 0)\n>> +\t\t\treturn -1;\n>> +\t\to->xdl_opts |= value;\n>\n> Isn't this new hunk wrong?  DIFF_WITH_ALG() removes the previously\n> chosen algorithm choice before OR'ing the new one in, so that\n>\n> \tdiff --histogram --patience\n>\n> would not end up with a value PATIENCE|HISTOGRAM OR'ed together in\n> the algo field.\n\nI misspoke; this is not \"diff\", but \"merge-recursive\".  The issue\nmay be the same here, though.\n"}]}