{"thread":{"id":"32647","subject":"[PATCH v3 3/3] diff: Introduce --diff-algorithm command line option","startedAt":"2013-01-16T07:51:55Z","lastAt":"2013-01-16T21:59:59Z","messageCount":6,"participants":["Michal Privoznik","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"207035","messageId":"cover.1358322212.git.mprivozn@redhat.com","threadId":"32647","inReplyTo":null,"subject":"[PATCH v3 0/3] Rework git-diff algorithm selection","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-16T07:51:55Z","receivedAt":"2013-01-16T07:51:55Z","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\ndiff to v1:\n-Junio's suggestions worked in\n\ndiff to v2:\n-yet more Junio's suggestions included\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          | 18 ++++++++++++++++++\n Documentation/diff-options.txt         | 21 +++++++++++++++++++++\n contrib/completion/git-completion.bash | 14 +++++++++++++-\n diff.c                                 | 34 ++++++++++++++++++++++++++++++++++\n diff.h                                 |  2 ++\n merge-recursive.c                      |  9 +++++++++\n 6 files changed, 97 insertions(+), 1 deletion(-)\n\n-- \n1.8.0.2\n"},{"id":"207033","messageId":"6f036d7e14a214e770edf32cb0e015999ba98d85.1358322212.git.mprivozn@redhat.com","threadId":"32647","inReplyTo":"cover.1358322212.git.mprivozn@redhat.com","subject":"[PATCH v3 1/3] git-completion.bash: Autocomplete --minimal and --histogram for git-diff","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-16T07:51:56Z","receivedAt":"2013-01-16T07:51:56Z","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":"207034","messageId":"4e2aacd5bbf005f0e372589bf423a8cbd776bc6d.1358322212.git.mprivozn@redhat.com","threadId":"32647","inReplyTo":"cover.1358322212.git.mprivozn@redhat.com","subject":"[PATCH v3 2/3] config: Introduce diff.algorithm variable","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-16T07:51:57Z","receivedAt":"2013-01-16T07:51:57Z","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          | 18 ++++++++++++++++++\n contrib/completion/git-completion.bash |  1 +\n diff.c                                 | 24 ++++++++++++++++++++++++\n 3 files changed, 43 insertions(+)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex 4314ad0..3a62ace 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -155,3 +155,21 @@ 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+`default`, `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this happens to be\n+\tthe default algorithm as well.\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 348f71b..7d6cc4c 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;\n \n static char diff_colors[][COLOR_MAXLEN] = {\n \tGIT_COLOR_RESET,\n@@ -143,6 +144,21 @@ 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)\n+\t\treturn -1;\n+\telse if (!strcasecmp(value, \"myers\") || !strcasecmp(value, \"default\"))\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+\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 +212,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 +3236,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":"207032","messageId":"9ab743d384a9de33a3410fac9ee3e74acf964700.1358322212.git.mprivozn@redhat.com","threadId":"32647","inReplyTo":"cover.1358322212.git.mprivozn@redhat.com","subject":"[PATCH v3 3/3] diff: Introduce --diff-algorithm command line option","fromName":"Michal Privoznik","fromEmail":"mprivozn@redhat.com","sentAt":"2013-01-16T07:51:58Z","receivedAt":"2013-01-16T07:51:58Z","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`.\n\nSigned-off-by: Michal Privoznik <mprivozn@redhat.com>\n---\n Documentation/diff-options.txt         | 21 +++++++++++++++++++++\n contrib/completion/git-completion.bash | 11 +++++++++++\n diff.c                                 | 12 +++++++++++-\n diff.h                                 |  2 ++\n merge-recursive.c                      |  9 +++++++++\n 5 files changed, 54 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt\nindex 39f2c50..6f11c34 100644\n--- a/Documentation/diff-options.txt\n+++ b/Documentation/diff-options.txt\n@@ -55,6 +55,27 @@ 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+`default`, `myers`;;\n+\tThe basic greedy diff algorithm. Currently, this happens to be\n+\tthe default algorithm as well.\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+For instance, if you configured diff.algorithm variable to a\n+non-default value and want to use the default one, then you\n+have to use `--diff-algorithm=default` option.\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 7d6cc4c..5fa40e9 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)\n \t\treturn -1;\n@@ -3634,6 +3634,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 33ba5dc..ea9dbd3 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -2068,6 +2068,15 @@ 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\t/* clear out previous settings */\n+\t\tDIFF_XDL_CLR(o, NEED_MINIMAL);\n+\t\to->xdl_opts &= ~XDF_DIFF_ALGORITHM_MASK;\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":"207074","messageId":"7vbocpxbwp.fsf@alter.siamese.dyndns.org","threadId":"32647","inReplyTo":"4e2aacd5bbf005f0e372589bf423a8cbd776bc6d.1358322212.git.mprivozn@redhat.com","subject":"Re: [PATCH v3 2/3] config: Introduce diff.algorithm variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-16T17:42:14Z","receivedAt":"2013-01-16T17:42:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Will replace the one in 'pu' with this round.  Looking good.\n\nThanks.\n"},{"id":"207109","messageId":"7v622wwzz4.fsf@alter.siamese.dyndns.org","threadId":"32647","inReplyTo":"7vbocpxbwp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 2/3] config: Introduce diff.algorithm variable","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-01-16T21:59:59Z","receivedAt":"2013-01-16T21:59:59Z","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> Will replace the one in 'pu' with this round.  Looking good.\n>\n> Thanks.\n\nBy the way, wouldn't we want some tests to protect this feature from\nfuture breakages?\n"}]}