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

[PATCH 2/8] xdl_change_compact(): clarify code

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Aug 3, 2016, 22:00 UTC
Message-ID
<f4ce27f389b64c9ae503152c66d183c4a4a970f1.1470259583.git.mhagger@alum.mit.edu>
In-Reply-To
<cover.1470259583.git.mhagger@alum.mit.edu>
Write things out a bit longer but less cryptically. Add more comments.
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
---
I find the loops in the old code, with unfamiliar patterns of embedded
increment/decrement operators, confusing, and I think that writing
things out a little bit more verbosely (and with more comments) makes
it much easier to read the code and be sure that it is correct.
The compiled code and performance shouldn't be affected materially.
 xdiff/xdiffi.c | 106 +++++++++++++++++++++++++++++++++++++++------------------
 1 file changed, 73 insertions(+), 33 deletions(-)
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index ff7fc42..a0a485c 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -434,8 +434,14 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 		 * not need index bounding since the array is prepared with
 		 * a zero at position -1 and N.
 		 */
-		for (; i < nrec && !rchg[i]; i++)
-			while (rchgo[io++]);
+		for (; i < nrec && !rchg[i]; i++) {
+			/* skip over any changed lines in the other file... */
+			while (rchgo[io])
+				io++;
+
+			/* ...plus one non-changed line. */
+			io++;
+		}
 		if (i == nrec)
 			break;
 
@@ -444,45 +450,70 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 		 * and find the end of it, on both to-be-compacted and other file
 		 * indexes (i and io).
 		 */
-		start = i;
-		for (i++; rchg[i]; i++);
-		for (; rchgo[io]; io++);
+		start = i++;
+
+		while (rchg[i])
+			i++;
+
+		while (rchgo[io])
+		       io++;
 
 		do {
 			groupsize = i - start;
+
+			/*
+			 * Are there any blank lines that could appear as the last
+			 * line of this group?
+			 */
 			blank_lines = 0;
 
 			/*
-			 * If the line before the current change group, is equal to
-			 * the last line of the current change group, shift backward
-			 * the group.
+			 * While the line before the current change group is equal
+			 * to the last line of the current change group, shift the
+			 * group backward.
 			 */
 			while (start > 0 && recs_match(recs, start - 1, i - 1, flags)) {
 				rchg[--start] = 1;
 				rchg[--i] = 0;
 
 				/*
-				 * This change might have joined two change groups,
-				 * so we try to take this scenario in account by moving
-				 * the start index accordingly (and so the other-file
-				 * end-of-group index).
+				 * This change might have joined two change groups.
+				 * If so, move the start index to the beginning of
+				 * the combined group:
 				 */
-				for (; rchg[start - 1]; start--);
-				while (rchgo[--io]);
+				while (rchg[start - 1])
+					start--;
+
+				/*
+				 * Move the other file index past a non-changed
+				 * line...
+				 */
+				io--;
+
+				/* ...and also past any changed lines: */
+				while (rchgo[io])
+					io--;
 			}
 
-			/*
-			 * Record the end-of-group position in case we are matched
-			 * with a group of changes in the other file (that is, the
-			 * change record before the end-of-group index in the other
-			 * file is set).
-			 */
-			ixref = rchgo[io - 1] ? i : nrec;
+			if (rchgo[io - 1]) {
+				/*
+				 * This change is matched to a group of changes in
+				 * the other file. Record the end-of-group
+				 * position:
+				 */
+				ixref = i;
+			} else {
+				/*
+				 * Otherwise, set a value to signify that there
+				 * are no matched changes in the other file:
+				 */
+				ixref = nrec;
+			}
 
 			/*
-			 * If the first line of the current change group, is equal to
-			 * the line next of the current change group, shift forward
-			 * the group.
+			 * Now shift the group forward as long as the first line
+			 * of the current change group is equal to the line after
+			 * the current change group.
 			 */
 			while (i < nrec && recs_match(recs, start, i, flags)) {
 				blank_lines += is_blank_line(recs, i, flags);
@@ -491,16 +522,22 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 				rchg[i++] = 1;
 
 				/*
-				 * This change might have joined two change groups,
-				 * so we try to take this scenario in account by moving
-				 * the start index accordingly (and so the other-file
-				 * end-of-group index). Keep tracking the reference
-				 * index in case we are shifting together with a
-				 * corresponding group of changes in the other file.
+				 * This change might have joined two change
+				 * groups. If so, move the start index accordingly
+				 * (and so the other-file end-of-group index).
+				 * Keep tracking the reference index in case we
+				 * are shifting together with a corresponding
+				 * group of changes in the other file.
 				 */
-				for (; rchg[i]; i++);
-				while (rchgo[++io])
+				while (rchg[i])
+					i++;
+
+				io++;
+				if (rchgo[io]) {
 					ixref = i;
+					while (rchgo[io])
+						io++;
+				}
 			}
 		} while (groupsize != i - start);
 
