diff: retire "compaction" heuristics
When a patch inserts a block of lines, whose last lines are the same as the existing lines that appear before the inserted block, "git diff" can choose any place between these existing lines as the boundary between the pre-context and the added lines (adjusting the end of the inserted block as appropriate) to come up with variants of the same patch, and some variants are easier to read than others. We have been trying to improve the choice of this boundary, and Git 2.11 shipped with an experimental "compaction-heuristic". Since then another attempt to improve the logic further resulted in a new "indent-heuristic" logic. It is agreed that the latter gives better result overall, and the former outlived its usefulness. Retire "compaction", and keep "indent" as an experimental feature. The latter hopefully will be turned on by default in a future release, but that should be done as a separate step. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Dec 23, 2016 at 12:32 UTC
3cde4e02ee891bff53bac7f6a7d977f50418a4b5
7 files changed
+8
-67
Documentation/diff-config.txt
+2
-4
@@ -172,10 +172,8 @@ diff.tool::
172
include::mergetools-diff.txt[]
173
174
diff.indentHeuristic::
175
-diff.compactionHeuristic::
176
- Set one of these options to `true` to enable one of two
177
- experimental heuristics that shift diff hunk boundaries to
178
- make patches easier to read.
175
+ Set this option to `true` to enable experimental heuristics
176
+ that shift diff hunk boundaries to make patches easier to read.
177
178
diff.algorithm::
179
Choose a diff algorithm. The variants are as follows:
Documentation/diff-heuristic-options.txt
-2
@@ -1,7 +1,5 @@
1
--indent-heuristic::
2
--no-indent-heuristic::
3
---compaction-heuristic::
4
---no-compaction-heuristic::
3
These are to help debugging and tuning experimental heuristics
4
(which are off by default) that shift diff hunk boundaries to
5
make patches easier to read.
builtin/blame.c
+2
-3
@@ -2596,8 +2596,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
2596
* and are only included here to get included in the "-h"
2597
* output:
2598
*/
2599
- { 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 },
2600
- { 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 },
2599
+ { OPTION_LOWLEVEL_CALLBACK, 0, "indent-heuristic", NULL, NULL, N_("Use an experimental heuristic to improve diffs"), PARSE_OPT_NOARG, parse_opt_unknown_cb },
2600
2601
OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
2602
OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions from <file> instead of calling git-rev-list")),
@@ -2645,7 +2644,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
2644
}
2645
parse_done:
2646
no_whole_file_rename = !DIFF_OPT_TST(&revs.diffopt, FOLLOW_RENAMES);
2648
- xdl_opts |= revs.diffopt.xdl_opts & (XDF_COMPACTION_HEURISTIC | XDF_INDENT_HEURISTIC);
2647
+ xdl_opts |= revs.diffopt.xdl_opts & XDF_INDENT_HEURISTIC;
2648
DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
2649
argc = parse_options_end(&ctx);
2650
diff.c
+3
-20
@@ -28,7 +28,6 @@
28
29
static int diff_detect_rename_default;
30
static int diff_indent_heuristic; /* experimental */
31
-static int diff_compaction_heuristic; /* experimental */
31
static int diff_rename_limit_default = 400;
32
static int diff_suppress_blank_empty;
33
static int diff_use_color_default = -1;
@@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)
222
223
int git_diff_heuristic_config(const char *var, const char *value, void *cb)
224
{
226
- if (!strcmp(var, "diff.indentheuristic")) {
225
+ if (!strcmp(var, "diff.indentheuristic"))
226
diff_indent_heuristic = git_config_bool(var, value);
228
- if (diff_indent_heuristic)
229
- diff_compaction_heuristic = 0;
230
- }
231
- if (!strcmp(var, "diff.compactionheuristic")) {
232
- diff_compaction_heuristic = git_config_bool(var, value);
233
- if (diff_compaction_heuristic)
234
- diff_indent_heuristic = 0;
235
- }
227
return 0;
228
}
229
@@ -3380,8 +3371,6 @@ void diff_setup(struct diff_options *options)
3371
options->xdl_opts |= diff_algorithm;
3372
if (diff_indent_heuristic)
3373
DIFF_XDL_SET(options, INDENT_HEURISTIC);
3383
- else if (diff_compaction_heuristic)
3384
- DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
3374
3375
options->orderfile = diff_order_file_cfg;
3376
@@ -3876,16 +3865,10 @@ int diff_opt_parse(struct diff_options *options,
3865
DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);
3866
else if (!strcmp(arg, "--ignore-blank-lines"))
3867
DIFF_XDL_SET(options, IGNORE_BLANK_LINES);
3879
- else if (!strcmp(arg, "--indent-heuristic")) {
3868
+ else if (!strcmp(arg, "--indent-heuristic"))
3869
DIFF_XDL_SET(options, INDENT_HEURISTIC);
3881
- DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
3882
- } else if (!strcmp(arg, "--no-indent-heuristic"))
3883
- DIFF_XDL_CLR(options, INDENT_HEURISTIC);
3884
- else if (!strcmp(arg, "--compaction-heuristic")) {
3885
- DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
3870
+ else if (!strcmp(arg, "--no-indent-heuristic"))
3871
DIFF_XDL_CLR(options, INDENT_HEURISTIC);
3887
- } else if (!strcmp(arg, "--no-compaction-heuristic"))
3888
- DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
3872
else if (!strcmp(arg, "--patience"))
3873
options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
3874
else if (!strcmp(arg, "--histogram"))
git-add--interactive.perl
-3
@@ -46,7 +46,6 @@ my $normal_color = $repo->get_color("", "reset");
46
47
my $diff_algorithm = $repo->config('diff.algorithm');
48
my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');
49
-my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');
49
my $diff_filter = $repo->config('interactive.difffilter');
50
51
my $use_readkey = 0;
@@ -753,8 +752,6 @@ sub parse_diff {
752
}
753
if ($diff_indent_heuristic) {
754
splice @diff_cmd, 1, 0, "--indent-heuristic";
756
- } elsif ($diff_compaction_heuristic) {
757
- splice @diff_cmd, 1, 0, "--compaction-heuristic";
755
}
756
if (defined $patch_mode_revision) {
757
push @diff_cmd, get_diff_reference($patch_mode_revision);
xdiff/xdiff.h
+1
-2
@@ -41,8 +41,7 @@ extern "C" {
41
42
#define XDF_IGNORE_BLANK_LINES (1 << 7)
43
44
-#define XDF_COMPACTION_HEURISTIC (1 << 8)
45
-#define XDF_INDENT_HEURISTIC (1 << 9)
44
+#define XDF_INDENT_HEURISTIC (1 << 8)
45
46
#define XDL_EMIT_FUNCNAMES (1 << 0)
47
#define XDL_EMIT_FUNCCONTEXT (1 << 2)
xdiff/xdiffi.c
-33
@@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
400
}
401
402
403
-static int is_blank_line(xrecord_t *rec, long flags)
404
-{
405
- return xdl_blankline(rec->ptr, rec->size, flags);
406
-}
407
-
403
static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)
404
{
405
return (rec1->ha == rec2->ha &&
@@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
816
struct xdlgroup g, go;
817
long earliest_end, end_matching_other;
818
long groupsize;
824
- unsigned int blank_lines;
819
820
group_init(xdf, &g);
821
group_init(xdfo, &go);
@@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
840
*/
841
end_matching_other = -1;
842
849
- /*
850
- * Boolean value that records whether there are any blank
851
- * lines that could be made to be the last line of this
852
- * group.
853
- */
854
- blank_lines = 0;
855
-
843
/* Shift the group backward as much as possible: */
844
while (!group_slide_up(xdf, &g, flags))
845
if (group_previous(xdfo, &go))
@@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
856
857
/* Now shift the group forward as far as possible: */
858
while (1) {
872
- if (!blank_lines)
873
- blank_lines = is_blank_line(
874
- xdf->recs[g.end - 1],
875
- flags);
876
-
859
if (group_slide_down(xdf, &g, flags))
860
break;
861
if (group_next(xdfo, &go))
@@ -906,21 +888,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
888
if (group_previous(xdfo, &go))
889
xdl_bug("group sync broken sliding to match");
890
}
909
- } else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {
910
- /*
911
- * Compaction heuristic: if it is possible to shift the
912
- * group to make its bottom line a blank line, do so.
913
- *
914
- * As we already shifted the group forward as far as
915
- * possible in the earlier loop, we only need to handle
916
- * backward shifts, not forward ones.
917
- */
918
- while (!is_blank_line(xdf->recs[g.end - 1], flags)) {
919
- if (group_slide_up(xdf, &g, flags))
920
- xdl_bug("blank line disappeared");
921
- if (group_previous(xdfo, &go))
922
- xdl_bug("group sync broken sliding to blank line");
923
- }
891
} else if (flags & XDF_INDENT_HEURISTIC) {
892
/*
893
* Indent heuristic: a group of pure add/delete lines