git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/7] xdl_change_compact(): only use heuristic if group can't be matched

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Aug 22, 2016, 11:22 UTC
Message-ID
<d2fd0eefdf6e4f2884df3214b32606e5b43e0555.1471864378.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1471864378.git.mhagger@alum.mit.edu>

If the changed group of lines can be matched to a group in the other file, then that positioning should take precedence over the compaction heuristic.

The old code tried the heuristic unconditionally, which cost redundant effort and also was broken if the matching code had already shifted the group higher than the blank line.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
 xdiff/xdiffi.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 95b037e..61deed8 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -504,25 +504,25 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 			}
 		} while (grpsiz != ix - ixs);
 
-		/*
-		 * Try to move back the possibly merged group of changes, to match
-		 * the recorded position in the other file.
-		 */
-		while (ixref < ix) {
-			rchg[--ixs] = 1;
-			rchg[--ix] = 0;
-			while (rchgo[--ixo]);
-		}
-
-		/*
-		 * If a group can be moved back and forth, see if there is a
-		 * blank line in the moving space. If there is a blank line,
-		 * make sure the last blank line is the end of the group.
-		 *
-		 * As we already shifted the group forward as far as possible
-		 * in the earlier loop, we need to shift it back only if at all.
-		 */
-		if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {
+		if (ixref < ix) {
+			/*
+			 * Try to move back the possibly merged group of changes, to match
+			 * the recorded position in the other file.
+			 */
+			while (ixref < ix) {
+				rchg[--ixs] = 1;
+				rchg[--ix] = 0;
+				while (rchgo[--ixo]);
+			}
+		} else if ((flags & XDF_COMPACTION_HEURISTIC) && blank_lines) {
+			/*
+			 * The group can be slid up to make its last line a
+			 * blank line. Do so.
+			 *
+			 * As we already shifted the group forward as far as
+			 * possible in the earlier loop, we need to shift it
+			 * back only if at all.
+			 */
 			while (ixs > 0 &&
 			       !is_blank_line(recs, ix - 1, flags) &&
 			       recs_match(recs, ixs - 1, ix - 1, flags)) {
-- 
2.9.3
Previous: Michael HaggertyNext: Michael Haggerty
Message 2 of 13 in “Better heuristics make prettier diffs”
  1. 0/7 Better heuristics make prettier diffsMichael Haggerty, Aug 22, 2016
  2. 2/7 xdl_change_compact(): only use heuristic if group can't be matchedMichael Haggerty, Aug 22, 2016
  3. 1/7 xdl_change_compact(): fix compaction heuristic to adjust ixoMichael Haggerty, Aug 22, 2016
  4. 6/7 diff: improve positioning of add/delete blocks in diffsMichael Haggerty, Aug 22, 2016
  5. 3/7 is_blank_line(): take a single xrecord_t as argumentMichael Haggerty, Aug 22, 2016
  6. 5/7 xdl_change_compact(): introduce the concept of a change groupMichael Haggerty, Aug 22, 2016
  7. Junio C HamanoAug 23, 2016
  8. 4/7 recs_match(): take two xrecord_t pointers as argumentsMichael Haggerty, Aug 22, 2016
  9. 7/7 blame: actually use the diff opts parsed from the command lineMichael Haggerty, Aug 22, 2016
  10. René ScharfeAug 23, 2016
  11. Michael HaggertySep 5, 2016
  12. Junio C HamanoSep 7, 2016
  13. Junio C HamanoAug 23, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.