From 3cde4e02ee891bff53bac7f6a7d977f50418a4b5 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 23 Dec 2016 12:32:22 -0800 Subject: 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 Signed-off-by: Junio C Hamano --- xdiff/xdiff.h | 3 +-- xdiff/xdiffi.c | 33 --------------------------------- 2 files changed, 1 insertion(+), 35 deletions(-) (limited to 'xdiff') diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h index 8db16d4ae..b090ad8ea 100644 --- a/xdiff/xdiff.h +++ b/xdiff/xdiff.h @@ -41,8 +41,7 @@ extern "C" { #define XDF_IGNORE_BLANK_LINES (1 << 7) -#define XDF_COMPACTION_HEURISTIC (1 << 8) -#define XDF_INDENT_HEURISTIC (1 << 9) +#define XDF_INDENT_HEURISTIC (1 << 8) #define XDL_EMIT_FUNCNAMES (1 << 0) #define XDL_EMIT_FUNCCONTEXT (1 << 2) diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c index 760fbb6db..93a65680a 100644 --- a/xdiff/xdiffi.c +++ b/xdiff/xdiffi.c @@ -400,11 +400,6 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1, } -static int is_blank_line(xrecord_t *rec, long flags) -{ - return xdl_blankline(rec->ptr, rec->size, flags); -} - static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags) { return (rec1->ha == rec2->ha && @@ -821,7 +816,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) { struct xdlgroup g, go; long earliest_end, end_matching_other; long groupsize; - unsigned int blank_lines; group_init(xdf, &g); group_init(xdfo, &go); @@ -846,13 +840,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) { */ end_matching_other = -1; - /* - * Boolean value that records whether there are any blank - * lines that could be made to be the last line of this - * group. - */ - blank_lines = 0; - /* Shift the group backward as much as possible: */ while (!group_slide_up(xdf, &g, flags)) if (group_previous(xdfo, &go)) @@ -869,11 +856,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) { /* Now shift the group forward as far as possible: */ while (1) { - if (!blank_lines) - blank_lines = is_blank_line( - xdf->recs[g.end - 1], - flags); - if (group_slide_down(xdf, &g, flags)) break; if (group_next(xdfo, &go)) @@ -906,21 +888,6 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) { if (group_previous(xdfo, &go)) xdl_bug("group sync broken sliding to match"); } - } else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) { - /* - * Compaction heuristic: if it is possible to shift the - * group to make its bottom line a blank line, do so. - * - * As we already shifted the group forward as far as - * possible in the earlier loop, we only need to handle - * backward shifts, not forward ones. - */ - while (!is_blank_line(xdf->recs[g.end - 1], flags)) { - if (group_slide_up(xdf, &g, flags)) - xdl_bug("blank line disappeared"); - if (group_previous(xdfo, &go)) - xdl_bug("group sync broken sliding to blank line"); - } } else if (flags & XDF_INDENT_HEURISTIC) { /* * Indent heuristic: a group of pure add/delete lines -- cgit v1.2.1