{"thread":{"id":"44722","subject":"[PATCH] diff: prefer indent heuristic over compaction heuristic","startedAt":"2016-12-17T00:54:49Z","lastAt":"2017-04-28T08:41:24Z","messageCount":17,"participants":["Jacob Keller","Junio C Hamano","Jeff King","Michael Haggerty","Stefan Beller","Martin Liška"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"307937","messageId":"20161217005442.5866-1-jacob.e.keller@intel.com","threadId":"44722","inReplyTo":null,"subject":"[PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jacob Keller","fromEmail":"jacob.e.keller@intel.com","sentAt":"2016-12-17T00:54:42Z","receivedAt":"2016-12-17T00:54:49Z","isPatch":true,"sender":{"key":"jacob.e.keller@intel.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"From: Jacob Keller <jacob.keller@gmail.com>\n\nThe current configuration code for enabling experimental heuristics\nprefers the last-set heuristic in the configuration. However, it is not\nnecessarily easy to see what order the configuration will be read. This\nmeans that it is possible for a user to have accidentally enabled both\nheuristics, and end up only enabling the older compaction heuristic.\n\nModify the code so that we do not clear the other heuristic when we set\neach heuristic enabled. Then, during diff_setup() when we check the\nconfiguration, we will first check the newer indent heuristic. This\nensures that we only enable the newer heuristic if both have been\nenabled.\n\nSigned-off-by: Jacob Keller <jacob.keller@gmail.com>\n---\n diff.c | 10 ++--------\n 1 file changed, 2 insertions(+), 8 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex ec8728362dae..48a5b2797e3d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -223,16 +223,10 @@ void init_diff_ui_defaults(void)\n \n int git_diff_heuristic_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"diff.indentheuristic\")) {\n+\tif (!strcmp(var, \"diff.indentheuristic\"))\n \t\tdiff_indent_heuristic = git_config_bool(var, value);\n-\t\tif (diff_indent_heuristic)\n-\t\t\tdiff_compaction_heuristic = 0;\n-\t}\n-\tif (!strcmp(var, \"diff.compactionheuristic\")) {\n+\tif (!strcmp(var, \"diff.compactionheuristic\"))\n \t\tdiff_compaction_heuristic = git_config_bool(var, value);\n-\t\tif (diff_compaction_heuristic)\n-\t\t\tdiff_indent_heuristic = 0;\n-\t}\n \treturn 0;\n }\n \n-- \n2.11.0.rc2.152.g4d04e67\n\n"},{"id":"307946","messageId":"xmqq7f6zqr3i.fsf@gitster.mtv.corp.google.com","threadId":"44722","inReplyTo":"20161217005442.5866-1-jacob.e.keller@intel.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-17T01:30:57Z","receivedAt":"2016-12-17T01:31:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.e.keller@intel.com> writes:\n\n> From: Jacob Keller <jacob.keller@gmail.com>\n>\n> The current configuration code for enabling experimental heuristics\n> prefers the last-set heuristic in the configuration. However, it is not\n> necessarily easy to see what order the configuration will be read. This\n> means that it is possible for a user to have accidentally enabled both\n> heuristics, and end up only enabling the older compaction heuristic.\n>\n> Modify the code so that we do not clear the other heuristic when we set\n> each heuristic enabled. Then, during diff_setup() when we check the\n> configuration, we will first check the newer indent heuristic. This\n> ensures that we only enable the newer heuristic if both have been\n> enabled.\n>\n> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>\n> ---\n>  diff.c | 10 ++--------\n>  1 file changed, 2 insertions(+), 8 deletions(-)\n\nAlthough I do not think we should spend too much braincycles on this\none (we should rather just removing the older one soonish), I think\nthis patch is going in a wrong direction.  I agree that \"the last\none wins\" is a bit hard to see (until you check with \"git config -l\"\nperhaps) but it at least is predictable.  With this patch, you need\nto KNOW that indent wins over compaction, perhaps by knowing the\norder they were developed, which demands a lot more from the users.\n\nWe probably should just keep one and remove the other.\n\n> diff --git a/diff.c b/diff.c\n> index ec8728362dae..48a5b2797e3d 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -223,16 +223,10 @@ void init_diff_ui_defaults(void)\n>  \n>  int git_diff_heuristic_config(const char *var, const char *value, void *cb)\n>  {\n> +\tif (!strcmp(var, \"diff.indentheuristic\"))\n>  \t\tdiff_indent_heuristic = git_config_bool(var, value);\n> +\tif (!strcmp(var, \"diff.compactionheuristic\"))\n>  \t\tdiff_compaction_heuristic = git_config_bool(var, value);\n>  \treturn 0;\n>  }\n"},{"id":"307952","messageId":"CA+P7+xp=7h7oATwO6vunqO+nfGhvQgiRkwG0P44hC4YLW2MRhA@mail.gmail.com","threadId":"44722","inReplyTo":"xmqq7f6zqr3i.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-17T08:02:40Z","receivedAt":"2016-12-17T08:03:08Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Dec 16, 2016 at 5:30 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Although I do not think we should spend too much braincycles on this\n> one (we should rather just removing the older one soonish), I think\n> this patch is going in a wrong direction.  I agree that \"the last\n> one wins\" is a bit hard to see (until you check with \"git config -l\"\n> perhaps) but it at least is predictable.  With this patch, you need\n> to KNOW that indent wins over compaction, perhaps by knowing the\n> order they were developed, which demands a lot more from the users.\n>\n\nI don't think we have too many config options that interact in this\nway, so I understand that \"last writing of a particular configuration\"\nmakes sense, but interactions between configs is something that would\nhave never occurred to me. I'll send a patch to drop the compaction\nheuristic since I think we're all 100% in agreement that it is\nsuperseded by the new configuration (as no case has been shown where\nthe new one is worse than compaction, and most show it to be better).\n\nThanks,\nJake\n"},{"id":"308263","messageId":"xmqqinqbfz2r.fsf@gitster.mtv.corp.google.com","threadId":"44722","inReplyTo":"CA+P7+xp=7h7oATwO6vunqO+nfGhvQgiRkwG0P44hC4YLW2MRhA@mail.gmail.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-22T21:12:12Z","receivedAt":"2016-12-22T21:12:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.keller@gmail.com> writes:\n\n> I don't think we have too many config options that interact in this\n> way, so I understand that \"last writing of a particular configuration\"\n> makes sense, but interactions between configs is something that would\n> have never occurred to me. I'll send a patch to drop the compaction\n> heuristic since I think we're all 100% in agreement that it is\n> superseded by the new configuration (as no case has been shown where\n> the new one is worse than compaction, and most show it to be better).\n\nIf I recall correctly, we agreed that we'll drop the implementation\nof compaction, but use the name --compaction-heuristics to trigger\nthe new and improved \"indent heuristics\":\n\n    <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>\n\nSo let's do this.\n\n-- >8 --\nSubject: [PATCH] diff: retire the original experimental \"compaction\" heuristics\n\nThis retires the experimental \"compaction\" heuristics but with a\ntwist.  It removes the mention of \"indent\" heuristics, which was a\ncompeting experiment, from everywhere, guts the core logic of the\noriginal \"compaction\" heuristics out and replaces it with the logic\nused by the \"indent\" heuristics.\n\nThe externally visible effect of this change is that people who have\nbeen experimenting by setting diff.compactionHeuristic configuration\nor giving the command line option --compaction-heuristic will start\ngetting the behaviour based on the improved heuristics that used to\nbe called \"indent\" heuristics.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/diff-config.txt            |  6 ++---\n Documentation/diff-heuristic-options.txt |  2 --\n builtin/blame.c                          |  3 +--\n diff.c                                   | 25 ++++-----------------\n git-add--interactive.perl                |  5 +----\n t/t4061-diff-indent.sh                   | 38 ++++++++++++++++----------------\n xdiff/xdiff.h                            |  1 -\n xdiff/xdiffi.c                           | 37 +++----------------------------\n 8 files changed, 30 insertions(+), 87 deletions(-)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex 58f4bd6afa..39fff3aef9 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -171,11 +171,9 @@ diff.tool::\n \n include::mergetools-diff.txt[]\n \n-diff.indentHeuristic::\n diff.compactionHeuristic::\n-\tSet one of these options to `true` to enable one of two\n-\texperimental heuristics that shift diff hunk boundaries to\n-\tmake patches easier to read.\n+\tSet this option to `true` to enable experimental heuristics\n+\tthat shift diff hunk boundaries to make patches easier to read.\n \n diff.algorithm::\n \tChoose a diff algorithm.  The variants are as follows:\ndiff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt\nindex 36cb549df9..3cb024aa22 100644\n--- a/Documentation/diff-heuristic-options.txt\n+++ b/Documentation/diff-heuristic-options.txt\n@@ -1,5 +1,3 @@\n---indent-heuristic::\n---no-indent-heuristic::\n --compaction-heuristic::\n --no-compaction-heuristic::\n \tThese are to help debugging and tuning experimental heuristics\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 4ddfadb71f..395d4011fb 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2596,7 +2596,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t * and are only included here to get included in the \"-h\"\n \t\t * output:\n \t\t */\n-\t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"indent-heuristic\", NULL, NULL, N_(\"Use an experimental indent-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n \t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"compaction-heuristic\", NULL, NULL, N_(\"Use an experimental blank-line-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n \n \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"Spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n@@ -2645,7 +2644,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t}\n parse_done:\n \tno_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);\n-\txdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);\n+\txdl_opts |= revs.diffopt.xdl_opts & XDF_COMPACTION_HEURISTIC;\n \tDIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);\n \targc = parse_options_end(&ctx);\n \ndiff --git a/diff.c b/diff.c\nindex 8981477c43..f1b01f5b1e 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -27,7 +27,6 @@\n #endif\n \n static int diff_detect_rename_default;\n-static int diff_indent_heuristic; /* experimental */\n static int diff_compaction_heuristic; /* experimental */\n static int diff_rename_limit_default = 400;\n static int diff_suppress_blank_empty;\n@@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)\n \n int git_diff_heuristic_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"diff.indentheuristic\")) {\n-\t\tdiff_indent_heuristic = git_config_bool(var, value);\n-\t\tif (diff_indent_heuristic)\n-\t\t\tdiff_compaction_heuristic = 0;\n-\t}\n-\tif (!strcmp(var, \"diff.compactionheuristic\")) {\n+\tif (!strcmp(var, \"diff.compactionheuristic\"))\n \t\tdiff_compaction_heuristic = git_config_bool(var, value);\n-\t\tif (diff_compaction_heuristic)\n-\t\t\tdiff_indent_heuristic = 0;\n-\t}\n \treturn 0;\n }\n \n@@ -3378,9 +3369,7 @@ void diff_setup(struct diff_options *options)\n \toptions->use_color = diff_use_color_default;\n \toptions->detect_rename = diff_detect_rename_default;\n \toptions->xdl_opts |= diff_algorithm;\n-\tif (diff_indent_heuristic)\n-\t\tDIFF_XDL_SET(options, INDENT_HEURISTIC);\n-\telse if (diff_compaction_heuristic)\n+\tif (diff_compaction_heuristic)\n \t\tDIFF_XDL_SET(options, COMPACTION_HEURISTIC);\n \n \toptions->orderfile = diff_order_file_cfg;\n@@ -3876,15 +3865,9 @@ int diff_opt_parse(struct diff_options *options,\n \t\tDIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);\n \telse if (!strcmp(arg, \"--ignore-blank-lines\"))\n \t\tDIFF_XDL_SET(options, IGNORE_BLANK_LINES);\n-\telse if (!strcmp(arg, \"--indent-heuristic\")) {\n-\t\tDIFF_XDL_SET(options, INDENT_HEURISTIC);\n-\t\tDIFF_XDL_CLR(options, COMPACTION_HEURISTIC);\n-\t} else if (!strcmp(arg, \"--no-indent-heuristic\"))\n-\t\tDIFF_XDL_CLR(options, INDENT_HEURISTIC);\n-\telse if (!strcmp(arg, \"--compaction-heuristic\")) {\n+\telse if (!strcmp(arg, \"--compaction-heuristic\"))\n \t\tDIFF_XDL_SET(options, COMPACTION_HEURISTIC);\n-\t\tDIFF_XDL_CLR(options, INDENT_HEURISTIC);\n-\t} else if (!strcmp(arg, \"--no-compaction-heuristic\"))\n+\telse if (!strcmp(arg, \"--no-compaction-heuristic\"))\n \t\tDIFF_XDL_CLR(options, COMPACTION_HEURISTIC);\n \telse if (!strcmp(arg, \"--patience\"))\n \t\toptions->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex ee3d812695..642cce1ac6 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -45,7 +45,6 @@\n my $normal_color = $repo->get_color(\"\", \"reset\");\n \n my $diff_algorithm = $repo->config('diff.algorithm');\n-my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');\n my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');\n my $diff_filter = $repo->config('interactive.difffilter');\n \n@@ -751,9 +750,7 @@ sub parse_diff {\n \tif (defined $diff_algorithm) {\n \t\tsplice @diff_cmd, 1, 0, \"--diff-algorithm=${diff_algorithm}\";\n \t}\n-\tif ($diff_indent_heuristic) {\n-\t\tsplice @diff_cmd, 1, 0, \"--indent-heuristic\";\n-\t} elsif ($diff_compaction_heuristic) {\n+\tif ($diff_compaction_heuristic) {\n \t\tsplice @diff_cmd, 1, 0, \"--compaction-heuristic\";\n \t}\n \tif (defined $patch_mode_revision) {\ndiff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh\nindex 556450609b..30f809d0d3 100755\n--- a/t/t4061-diff-indent.sh\n+++ b/t/t4061-diff-indent.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='Test diff indent heuristic.\n+test_description='Test diff compaction heuristic.\n \n '\n . ./test-lib.sh\n@@ -157,28 +157,28 @@ test_expect_success 'diff: ugly spaces' '\n \tcompare_diff spaces-expect out\n '\n \n-test_expect_success 'diff: nice spaces with --indent-heuristic' '\n-\tgit diff --indent-heuristic old new -- spaces.txt >out-compacted &&\n+test_expect_success 'diff: nice spaces with --compaction-heuristic' '\n+\tgit diff --compaction-heuristic old new -- spaces.txt >out-compacted &&\n \tcompare_diff spaces-compacted-expect out-compacted\n '\n \n-test_expect_success 'diff: nice spaces with diff.indentHeuristic' '\n-\tgit -c diff.indentHeuristic=true diff old new -- spaces.txt >out-compacted2 &&\n+test_expect_success 'diff: nice spaces with diff.compactionHeuristic' '\n+\tgit -c diff.compactionHeuristic=true diff old new -- spaces.txt >out-compacted2 &&\n \tcompare_diff spaces-compacted-expect out-compacted2\n '\n \n-test_expect_success 'diff: --no-indent-heuristic overrides config' '\n-\tgit -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- spaces.txt >out2 &&\n+test_expect_success 'diff: --no-compaction-heuristic overrides config' '\n+\tgit -c diff.compactionHeuristic=true diff --no-compaction-heuristic old new -- spaces.txt >out2 &&\n \tcompare_diff spaces-expect out2\n '\n \n-test_expect_success 'diff: --indent-heuristic with --patience' '\n-\tgit diff --indent-heuristic --patience old new -- spaces.txt >out-compacted3 &&\n+test_expect_success 'diff: --compaction-heuristic with --patience' '\n+\tgit diff --compaction-heuristic --patience old new -- spaces.txt >out-compacted3 &&\n \tcompare_diff spaces-compacted-expect out-compacted3\n '\n \n-test_expect_success 'diff: --indent-heuristic with --histogram' '\n-\tgit diff --indent-heuristic --histogram old new -- spaces.txt >out-compacted4 &&\n+test_expect_success 'diff: --compaction-heuristic with --histogram' '\n+\tgit diff --compaction-heuristic --histogram old new -- spaces.txt >out-compacted4 &&\n \tcompare_diff spaces-compacted-expect out-compacted4\n '\n \n@@ -187,8 +187,8 @@ test_expect_success 'diff: ugly functions' '\n \tcompare_diff functions-expect out\n '\n \n-test_expect_success 'diff: nice functions with --indent-heuristic' '\n-\tgit diff --indent-heuristic old new -- functions.c >out-compacted &&\n+test_expect_success 'diff: nice functions with --compaction-heuristic' '\n+\tgit diff --compaction-heuristic old new -- functions.c >out-compacted &&\n \tcompare_diff functions-compacted-expect out-compacted\n '\n \n@@ -197,18 +197,18 @@ test_expect_success 'blame: ugly spaces' '\n \tcompare_blame spaces-expect out-blame\n '\n \n-test_expect_success 'blame: nice spaces with --indent-heuristic' '\n-\tgit blame --indent-heuristic old..new -- spaces.txt >out-blame-compacted &&\n+test_expect_success 'blame: nice spaces with --compaction-heuristic' '\n+\tgit blame --compaction-heuristic old..new -- spaces.txt >out-blame-compacted &&\n \tcompare_blame spaces-compacted-expect out-blame-compacted\n '\n \n-test_expect_success 'blame: nice spaces with diff.indentHeuristic' '\n-\tgit -c diff.indentHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&\n+test_expect_success 'blame: nice spaces with diff.compactionHeuristic' '\n+\tgit -c diff.compactionHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&\n \tcompare_blame spaces-compacted-expect out-blame-compacted2\n '\n \n-test_expect_success 'blame: --no-indent-heuristic overrides config' '\n-\tgit -c diff.indentHeuristic=true blame --no-indent-heuristic old..new -- spaces.txt >out-blame2 &&\n+test_expect_success 'blame: --no-compaction-heuristic overrides config' '\n+\tgit -c diff.compactionHeuristic=true blame --no-compaction-heuristic old..new -- spaces.txt >out-blame2 &&\n \tgit blame old..new -- spaces.txt >out-blame &&\n \tcompare_blame spaces-expect out-blame2\n '\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 8db16d4ae6..7423f77fc8 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -42,7 +42,6 @@ extern \"C\" {\n #define XDF_IGNORE_BLANK_LINES (1 << 7)\n \n #define XDF_COMPACTION_HEURISTIC (1 << 8)\n-#define XDF_INDENT_HEURISTIC (1 << 9)\n \n #define XDL_EMIT_FUNCNAMES (1 << 0)\n #define XDL_EMIT_FUNCCONTEXT (1 << 2)\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex 760fbb6db7..2131ea4920 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,\n }\n \n \n-static int is_blank_line(xrecord_t *rec, long flags)\n-{\n-\treturn xdl_blankline(rec->ptr, rec->size, flags);\n-}\n-\n static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)\n {\n \treturn (rec1->ha == rec2->ha &&\n@@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \tstruct xdlgroup g, go;\n \tlong earliest_end, end_matching_other;\n \tlong groupsize;\n-\tunsigned int blank_lines;\n \n \tgroup_init(xdf, &g);\n \tgroup_init(xdfo, &go);\n@@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t */\n \t\t\tend_matching_other = -1;\n \n-\t\t\t/*\n-\t\t\t * Boolean value that records whether there are any blank\n-\t\t\t * lines that could be made to be the last line of this\n-\t\t\t * group.\n-\t\t\t */\n-\t\t\tblank_lines = 0;\n-\n \t\t\t/* Shift the group backward as much as possible: */\n \t\t\twhile (!group_slide_up(xdf, &g, flags))\n \t\t\t\tif (group_previous(xdfo, &go))\n@@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \n \t\t\t/* Now shift the group forward as far as possible: */\n \t\t\twhile (1) {\n-\t\t\t\tif (!blank_lines)\n-\t\t\t\t\tblank_lines = is_blank_line(\n-\t\t\t\t\t\t\txdf->recs[g.end - 1],\n-\t\t\t\t\t\t\tflags);\n-\n \t\t\t\tif (group_slide_down(xdf, &g, flags))\n \t\t\t\t\tbreak;\n \t\t\t\tif (group_next(xdfo, &go))\n@@ -906,24 +888,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t\tif (group_previous(xdfo, &go))\n \t\t\t\t\txdl_bug(\"group sync broken sliding to match\");\n \t\t\t}\n-\t\t} else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {\n+\t\t} else if (flags & XDF_COMPACTION_HEURISTIC) {\n \t\t\t/*\n-\t\t\t * Compaction heuristic: if it is possible to shift the\n-\t\t\t * group to make its bottom line a blank line, do so.\n+\t\t\t * Heuristic based on the indentation level.\n \t\t\t *\n-\t\t\t * As we already shifted the group forward as far as\n-\t\t\t * possible in the earlier loop, we only need to handle\n-\t\t\t * backward shifts, not forward ones.\n-\t\t\t */\n-\t\t\twhile (!is_blank_line(xdf->recs[g.end - 1], flags)) {\n-\t\t\t\tif (group_slide_up(xdf, &g, flags))\n-\t\t\t\t\txdl_bug(\"blank line disappeared\");\n-\t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\txdl_bug(\"group sync broken sliding to blank line\");\n-\t\t\t}\n-\t\t} else if (flags & XDF_INDENT_HEURISTIC) {\n-\t\t\t/*\n-\t\t\t * Indent heuristic: a group of pure add/delete lines\n+\t\t\t * A group of pure add/delete lines\n \t\t\t * implies two splits, one between the end of the \"before\"\n \t\t\t * context and the start of the group, and another between\n \t\t\t * the end of the group and the beginning of the \"after\"\n-- \n2.11.0-448-g9a11f8a62b\n\n"},{"id":"308272","messageId":"CA+P7+xrkp-qiUVmfeLUcaMP-RSDbH4u3vCjVoQN8=mhz25Cd3A@mail.gmail.com","threadId":"44722","inReplyTo":"xmqqinqbfz2r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-22T21:41:38Z","receivedAt":"2016-12-22T21:42:06Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Dec 22, 2016 at 1:12 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jacob Keller <jacob.keller@gmail.com> writes:\n>\n>> I don't think we have too many config options that interact in this\n>> way, so I understand that \"last writing of a particular configuration\"\n>> makes sense, but interactions between configs is something that would\n>> have never occurred to me. I'll send a patch to drop the compaction\n>> heuristic since I think we're all 100% in agreement that it is\n>> superseded by the new configuration (as no case has been shown where\n>> the new one is worse than compaction, and most show it to be better).\n>\n> If I recall correctly, we agreed that we'll drop the implementation\n> of compaction, but use the name --compaction-heuristics to trigger\n> the new and improved \"indent heuristics\":\n>\n>     <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>\n>\n\nThat sounds familiar.\n\n> So let's do this.\n>\n> -- >8 --\n> Subject: [PATCH] diff: retire the original experimental \"compaction\" heuristics\n>\n> This retires the experimental \"compaction\" heuristics but with a\n> twist.  It removes the mention of \"indent\" heuristics, which was a\n> competing experiment, from everywhere, guts the core logic of the\n> original \"compaction\" heuristics out and replaces it with the logic\n> used by the \"indent\" heuristics.\n>\n> The externally visible effect of this change is that people who have\n> been experimenting by setting diff.compactionHeuristic configuration\n> or giving the command line option --compaction-heuristic will start\n> getting the behaviour based on the improved heuristics that used to\n> be called \"indent\" heuristics.\n>\n> Suggested-by: Jeff King <peff@peff.net>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  Documentation/diff-config.txt            |  6 ++---\n>  Documentation/diff-heuristic-options.txt |  2 --\n>  builtin/blame.c                          |  3 +--\n>  diff.c                                   | 25 ++++-----------------\n>  git-add--interactive.perl                |  5 +----\n>  t/t4061-diff-indent.sh                   | 38 ++++++++++++++++----------------\n>  xdiff/xdiff.h                            |  1 -\n>  xdiff/xdiffi.c                           | 37 +++----------------------------\n>  8 files changed, 30 insertions(+), 87 deletions(-)\n>\n> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\n> index 58f4bd6afa..39fff3aef9 100644\n> --- a/Documentation/diff-config.txt\n> +++ b/Documentation/diff-config.txt\n> @@ -171,11 +171,9 @@ diff.tool::\n>\n>  include::mergetools-diff.txt[]\n>\n> -diff.indentHeuristic::\n>  diff.compactionHeuristic::\n> -       Set one of these options to `true` to enable one of two\n> -       experimental heuristics that shift diff hunk boundaries to\n> -       make patches easier to read.\n> +       Set this option to `true` to enable experimental heuristics\n> +       that shift diff hunk boundaries to make patches easier to read.\n>\n>  diff.algorithm::\n>         Choose a diff algorithm.  The variants are as follows:\n> diff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt\n> index 36cb549df9..3cb024aa22 100644\n> --- a/Documentation/diff-heuristic-options.txt\n> +++ b/Documentation/diff-heuristic-options.txt\n> @@ -1,5 +1,3 @@\n> ---indent-heuristic::\n> ---no-indent-heuristic::\n>  --compaction-heuristic::\n>  --no-compaction-heuristic::\n>         These are to help debugging and tuning experimental heuristics\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 4ddfadb71f..395d4011fb 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -2596,7 +2596,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>                  * and are only included here to get included in the \"-h\"\n>                  * output:\n>                  */\n> -               { OPTION_LOWLEVEL_CALLBACK, 0, \"indent-heuristic\", NULL, NULL, N_(\"Use an experimental indent-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n>                 { OPTION_LOWLEVEL_CALLBACK, 0, \"compaction-heuristic\", NULL, NULL, N_(\"Use an experimental blank-line-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n>\n\nThe unchanged context line should have its description re-worded to\nsomething like \"Use an experimental heuristic to improve diffs\" as it\nno longer uses only blank lines.\n\nEverything else looked correct.\n\nThanks,\nJake\n"},{"id":"308281","messageId":"xmqqpokjeg9i.fsf@gitster.mtv.corp.google.com","threadId":"44722","inReplyTo":"CA+P7+xrkp-qiUVmfeLUcaMP-RSDbH4u3vCjVoQN8=mhz25Cd3A@mail.gmail.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-22T22:43:53Z","receivedAt":"2016-12-22T22:44:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jacob Keller <jacob.keller@gmail.com> writes:\n\n>>                 { OPTION_LOWLEVEL_CALLBACK, 0, \"compaction-heuristic\", NULL, NULL, N_(\"Use an experimental blank-line-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n>>\n>\n> The unchanged context line should have its description re-worded to\n> something like \"Use an experimental heuristic to improve diffs\" as it\n> no longer uses only blank lines.\n\nThanks.  The final copy I pushed out has that change.\n\n"},{"id":"308294","messageId":"20161223072201.zw2lwkdcs6qmb4rp@sigill.intra.peff.net","threadId":"44722","inReplyTo":"xmqqinqbfz2r.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-23T07:22:01Z","receivedAt":"2016-12-23T07:22:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 22, 2016 at 01:12:12PM -0800, Junio C Hamano wrote:\n\n> Jacob Keller <jacob.keller@gmail.com> writes:\n> \n> > I don't think we have too many config options that interact in this\n> > way, so I understand that \"last writing of a particular configuration\"\n> > makes sense, but interactions between configs is something that would\n> > have never occurred to me. I'll send a patch to drop the compaction\n> > heuristic since I think we're all 100% in agreement that it is\n> > superseded by the new configuration (as no case has been shown where\n> > the new one is worse than compaction, and most show it to be better).\n> \n> If I recall correctly, we agreed that we'll drop the implementation\n> of compaction, but use the name --compaction-heuristics to trigger\n> the new and improved \"indent heuristics\":\n> \n>     <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>\n\nFWIW, I was swayed in the other direction by later messages in the\nthread. Especially your noting that the \"compaction\" name has always\nbeen labeled experimental, and Michael's argument in:\n\n  http://public-inbox.org/git/8dbbd28b-af60-5e66-ae27-d7cddca233dc@alum.mit.edu/\n\nI.e., we could keep calling it \"--indent-heuristic\", and probably drop\nthe other heuristic entirely as a failed experiment.\n\nI can live with it either way, but since I am being quoted as the source\nof the suggestion, I feel like that's an invitation to add my 2 cents. :)\n\nLiberal quoting below since I am adding Michael to the cc list.\n\n-Peff\n\n> So let's do this.\n> \n> -- >8 --\n> Subject: [PATCH] diff: retire the original experimental \"compaction\" heuristics\n> \n> This retires the experimental \"compaction\" heuristics but with a\n> twist.  It removes the mention of \"indent\" heuristics, which was a\n> competing experiment, from everywhere, guts the core logic of the\n> original \"compaction\" heuristics out and replaces it with the logic\n> used by the \"indent\" heuristics.\n> \n> The externally visible effect of this change is that people who have\n> been experimenting by setting diff.compactionHeuristic configuration\n> or giving the command line option --compaction-heuristic will start\n> getting the behaviour based on the improved heuristics that used to\n> be called \"indent\" heuristics.\n> \n> Suggested-by: Jeff King <peff@peff.net>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>  Documentation/diff-config.txt            |  6 ++---\n>  Documentation/diff-heuristic-options.txt |  2 --\n>  builtin/blame.c                          |  3 +--\n>  diff.c                                   | 25 ++++-----------------\n>  git-add--interactive.perl                |  5 +----\n>  t/t4061-diff-indent.sh                   | 38 ++++++++++++++++----------------\n>  xdiff/xdiff.h                            |  1 -\n>  xdiff/xdiffi.c                           | 37 +++----------------------------\n>  8 files changed, 30 insertions(+), 87 deletions(-)\n> \n> diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\n> index 58f4bd6afa..39fff3aef9 100644\n> --- a/Documentation/diff-config.txt\n> +++ b/Documentation/diff-config.txt\n> @@ -171,11 +171,9 @@ diff.tool::\n>  \n>  include::mergetools-diff.txt[]\n>  \n> -diff.indentHeuristic::\n>  diff.compactionHeuristic::\n> -\tSet one of these options to `true` to enable one of two\n> -\texperimental heuristics that shift diff hunk boundaries to\n> -\tmake patches easier to read.\n> +\tSet this option to `true` to enable experimental heuristics\n> +\tthat shift diff hunk boundaries to make patches easier to read.\n>  \n>  diff.algorithm::\n>  \tChoose a diff algorithm.  The variants are as follows:\n> diff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt\n> index 36cb549df9..3cb024aa22 100644\n> --- a/Documentation/diff-heuristic-options.txt\n> +++ b/Documentation/diff-heuristic-options.txt\n> @@ -1,5 +1,3 @@\n> ---indent-heuristic::\n> ---no-indent-heuristic::\n>  --compaction-heuristic::\n>  --no-compaction-heuristic::\n>  \tThese are to help debugging and tuning experimental heuristics\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 4ddfadb71f..395d4011fb 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -2596,7 +2596,6 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>  \t\t * and are only included here to get included in the \"-h\"\n>  \t\t * output:\n>  \t\t */\n> -\t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"indent-heuristic\", NULL, NULL, N_(\"Use an experimental indent-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n>  \t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"compaction-heuristic\", NULL, NULL, N_(\"Use an experimental blank-line-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n>  \n>  \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"Spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n> @@ -2645,7 +2644,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n>  \t}\n>  parse_done:\n>  \tno_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);\n> -\txdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);\n> +\txdl_opts |= revs.diffopt.xdl_opts & XDF_COMPACTION_HEURISTIC;\n>  \tDIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);\n>  \targc = parse_options_end(&ctx);\n>  \n> diff --git a/diff.c b/diff.c\n> index 8981477c43..f1b01f5b1e 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -27,7 +27,6 @@\n>  #endif\n>  \n>  static int diff_detect_rename_default;\n> -static int diff_indent_heuristic; /* experimental */\n>  static int diff_compaction_heuristic; /* experimental */\n>  static int diff_rename_limit_default = 400;\n>  static int diff_suppress_blank_empty;\n> @@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)\n>  \n>  int git_diff_heuristic_config(const char *var, const char *value, void *cb)\n>  {\n> -\tif (!strcmp(var, \"diff.indentheuristic\")) {\n> -\t\tdiff_indent_heuristic = git_config_bool(var, value);\n> -\t\tif (diff_indent_heuristic)\n> -\t\t\tdiff_compaction_heuristic = 0;\n> -\t}\n> -\tif (!strcmp(var, \"diff.compactionheuristic\")) {\n> +\tif (!strcmp(var, \"diff.compactionheuristic\"))\n>  \t\tdiff_compaction_heuristic = git_config_bool(var, value);\n> -\t\tif (diff_compaction_heuristic)\n> -\t\t\tdiff_indent_heuristic = 0;\n> -\t}\n>  \treturn 0;\n>  }\n>  \n> @@ -3378,9 +3369,7 @@ void diff_setup(struct diff_options *options)\n>  \toptions->use_color = diff_use_color_default;\n>  \toptions->detect_rename = diff_detect_rename_default;\n>  \toptions->xdl_opts |= diff_algorithm;\n> -\tif (diff_indent_heuristic)\n> -\t\tDIFF_XDL_SET(options, INDENT_HEURISTIC);\n> -\telse if (diff_compaction_heuristic)\n> +\tif (diff_compaction_heuristic)\n>  \t\tDIFF_XDL_SET(options, COMPACTION_HEURISTIC);\n>  \n>  \toptions->orderfile = diff_order_file_cfg;\n> @@ -3876,15 +3865,9 @@ int diff_opt_parse(struct diff_options *options,\n>  \t\tDIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);\n>  \telse if (!strcmp(arg, \"--ignore-blank-lines\"))\n>  \t\tDIFF_XDL_SET(options, IGNORE_BLANK_LINES);\n> -\telse if (!strcmp(arg, \"--indent-heuristic\")) {\n> -\t\tDIFF_XDL_SET(options, INDENT_HEURISTIC);\n> -\t\tDIFF_XDL_CLR(options, COMPACTION_HEURISTIC);\n> -\t} else if (!strcmp(arg, \"--no-indent-heuristic\"))\n> -\t\tDIFF_XDL_CLR(options, INDENT_HEURISTIC);\n> -\telse if (!strcmp(arg, \"--compaction-heuristic\")) {\n> +\telse if (!strcmp(arg, \"--compaction-heuristic\"))\n>  \t\tDIFF_XDL_SET(options, COMPACTION_HEURISTIC);\n> -\t\tDIFF_XDL_CLR(options, INDENT_HEURISTIC);\n> -\t} else if (!strcmp(arg, \"--no-compaction-heuristic\"))\n> +\telse if (!strcmp(arg, \"--no-compaction-heuristic\"))\n>  \t\tDIFF_XDL_CLR(options, COMPACTION_HEURISTIC);\n>  \telse if (!strcmp(arg, \"--patience\"))\n>  \t\toptions->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index ee3d812695..642cce1ac6 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -45,7 +45,6 @@\n>  my $normal_color = $repo->get_color(\"\", \"reset\");\n>  \n>  my $diff_algorithm = $repo->config('diff.algorithm');\n> -my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');\n>  my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');\n>  my $diff_filter = $repo->config('interactive.difffilter');\n>  \n> @@ -751,9 +750,7 @@ sub parse_diff {\n>  \tif (defined $diff_algorithm) {\n>  \t\tsplice @diff_cmd, 1, 0, \"--diff-algorithm=${diff_algorithm}\";\n>  \t}\n> -\tif ($diff_indent_heuristic) {\n> -\t\tsplice @diff_cmd, 1, 0, \"--indent-heuristic\";\n> -\t} elsif ($diff_compaction_heuristic) {\n> +\tif ($diff_compaction_heuristic) {\n>  \t\tsplice @diff_cmd, 1, 0, \"--compaction-heuristic\";\n>  \t}\n>  \tif (defined $patch_mode_revision) {\n> diff --git a/t/t4061-diff-indent.sh b/t/t4061-diff-indent.sh\n> index 556450609b..30f809d0d3 100755\n> --- a/t/t4061-diff-indent.sh\n> +++ b/t/t4061-diff-indent.sh\n> @@ -1,6 +1,6 @@\n>  #!/bin/sh\n>  \n> -test_description='Test diff indent heuristic.\n> +test_description='Test diff compaction heuristic.\n>  \n>  '\n>  . ./test-lib.sh\n> @@ -157,28 +157,28 @@ test_expect_success 'diff: ugly spaces' '\n>  \tcompare_diff spaces-expect out\n>  '\n>  \n> -test_expect_success 'diff: nice spaces with --indent-heuristic' '\n> -\tgit diff --indent-heuristic old new -- spaces.txt >out-compacted &&\n> +test_expect_success 'diff: nice spaces with --compaction-heuristic' '\n> +\tgit diff --compaction-heuristic old new -- spaces.txt >out-compacted &&\n>  \tcompare_diff spaces-compacted-expect out-compacted\n>  '\n>  \n> -test_expect_success 'diff: nice spaces with diff.indentHeuristic' '\n> -\tgit -c diff.indentHeuristic=true diff old new -- spaces.txt >out-compacted2 &&\n> +test_expect_success 'diff: nice spaces with diff.compactionHeuristic' '\n> +\tgit -c diff.compactionHeuristic=true diff old new -- spaces.txt >out-compacted2 &&\n>  \tcompare_diff spaces-compacted-expect out-compacted2\n>  '\n>  \n> -test_expect_success 'diff: --no-indent-heuristic overrides config' '\n> -\tgit -c diff.indentHeuristic=true diff --no-indent-heuristic old new -- spaces.txt >out2 &&\n> +test_expect_success 'diff: --no-compaction-heuristic overrides config' '\n> +\tgit -c diff.compactionHeuristic=true diff --no-compaction-heuristic old new -- spaces.txt >out2 &&\n>  \tcompare_diff spaces-expect out2\n>  '\n>  \n> -test_expect_success 'diff: --indent-heuristic with --patience' '\n> -\tgit diff --indent-heuristic --patience old new -- spaces.txt >out-compacted3 &&\n> +test_expect_success 'diff: --compaction-heuristic with --patience' '\n> +\tgit diff --compaction-heuristic --patience old new -- spaces.txt >out-compacted3 &&\n>  \tcompare_diff spaces-compacted-expect out-compacted3\n>  '\n>  \n> -test_expect_success 'diff: --indent-heuristic with --histogram' '\n> -\tgit diff --indent-heuristic --histogram old new -- spaces.txt >out-compacted4 &&\n> +test_expect_success 'diff: --compaction-heuristic with --histogram' '\n> +\tgit diff --compaction-heuristic --histogram old new -- spaces.txt >out-compacted4 &&\n>  \tcompare_diff spaces-compacted-expect out-compacted4\n>  '\n>  \n> @@ -187,8 +187,8 @@ test_expect_success 'diff: ugly functions' '\n>  \tcompare_diff functions-expect out\n>  '\n>  \n> -test_expect_success 'diff: nice functions with --indent-heuristic' '\n> -\tgit diff --indent-heuristic old new -- functions.c >out-compacted &&\n> +test_expect_success 'diff: nice functions with --compaction-heuristic' '\n> +\tgit diff --compaction-heuristic old new -- functions.c >out-compacted &&\n>  \tcompare_diff functions-compacted-expect out-compacted\n>  '\n>  \n> @@ -197,18 +197,18 @@ test_expect_success 'blame: ugly spaces' '\n>  \tcompare_blame spaces-expect out-blame\n>  '\n>  \n> -test_expect_success 'blame: nice spaces with --indent-heuristic' '\n> -\tgit blame --indent-heuristic old..new -- spaces.txt >out-blame-compacted &&\n> +test_expect_success 'blame: nice spaces with --compaction-heuristic' '\n> +\tgit blame --compaction-heuristic old..new -- spaces.txt >out-blame-compacted &&\n>  \tcompare_blame spaces-compacted-expect out-blame-compacted\n>  '\n>  \n> -test_expect_success 'blame: nice spaces with diff.indentHeuristic' '\n> -\tgit -c diff.indentHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&\n> +test_expect_success 'blame: nice spaces with diff.compactionHeuristic' '\n> +\tgit -c diff.compactionHeuristic=true blame old..new -- spaces.txt >out-blame-compacted2 &&\n>  \tcompare_blame spaces-compacted-expect out-blame-compacted2\n>  '\n>  \n> -test_expect_success 'blame: --no-indent-heuristic overrides config' '\n> -\tgit -c diff.indentHeuristic=true blame --no-indent-heuristic old..new -- spaces.txt >out-blame2 &&\n> +test_expect_success 'blame: --no-compaction-heuristic overrides config' '\n> +\tgit -c diff.compactionHeuristic=true blame --no-compaction-heuristic old..new -- spaces.txt >out-blame2 &&\n>  \tgit blame old..new -- spaces.txt >out-blame &&\n>  \tcompare_blame spaces-expect out-blame2\n>  '\n> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\n> index 8db16d4ae6..7423f77fc8 100644\n> --- a/xdiff/xdiff.h\n> +++ b/xdiff/xdiff.h\n> @@ -42,7 +42,6 @@ extern \"C\" {\n>  #define XDF_IGNORE_BLANK_LINES (1 << 7)\n>  \n>  #define XDF_COMPACTION_HEURISTIC (1 << 8)\n> -#define XDF_INDENT_HEURISTIC (1 << 9)\n>  \n>  #define XDL_EMIT_FUNCNAMES (1 << 0)\n>  #define XDL_EMIT_FUNCCONTEXT (1 << 2)\n> diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\n> index 760fbb6db7..2131ea4920 100644\n> --- a/xdiff/xdiffi.c\n> +++ b/xdiff/xdiffi.c\n> @@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,\n>  }\n>  \n>  \n> -static int is_blank_line(xrecord_t *rec, long flags)\n> -{\n> -\treturn xdl_blankline(rec->ptr, rec->size, flags);\n> -}\n> -\n>  static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)\n>  {\n>  \treturn (rec1->ha == rec2->ha &&\n> @@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n>  \tstruct xdlgroup g, go;\n>  \tlong earliest_end, end_matching_other;\n>  \tlong groupsize;\n> -\tunsigned int blank_lines;\n>  \n>  \tgroup_init(xdf, &g);\n>  \tgroup_init(xdfo, &go);\n> @@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n>  \t\t\t */\n>  \t\t\tend_matching_other = -1;\n>  \n> -\t\t\t/*\n> -\t\t\t * Boolean value that records whether there are any blank\n> -\t\t\t * lines that could be made to be the last line of this\n> -\t\t\t * group.\n> -\t\t\t */\n> -\t\t\tblank_lines = 0;\n> -\n>  \t\t\t/* Shift the group backward as much as possible: */\n>  \t\t\twhile (!group_slide_up(xdf, &g, flags))\n>  \t\t\t\tif (group_previous(xdfo, &go))\n> @@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n>  \n>  \t\t\t/* Now shift the group forward as far as possible: */\n>  \t\t\twhile (1) {\n> -\t\t\t\tif (!blank_lines)\n> -\t\t\t\t\tblank_lines = is_blank_line(\n> -\t\t\t\t\t\t\txdf->recs[g.end - 1],\n> -\t\t\t\t\t\t\tflags);\n> -\n>  \t\t\t\tif (group_slide_down(xdf, &g, flags))\n>  \t\t\t\t\tbreak;\n>  \t\t\t\tif (group_next(xdfo, &go))\n> @@ -906,24 +888,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n>  \t\t\t\tif (group_previous(xdfo, &go))\n>  \t\t\t\t\txdl_bug(\"group sync broken sliding to match\");\n>  \t\t\t}\n> -\t\t} else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {\n> +\t\t} else if (flags & XDF_COMPACTION_HEURISTIC) {\n>  \t\t\t/*\n> -\t\t\t * Compaction heuristic: if it is possible to shift the\n> -\t\t\t * group to make its bottom line a blank line, do so.\n> +\t\t\t * Heuristic based on the indentation level.\n>  \t\t\t *\n> -\t\t\t * As we already shifted the group forward as far as\n> -\t\t\t * possible in the earlier loop, we only need to handle\n> -\t\t\t * backward shifts, not forward ones.\n> -\t\t\t */\n> -\t\t\twhile (!is_blank_line(xdf->recs[g.end - 1], flags)) {\n> -\t\t\t\tif (group_slide_up(xdf, &g, flags))\n> -\t\t\t\t\txdl_bug(\"blank line disappeared\");\n> -\t\t\t\tif (group_previous(xdfo, &go))\n> -\t\t\t\t\txdl_bug(\"group sync broken sliding to blank line\");\n> -\t\t\t}\n> -\t\t} else if (flags & XDF_INDENT_HEURISTIC) {\n> -\t\t\t/*\n> -\t\t\t * Indent heuristic: a group of pure add/delete lines\n> +\t\t\t * A group of pure add/delete lines\n>  \t\t\t * implies two splits, one between the end of the \"before\"\n>  \t\t\t * context and the start of the group, and another between\n>  \t\t\t * the end of the group and the beginning of the \"after\"\n> -- \n> 2.11.0-448-g9a11f8a62b\n> \n"},{"id":"308295","messageId":"CA+P7+xrWsCkABzpSkYJ4fb2_JijmUx=Sf4Hgsr6Z+k=_GogE_Q@mail.gmail.com","threadId":"44722","inReplyTo":"20161223072201.zw2lwkdcs6qmb4rp@sigill.intra.peff.net","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-23T08:12:13Z","receivedAt":"2016-12-23T08:15:22Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Thu, Dec 22, 2016 at 11:22 PM, Jeff King <peff@peff.net> wrote:\n> On Thu, Dec 22, 2016 at 01:12:12PM -0800, Junio C Hamano wrote:\n>\n>> Jacob Keller <jacob.keller@gmail.com> writes:\n>>\n>> > I don't think we have too many config options that interact in this\n>> > way, so I understand that \"last writing of a particular configuration\"\n>> > makes sense, but interactions between configs is something that would\n>> > have never occurred to me. I'll send a patch to drop the compaction\n>> > heuristic since I think we're all 100% in agreement that it is\n>> > superseded by the new configuration (as no case has been shown where\n>> > the new one is worse than compaction, and most show it to be better).\n>>\n>> If I recall correctly, we agreed that we'll drop the implementation\n>> of compaction, but use the name --compaction-heuristics to trigger\n>> the new and improved \"indent heuristics\":\n>>\n>>     <20161101205916.d74n6lhgp2hexpzr@sigill.intra.peff.net>\n>\n> FWIW, I was swayed in the other direction by later messages in the\n> thread. Especially your noting that the \"compaction\" name has always\n> been labeled experimental, and Michael's argument in:\n>\n>   http://public-inbox.org/git/8dbbd28b-af60-5e66-ae27-d7cddca233dc@alum.mit.edu/\n>\n> I.e., we could keep calling it \"--indent-heuristic\", and probably drop\n> the other heuristic entirely as a failed experiment.\n>\n> I can live with it either way, but since I am being quoted as the source\n> of the suggestion, I feel like that's an invitation to add my 2 cents. :)\n>\n> Liberal quoting below since I am adding Michael to the cc list.\n>\n> -Peff\n>\n\nI actually would prefer that we just say \"this is the default now\" and\nprovide some knob \"no-indent-heuristic\" or \"no-compaction-heuristic\"\nand go with that, I think, since I am pretty sure we're all in\nagreement that the heuristic is an improvement in almost every case,\ncertainly all the ones we've found. It's at least not worse in any\ncase I've seen, and is usually better.\n\nThoughts? I don't have a super strong opinion about which name we went\nwith for the knob.\n\nThanks,\nJake\n"},{"id":"308300","messageId":"20161223161917.4a352c2wzerj5uyz@sigill.intra.peff.net","threadId":"44722","inReplyTo":"CA+P7+xrWsCkABzpSkYJ4fb2_JijmUx=Sf4Hgsr6Z+k=_GogE_Q@mail.gmail.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-23T16:19:18Z","receivedAt":"2016-12-23T16:19:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 23, 2016 at 12:12:13AM -0800, Jacob Keller wrote:\n\n> I actually would prefer that we just say \"this is the default now\" and\n> provide some knob \"no-indent-heuristic\" or \"no-compaction-heuristic\"\n> and go with that, I think, since I am pretty sure we're all in\n> agreement that the heuristic is an improvement in almost every case,\n> certainly all the ones we've found. It's at least not worse in any\n> case I've seen, and is usually better.\n> \n> Thoughts? I don't have a super strong opinion about which name we went\n> with for the knob.\n\nYes, I think we should also make --indent-heuristic the default. That's\ntechnically orthogonal to the name, but I agree the name becomes a lot\nless important when it is just on by default.\n\n-Peff\n"},{"id":"308304","messageId":"xmqqh95uedzu.fsf@gitster.mtv.corp.google.com","threadId":"44722","inReplyTo":"20161223161917.4a352c2wzerj5uyz@sigill.intra.peff.net","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-23T17:45:09Z","receivedAt":"2016-12-23T17:52:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Fri, Dec 23, 2016 at 12:12:13AM -0800, Jacob Keller wrote:\n>\n>> I actually would prefer that we just say \"this is the default now\" and\n>> provide some knob \"no-indent-heuristic\" or \"no-compaction-heuristic\"\n>> and go with that, I think, since I am pretty sure we're all in\n>> agreement that the heuristic is an improvement in almost every case,\n>> certainly all the ones we've found. It's at least not worse in any\n>> case I've seen, and is usually better.\n>> \n>> Thoughts? I don't have a super strong opinion about which name we went\n>> with for the knob.\n>\n> Yes, I think we should also make --indent-heuristic the default. That's\n> technically orthogonal to the name, but I agree the name becomes a lot\n> less important when it is just on by default.\n\nYes, I agree that will be the endgame, but one step at the time, and\nalso removing one of them is orthogonal that we would want to do\nsooner rather than later.  So the step represented by the patch\nunder discussion is the first one among the two towards the final\nstate.\n\nI guess both you and Michael are in favor of just removing compaction\nvariant without any renames, so let me prepare a reroll and queue\nthat instead.  We can flip the default perhaps one release later.\n\nThanks.\n"},{"id":"308308","messageId":"xmqq8tr6e46o.fsf@gitster.mtv.corp.google.com","threadId":"44722","inReplyTo":"xmqqh95uedzu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-12-23T21:17:03Z","receivedAt":"2016-12-23T21:17:15Z","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> I guess both you and Michael are in favor of just removing compaction\n> variant without any renames, so let me prepare a reroll and queue\n> that instead.  We can flip the default perhaps one release later.\n\n-- >8 --\nSubject: [PATCH] diff: retire \"compaction\" heuristics\n\nWhen a patch inserts a block of lines, whose last lines are the\nsame as the existing lines that appear before the inserted block,\n\"git diff\" can choose any place between these existing lines as the\nboundary between the pre-context and the added lines (adjusting the\nend of the inserted block as appropriate) to come up with variants\nof the same patch, and some variants are easier to read than others.\n\nWe have been trying to improve the choice of this boundary, and Git\n2.11 shipped with an experimental \"compaction-heuristic\".  Since\nthen another attempt to improve the logic further resulted in a new\n\"indent-heuristic\" logic.  It is agreed that the latter gives better\nresult overall, and the former outlived its usefulness.\n\nRetire \"compaction\", and keep \"indent\" as an experimental feature.\nThe latter hopefully will be turned on by default in a future\nrelease, but that should be done as a separate step.\n\nSuggested-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/diff-config.txt            |  6 ++----\n Documentation/diff-heuristic-options.txt |  2 --\n builtin/blame.c                          |  5 ++---\n diff.c                                   | 23 +++-------------------\n git-add--interactive.perl                |  3 ---\n xdiff/xdiff.h                            |  3 +--\n xdiff/xdiffi.c                           | 33 --------------------------------\n 7 files changed, 8 insertions(+), 67 deletions(-)\n\ndiff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt\nindex 58f4bd6afa..d8570f2a75 100644\n--- a/Documentation/diff-config.txt\n+++ b/Documentation/diff-config.txt\n@@ -172,10 +172,8 @@ diff.tool::\n include::mergetools-diff.txt[]\n \n diff.indentHeuristic::\n-diff.compactionHeuristic::\n-\tSet one of these options to `true` to enable one of two\n-\texperimental heuristics that shift diff hunk boundaries to\n-\tmake patches easier to read.\n+\tSet this option to `true` to enable experimental heuristics\n+\tthat shift diff hunk boundaries to make patches easier to read.\n \n diff.algorithm::\n \tChoose a diff algorithm.  The variants are as follows:\ndiff --git a/Documentation/diff-heuristic-options.txt b/Documentation/diff-heuristic-options.txt\nindex 36cb549df9..d4f3d95505 100644\n--- a/Documentation/diff-heuristic-options.txt\n+++ b/Documentation/diff-heuristic-options.txt\n@@ -1,7 +1,5 @@\n --indent-heuristic::\n --no-indent-heuristic::\n---compaction-heuristic::\n---no-compaction-heuristic::\n \tThese are to help debugging and tuning experimental heuristics\n \t(which are off by default) that shift diff hunk boundaries to\n \tmake patches easier to read.\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 4ddfadb71f..ab54a5c1f4 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2596,8 +2596,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t * and are only included here to get included in the \"-h\"\n \t\t * output:\n \t\t */\n-\t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"indent-heuristic\", NULL, NULL, N_(\"Use an experimental indent-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n-\t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"compaction-heuristic\", NULL, NULL, N_(\"Use an experimental blank-line-based heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n+\t\t{ OPTION_LOWLEVEL_CALLBACK, 0, \"indent-heuristic\", NULL, NULL, N_(\"Use an experimental heuristic to improve diffs\"), PARSE_OPT_NOARG, parse_opt_unknown_cb },\n \n \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"Spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n \t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"Use revisions from <file> instead of calling git-rev-list\")),\n@@ -2645,7 +2644,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t}\n parse_done:\n \tno_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);\n-\txdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);\n+\txdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;\n \tDIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);\n \targc = parse_options_end(&ctx);\n \ndiff --git a/diff.c b/diff.c\nindex 8981477c43..741ce8c68d 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -28,7 +28,6 @@\n \n static int diff_detect_rename_default;\n static int diff_indent_heuristic; /* experimental */\n-static int diff_compaction_heuristic; /* experimental */\n static int diff_rename_limit_default = 400;\n static int diff_suppress_blank_empty;\n static int diff_use_color_default = -1;\n@@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)\n \n int git_diff_heuristic_config(const char *var, const char *value, void *cb)\n {\n-\tif (!strcmp(var, \"diff.indentheuristic\")) {\n+\tif (!strcmp(var, \"diff.indentheuristic\"))\n \t\tdiff_indent_heuristic = git_config_bool(var, value);\n-\t\tif (diff_indent_heuristic)\n-\t\t\tdiff_compaction_heuristic = 0;\n-\t}\n-\tif (!strcmp(var, \"diff.compactionheuristic\")) {\n-\t\tdiff_compaction_heuristic = git_config_bool(var, value);\n-\t\tif (diff_compaction_heuristic)\n-\t\t\tdiff_indent_heuristic = 0;\n-\t}\n \treturn 0;\n }\n \n@@ -3380,8 +3371,6 @@ void diff_setup(struct diff_options *options)\n \toptions->xdl_opts |= diff_algorithm;\n \tif (diff_indent_heuristic)\n \t\tDIFF_XDL_SET(options, INDENT_HEURISTIC);\n-\telse if (diff_compaction_heuristic)\n-\t\tDIFF_XDL_SET(options, COMPACTION_HEURISTIC);\n \n \toptions->orderfile = diff_order_file_cfg;\n \n@@ -3876,16 +3865,10 @@ int diff_opt_parse(struct diff_options *options,\n \t\tDIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);\n \telse if (!strcmp(arg, \"--ignore-blank-lines\"))\n \t\tDIFF_XDL_SET(options, IGNORE_BLANK_LINES);\n-\telse if (!strcmp(arg, \"--indent-heuristic\")) {\n+\telse if (!strcmp(arg, \"--indent-heuristic\"))\n \t\tDIFF_XDL_SET(options, INDENT_HEURISTIC);\n-\t\tDIFF_XDL_CLR(options, COMPACTION_HEURISTIC);\n-\t} else if (!strcmp(arg, \"--no-indent-heuristic\"))\n-\t\tDIFF_XDL_CLR(options, INDENT_HEURISTIC);\n-\telse if (!strcmp(arg, \"--compaction-heuristic\")) {\n-\t\tDIFF_XDL_SET(options, COMPACTION_HEURISTIC);\n+\telse if (!strcmp(arg, \"--no-indent-heuristic\"))\n \t\tDIFF_XDL_CLR(options, INDENT_HEURISTIC);\n-\t} else if (!strcmp(arg, \"--no-compaction-heuristic\"))\n-\t\tDIFF_XDL_CLR(options, COMPACTION_HEURISTIC);\n \telse if (!strcmp(arg, \"--patience\"))\n \t\toptions->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);\n \telse if (!strcmp(arg, \"--histogram\"))\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex ee3d812695..5a55d80b9d 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -46,7 +46,6 @@\n \n my $diff_algorithm = $repo->config('diff.algorithm');\n my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');\n-my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');\n my $diff_filter = $repo->config('interactive.difffilter');\n \n my $use_readkey = 0;\n@@ -753,8 +752,6 @@ sub parse_diff {\n \t}\n \tif ($diff_indent_heuristic) {\n \t\tsplice @diff_cmd, 1, 0, \"--indent-heuristic\";\n-\t} elsif ($diff_compaction_heuristic) {\n-\t\tsplice @diff_cmd, 1, 0, \"--compaction-heuristic\";\n \t}\n \tif (defined $patch_mode_revision) {\n \t\tpush @diff_cmd, get_diff_reference($patch_mode_revision);\ndiff --git a/xdiff/xdiff.h b/xdiff/xdiff.h\nindex 8db16d4ae6..b090ad8eac 100644\n--- a/xdiff/xdiff.h\n+++ b/xdiff/xdiff.h\n@@ -41,8 +41,7 @@ extern \"C\" {\n \n #define XDF_IGNORE_BLANK_LINES (1 << 7)\n \n-#define XDF_COMPACTION_HEURISTIC (1 << 8)\n-#define XDF_INDENT_HEURISTIC (1 << 9)\n+#define XDF_INDENT_HEURISTIC (1 << 8)\n \n #define XDL_EMIT_FUNCNAMES (1 << 0)\n #define XDL_EMIT_FUNCCONTEXT (1 << 2)\ndiff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c\nindex 760fbb6db7..93a65680a1 100644\n--- a/xdiff/xdiffi.c\n+++ b/xdiff/xdiffi.c\n@@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,\n }\n \n \n-static int is_blank_line(xrecord_t *rec, long flags)\n-{\n-\treturn xdl_blankline(rec->ptr, rec->size, flags);\n-}\n-\n static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)\n {\n \treturn (rec1->ha == rec2->ha &&\n@@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \tstruct xdlgroup g, go;\n \tlong earliest_end, end_matching_other;\n \tlong groupsize;\n-\tunsigned int blank_lines;\n \n \tgroup_init(xdf, &g);\n \tgroup_init(xdfo, &go);\n@@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t */\n \t\t\tend_matching_other = -1;\n \n-\t\t\t/*\n-\t\t\t * Boolean value that records whether there are any blank\n-\t\t\t * lines that could be made to be the last line of this\n-\t\t\t * group.\n-\t\t\t */\n-\t\t\tblank_lines = 0;\n-\n \t\t\t/* Shift the group backward as much as possible: */\n \t\t\twhile (!group_slide_up(xdf, &g, flags))\n \t\t\t\tif (group_previous(xdfo, &go))\n@@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \n \t\t\t/* Now shift the group forward as far as possible: */\n \t\t\twhile (1) {\n-\t\t\t\tif (!blank_lines)\n-\t\t\t\t\tblank_lines = is_blank_line(\n-\t\t\t\t\t\t\txdf->recs[g.end - 1],\n-\t\t\t\t\t\t\tflags);\n-\n \t\t\t\tif (group_slide_down(xdf, &g, flags))\n \t\t\t\t\tbreak;\n \t\t\t\tif (group_next(xdfo, &go))\n@@ -906,21 +888,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {\n \t\t\t\tif (group_previous(xdfo, &go))\n \t\t\t\t\txdl_bug(\"group sync broken sliding to match\");\n \t\t\t}\n-\t\t} else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {\n-\t\t\t/*\n-\t\t\t * Compaction heuristic: if it is possible to shift the\n-\t\t\t * group to make its bottom line a blank line, do so.\n-\t\t\t *\n-\t\t\t * As we already shifted the group forward as far as\n-\t\t\t * possible in the earlier loop, we only need to handle\n-\t\t\t * backward shifts, not forward ones.\n-\t\t\t */\n-\t\t\twhile (!is_blank_line(xdf->recs[g.end - 1], flags)) {\n-\t\t\t\tif (group_slide_up(xdf, &g, flags))\n-\t\t\t\t\txdl_bug(\"blank line disappeared\");\n-\t\t\t\tif (group_previous(xdfo, &go))\n-\t\t\t\t\txdl_bug(\"group sync broken sliding to blank line\");\n-\t\t\t}\n \t\t} else if (flags & XDF_INDENT_HEURISTIC) {\n \t\t\t/*\n \t\t\t * Indent heuristic: a group of pure add/delete lines\n-- \n2.11.0-448-g09546ed716\n\n"},{"id":"308312","messageId":"20161223222145.vkf6mjvs5t7ag3od@sigill.intra.peff.net","threadId":"44722","inReplyTo":"xmqq8tr6e46o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-12-23T22:21:45Z","receivedAt":"2016-12-23T22:21:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 23, 2016 at 01:17:03PM -0800, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > I guess both you and Michael are in favor of just removing compaction\n> > variant without any renames, so let me prepare a reroll and queue\n> > that instead.  We can flip the default perhaps one release later.\n> \n> -- >8 --\n> Subject: [PATCH] diff: retire \"compaction\" heuristics\n\nLooks good to me from a cursory read.\n\nThanks.\n\n-Peff\n"},{"id":"308313","messageId":"CA+P7+xrMFnW8NCySpMuWWnXBE30RwmOgr-0Xgjaa5i8eDPGGUw@mail.gmail.com","threadId":"44722","inReplyTo":"20161223222145.vkf6mjvs5t7ag3od@sigill.intra.peff.net","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2016-12-23T22:23:24Z","receivedAt":"2016-12-23T22:23:52Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Fri, Dec 23, 2016 at 2:21 PM, Jeff King <peff@peff.net> wrote:\n> On Fri, Dec 23, 2016 at 01:17:03PM -0800, Junio C Hamano wrote:\n>\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>> > I guess both you and Michael are in favor of just removing compaction\n>> > variant without any renames, so let me prepare a reroll and queue\n>> > that instead.  We can flip the default perhaps one release later.\n>>\n>> -- >8 --\n>> Subject: [PATCH] diff: retire \"compaction\" heuristics\n>\n> Looks good to me from a cursory read.\n>\n> Thanks.\n>\n> -Peff\n\nSame. This is more obviously correct since we didn't have to change a\nbunch of references to INDENT_HEURISTIC. I agree that the name does\nnot make sense now, but if our goal is to make it default with a\ndisable option, I think that we shouldn't worry too much about the\nnaming.\n\nThanks,\nJake\n"},{"id":"308321","messageId":"653b67e6-9dba-d331-c396-932a59cdb4da@alum.mit.edu","threadId":"44722","inReplyTo":"xmqq8tr6e46o.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2016-12-24T12:55:33Z","receivedAt":"2016-12-24T12:55:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 12/23/2016 10:17 PM, Junio C Hamano wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n>> I guess both you and Michael are in favor of just removing compaction\n>> variant without any renames, so let me prepare a reroll and queue\n>> that instead.  We can flip the default perhaps one release later.\n> \n> -- >8 --\n> Subject: [PATCH] diff: retire \"compaction\" heuristics\n> \n> When a patch inserts a block of lines, whose last lines are the\n> same as the existing lines that appear before the inserted block,\n> \"git diff\" can choose any place between these existing lines as the\n> boundary between the pre-context and the added lines (adjusting the\n> end of the inserted block as appropriate) to come up with variants\n> of the same patch, and some variants are easier to read than others.\n> \n> We have been trying to improve the choice of this boundary, and Git\n> 2.11 shipped with an experimental \"compaction-heuristic\".  Since\n> then another attempt to improve the logic further resulted in a new\n> \"indent-heuristic\" logic.  It is agreed that the latter gives better\n> result overall, and the former outlived its usefulness.\n> \n> Retire \"compaction\", and keep \"indent\" as an experimental feature.\n> The latter hopefully will be turned on by default in a future\n> release, but that should be done as a separate step.\n\nThe whole patch looks good to me. Thanks for taking care of this.\n\nMichael\n\n"},{"id":"318094","messageId":"CAGZ79kY2Yu=B+75D6DpNjdriAZnJ5pZSmYAUiYtF2aUK47_HPw@mail.gmail.com","threadId":"44722","inReplyTo":"653b67e6-9dba-d331-c396-932a59cdb4da@alum.mit.edu","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-27T21:17:55Z","receivedAt":"2017-04-27T21:18:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"picking up this old topic,\nwith Martin, Marc and SZEDER cc'd,\nas we got patch proposals regarding the indent heuristic.\n\nOn Sat, Dec 24, 2016 at 4:55 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> On 12/23/2016 10:17 PM, Junio C Hamano wrote:\n>> Junio C Hamano <gitster@pobox.com> writes:\n>>\n>>> I guess both you and Michael are in favor of just removing compaction\n>>> variant without any renames, so let me prepare a reroll and queue\n>>> that instead.  We can flip the default perhaps one release later.\n>>\n>> -- >8 --\n>> Subject: [PATCH] diff: retire \"compaction\" heuristics\n>>\n>> When a patch inserts a block of lines, whose last lines are the\n>> same as the existing lines that appear before the inserted block,\n>> \"git diff\" can choose any place between these existing lines as the\n>> boundary between the pre-context and the added lines (adjusting the\n>> end of the inserted block as appropriate) to come up with variants\n>> of the same patch, and some variants are easier to read than others.\n>>\n>> We have been trying to improve the choice of this boundary, and Git\n>> 2.11 shipped with an experimental \"compaction-heuristic\".  Since\n>> then another attempt to improve the logic further resulted in a new\n>> \"indent-heuristic\" logic.  It is agreed that the latter gives better\n>> result overall, and the former outlived its usefulness.\n>>\n>> Retire \"compaction\", and keep \"indent\" as an experimental feature.\n>> The latter hopefully will be turned on by default in a future\n>> release, but that should be done as a separate step.\n\nMaybe turning on this feature by default is the next step instead of\nadding them to bash competition or making them available in plumbing\ncommands for the upcoming release.\n\nThen in a later release we could even remove the knobs to turn it off.\n\nThanks,\nStefan\n"},{"id":"318124","messageId":"20170428081143.7ae7jflhrjukxcsh@sigill.intra.peff.net","threadId":"44722","inReplyTo":"CAGZ79kY2Yu=B+75D6DpNjdriAZnJ5pZSmYAUiYtF2aUK47_HPw@mail.gmail.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-28T08:11:43Z","receivedAt":"2017-04-28T08:11:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Apr 27, 2017 at 02:17:55PM -0700, Stefan Beller wrote:\n\n> >> Retire \"compaction\", and keep \"indent\" as an experimental feature.\n> >> The latter hopefully will be turned on by default in a future\n> >> release, but that should be done as a separate step.\n> \n> Maybe turning on this feature by default is the next step instead of\n> adding them to bash competition or making them available in plumbing\n> commands for the upcoming release.\n> \n> Then in a later release we could even remove the knobs to turn it off.\n\nYeah, if we are going to proceed with making it the default, I'd rather\ndo that than worry about adding frills that will eventually not matter.\n\nWe could take Marc's patch to move the config to git_diff_basic_config()\nas an interim step, though. It gives people an escape hatch if they find\nthat they need to disable the feature for some plumbing command.\n\nIn that discussion, the only argument I could come up with against\nmaking it the default (or respecting it for plumbing) is that it may\nhave an impact on patch-ids. I'm not sure I'm swayed by that, but I\nwanted to mention it to make sure we're deciding consciously not to care\nabout it.\n\n-Peff\n"},{"id":"318126","messageId":"86d57f89-b554-4268-6f4e-ea9812ad4b7a@suse.cz","threadId":"44722","inReplyTo":"CAGZ79kY2Yu=B+75D6DpNjdriAZnJ5pZSmYAUiYtF2aUK47_HPw@mail.gmail.com","subject":"Re: [PATCH] diff: prefer indent heuristic over compaction heuristic","fromName":"Martin Liška","fromEmail":"mliska@suse.cz","sentAt":"2017-04-28T08:40:46Z","receivedAt":"2017-04-28T08:41:24Z","isPatch":true,"sender":{"key":"mliska@suse.cz","avatar":null},"body":"On 04/27/2017 11:17 PM, Stefan Beller wrote:\n> Maybe turning on this feature by default is the next step instead of\n> adding them to bash competition or making them available in plumbing\n> commands for the upcoming release.\n\nHello.\n\nWorks for me, please ignore the patch I've sent ;)\n\nAnyhow, nice option (functionality), I really like it.\nMartin\n"}]}