Skip to content

Commit 3cde4e0

Browse files
gitstergitster
committed
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>
1 parent be5a750 commit 3cde4e0

File tree

7 files changed

+8
-67
lines changed

7 files changed

+8
-67
lines changed

‎Documentation/diff-config.txt‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -172,10 +172,8 @@ diff.tool::
172172
include::mergetools-diff.txt[]
173173

174174
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.
179177

180178
diff.algorithm::
181179
Choose a diff algorithm. The variants are as follows:

‎Documentation/diff-heuristic-options.txt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
--indent-heuristic::
22
--no-indent-heuristic::
3-
--compaction-heuristic::
4-
--no-compaction-heuristic::
53
These are to help debugging and tuning experimental heuristics
64
(which are off by default) that shift diff hunk boundaries to
75
make patches easier to read.

‎builtin/blame.c‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2596,8 +2596,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
25962596
* and are only included here to get included in the "-h"
25972597
* output:
25982598
*/
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 },
26012600

26022601
OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find better match"), XDF_NEED_MINIMAL),
26032602
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)
26452644
}
26462645
parse_done:
26472646
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;
26492648
DIFF_OPT_CLR(&revs.diffopt, FOLLOW_RENAMES);
26502649
argc = parse_options_end(&ctx);
26512650

‎diff.c‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828

2929
static int diff_detect_rename_default;
3030
static int diff_indent_heuristic; /* experimental */
31-
static int diff_compaction_heuristic; /* experimental */
3231
static int diff_rename_limit_default = 400;
3332
static int diff_suppress_blank_empty;
3433
static int diff_use_color_default = -1;
@@ -223,16 +222,8 @@ void init_diff_ui_defaults(void)
223222

224223
int git_diff_heuristic_config(const char *var, const char *value, void *cb)
225224
{
226-
if (!strcmp(var, "diff.indentheuristic")) {
225+
if (!strcmp(var, "diff.indentheuristic"))
227226
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-
}
236227
return 0;
237228
}
238229

@@ -3380,8 +3371,6 @@ void diff_setup(struct diff_options *options)
33803371
options->xdl_opts |= diff_algorithm;
33813372
if (diff_indent_heuristic)
33823373
DIFF_XDL_SET(options, INDENT_HEURISTIC);
3383-
else if (diff_compaction_heuristic)
3384-
DIFF_XDL_SET(options, COMPACTION_HEURISTIC);
33853374

33863375
options->orderfile = diff_order_file_cfg;
33873376

@@ -3876,16 +3865,10 @@ int diff_opt_parse(struct diff_options *options,
38763865
DIFF_XDL_SET(options, IGNORE_WHITESPACE_AT_EOL);
38773866
else if (!strcmp(arg, "--ignore-blank-lines"))
38783867
DIFF_XDL_SET(options, IGNORE_BLANK_LINES);
3879-
else if (!strcmp(arg, "--indent-heuristic")) {
3868+
else if (!strcmp(arg, "--indent-heuristic"))
38803869
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"))
38863871
DIFF_XDL_CLR(options, INDENT_HEURISTIC);
3887-
} else if (!strcmp(arg, "--no-compaction-heuristic"))
3888-
DIFF_XDL_CLR(options, COMPACTION_HEURISTIC);
38893872
else if (!strcmp(arg, "--patience"))
38903873
options->xdl_opts = DIFF_WITH_ALG(options, PATIENCE_DIFF);
38913874
else if (!strcmp(arg, "--histogram"))

‎git-add--interactive.perl‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646

4747
my $diff_algorithm = $repo->config('diff.algorithm');
4848
my $diff_indent_heuristic = $repo->config_bool('diff.indentheuristic');
49-
my $diff_compaction_heuristic = $repo->config_bool('diff.compactionheuristic');
5049
my $diff_filter = $repo->config('interactive.difffilter');
5150

5251
my $use_readkey = 0;
@@ -753,8 +752,6 @@ sub parse_diff {
753752
}
754753
if ($diff_indent_heuristic) {
755754
splice @diff_cmd, 1, 0, "--indent-heuristic";
756-
} elsif ($diff_compaction_heuristic) {
757-
splice @diff_cmd, 1, 0, "--compaction-heuristic";
758755
}
759756
if (defined $patch_mode_revision) {
760757
push @diff_cmd, get_diff_reference($patch_mode_revision);

‎xdiff/xdiff.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,7 @@ extern "C" {
4141

4242
#define XDF_IGNORE_BLANK_LINES (1 << 7)
4343

44-
#define XDF_COMPACTION_HEURISTIC (1 << 8)
45-
#define XDF_INDENT_HEURISTIC (1 << 9)
44+
#define XDF_INDENT_HEURISTIC (1 << 8)
4645

4746
#define XDL_EMIT_FUNCNAMES (1 << 0)
4847
#define XDL_EMIT_FUNCCONTEXT (1 << 2)

‎xdiff/xdiffi.c‎

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
400400
}
401401

402402

403-
static int is_blank_line(xrecord_t *rec, long flags)
404-
{
405-
return xdl_blankline(rec->ptr, rec->size, flags);
406-
}
407-
408403
static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)
409404
{
410405
return (rec1->ha == rec2->ha &&
@@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
821816
struct xdlgroup g, go;
822817
long earliest_end, end_matching_other;
823818
long groupsize;
824-
unsigned int blank_lines;
825819

826820
group_init(xdf, &g);
827821
group_init(xdfo, &go);
@@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
846840
*/
847841
end_matching_other = -1;
848842

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-
856843
/* Shift the group backward as much as possible: */
857844
while (!group_slide_up(xdf, &g, flags))
858845
if (group_previous(xdfo, &go))
@@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
869856

870857
/* Now shift the group forward as far as possible: */
871858
while (1) {
872-
if (!blank_lines)
873-
blank_lines = is_blank_line(
874-
xdf->recs[g.end - 1],
875-
flags);
876-
877859
if (group_slide_down(xdf, &g, flags))
878860
break;
879861
if (group_next(xdfo, &go))
@@ -906,21 +888,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
906888
if (group_previous(xdfo, &go))
907889
xdl_bug("group sync broken sliding to match");
908890
}
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-
}
924891
} else if (flags & XDF_INDENT_HEURISTIC) {
925892
/*
926893
* Indent heuristic: a group of pure add/delete lines

0 commit comments

Comments
 (0)