@@ -511,7 +548,10 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 		while (ixref < i) {
 			rchg[--start] = 1;
 			rchg[--i] = 0;
-			while (rchgo[--io]);
+
+			io--;
+			while (rchgo[io])
+				io--;
 		}
 
 		/*
-- 
2.8.1
Previous: Michael HaggertyNext: Stefan Beller
Message 2 of 57 in “Better heuristics make prettier diffs”
  1. 0/8 Better heuristics make prettier diffsMichael Haggerty, Aug 3, 2016
  2. 2/8 xdl_change_compact(): clarify codeMichael Haggerty, Aug 3, 2016
  3. Stefan BellerAug 3, 2016
  4. Michael HaggertyAug 3, 2016
  5. Stefan BellerAug 3, 2016
  6. Jeff KingAug 4, 2016
  7. Michael HaggertyAug 10, 2016
  8. Stefan BellerAug 10, 2016
  9. 5/8 xdl_change_compact(): fix compaction heuristic to adjust ioMichael Haggerty, Aug 3, 2016
  10. Jeff KingAug 4, 2016
  11. Michael HaggertyAug 10, 2016
  12. Michael HaggertyAug 10, 2016
  13. Jeff KingAug 11, 2016
  14. Junio C HamanoAug 4, 2016
  15. Michael HaggertyAug 10, 2016
  16. 7/8 is_blank_line: take a single xrecord_t as argumentMichael Haggerty, Aug 3, 2016
  17. Junio C HamanoAug 4, 2016
  18. 6/8 xdl_change_compact(): keep track of the earliest endMichael Haggerty, Aug 3, 2016
  19. Junio C HamanoAug 4, 2016
  20. Michael HaggertyAug 10, 2016
  21. 4/8 xdl_change_compact(): do one final shift or the other, not bothMichael Haggerty, Aug 3, 2016
  22. 1/8 xdl_change_compact(): rename some local variables for clarityMichael Haggerty, Aug 3, 2016
  23. Jeff KingAug 4, 2016
  24. Junio C HamanoAug 4, 2016
  25. Michael HaggertyAug 13, 2016
  26. Jeff KingAug 14, 2016
  27. Michael HaggertyAug 3, 2016
  28. 3/8 xdl_change_compact(): rename i to endMichael Haggerty, Aug 3, 2016
  29. Jeff KingAug 4, 2016
  30. 8/8 diff: improve positioning of add/delete blocks in diffsMichael Haggerty, Aug 3, 2016
  31. Jacob KellerAug 3, 2016
  32. Michael HaggertyAug 3, 2016
  33. Jacob KellerAug 4, 2016
  34. Junio C HamanoAug 4, 2016
  35. Michael HaggertyAug 10, 2016
  36. Junio C HamanoAug 10, 2016
  37. Stefan BellerAug 3, 2016
  38. Michael HaggertyAug 3, 2016
  39. Stefan BellerAug 3, 2016
  40. Michael HaggertyAug 3, 2016
  41. Stefan BellerAug 4, 2016
  42. Michael HaggertyAug 10, 2016
  43. Jeff KingAug 4, 2016
  44. Stefan BellerAug 4, 2016
  45. Junio C HamanoAug 4, 2016
  46. Michael HaggertyAug 13, 2016
  47. Michael HaggertyAug 12, 2016
  48. Jeff KingAug 13, 2016
  49. Junio C HamanoAug 13, 2016
  50. Jacob KellerAug 14, 2016
  51. Stefan BellerAug 15, 2016
  52. Junio C HamanoAug 15, 2016
  53. Junio C HamanoAug 4, 2016
  54. Michael HaggertyAug 13, 2016
  55. Jeff KingAug 4, 2016
  56. Junio C HamanoAug 4, 2016
  57. Jeff KingAug 4, 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.