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

[PATCH v1 9/9] diff --color-moved-ws: handle blank lines

From
PWPhillip Wood <phillip.wood@talktalk.net>
Date
Nov 16, 2018, 11:03 UTC
Message-ID
<20181116110356.12311-10-phillip.wood@talktalk.net>
In-Reply-To
<20181116110356.12311-1-phillip.wood@talktalk.net>
From: Phillip Wood <phillip.wood@dunelm.org.uk>

When using --color-moved-ws=allow-indentation-change allow lines with the same indentation change to be grouped across blank lines. For now this only works if the blank lines have been moved as well, not for blocks that have just had their indentation changed.

This completes the changes to the implementation of --color-moved=allow-indentation-change. Running

  git diff --color-moved=allow-indentation-change v2.18.0 v2.19.0

now takes 5.0s. This is a saving of 41% from 8.5s for the optimized version of the previous implementation and 66% from the original which took 14.6s.

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Notes:
    Changes since rfc:
     - Split these changes into a separate commit.
     - Detect blank lines when processing the indentation rather than
       parsing each line twice.
     - Tweaked the test to make it harder as suggested by Stefan.
     - Added timing data to the commit message.
 diff.c                     | 34 ++++++++++++++++++++++++++++---
 t/t4015-diff-whitespace.sh | 41 ++++++++++++++++++++++++++++++++++----
 2 files changed, 68 insertions(+), 7 deletions(-)
diff --git a/diff.c b/diff.c
index 89559293e7..072b5bced6 100644
--- a/diff.c
+++ b/diff.c
@@ -792,9 +792,11 @@ static void moved_block_clear(struct moved_block *b)
 	memset(b, 0, sizeof(*b));
 }
 
+#define INDENT_BLANKLINE INT_MIN
+
 static void fill_es_indent_data(struct emitted_diff_symbol *es)
 {
-	unsigned int off = 0;
+	unsigned int off = 0, i;
 	int width = 0, tab_width = es->flags & WS_TAB_WIDTH_MASK;
 	const char *s = es->line;
 	const int len = es->len;
@@ -818,8 +820,18 @@ static void fill_es_indent_data(struct emitted_diff_symbol *es)
 		}
 	}
 
-	es->indent_off = off;
-	es->indent_width = width;
+	/* check if this line is blank */
+	for (i = off; i < len; i++)
+		if (!isspace(s[i]))
+		    break;
+
+	if (i == len) {
+		es->indent_width = INDENT_BLANKLINE;
+		es->indent_off = len;
+	} else {
+		es->indent_off = off;
+		es->indent_width = width;
+	}
 }
 
 static int compute_ws_delta(const struct emitted_diff_symbol *a,
@@ -834,6 +846,11 @@ static int compute_ws_delta(const struct emitted_diff_symbol *a,
 	    b_width = b->indent_width;
 	int delta;
 
+	if (a_width == INDENT_BLANKLINE && b_width == INDENT_BLANKLINE) {
+		*out = INDENT_BLANKLINE;
+		return 1;
+	}
+
 	if (a->s == DIFF_SYMBOL_PLUS)
 		delta = a_width - b_width;
 	else
@@ -877,6 +894,10 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
 	if (al != bl)
 		return 1;
 
+	/* If 'l' and 'cur' are both blank then they match. */
+	if (a_width == INDENT_BLANKLINE && c_width == INDENT_BLANKLINE)
+		return 0;
+
 	/*
 	 * The indent changes of the block are known and stored in pmb->wsd;
 	 * however we need to check if the indent changes of the current line
@@ -888,6 +909,13 @@ static int cmp_in_block_with_wsd(const struct diff_options *o,
 	else
 		delta = c_width - a_width;
 
+	/*
+	 * If the previous lines of this block were all blank then set its
+	 * whitespace delta.
+	 */
+	if (pmb->wsd == INDENT_BLANKLINE)
+		pmb->wsd = delta;
+
 	return !(delta == pmb->wsd && al - a_off == cl - c_off &&
 		 !memcmp(a, b, al) && !
 		 memcmp(a + a_off, c + c_off, al - a_off));
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index e023839ba6..9d6f88b07f 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1901,10 +1901,20 @@ test_expect_success 'compare whitespace delta incompatible with other space opti
 	test_i18ngrep allow-indentation-change err
 '
 
+EMPTY=''
 test_expect_success 'compare mixed whitespace delta across moved blocks' '
 
 	git reset --hard &&
 	tr Q_ "\t " <<-EOF >text.txt &&
+	${EMPTY}
+	____too short without
+	${EMPTY}
+	___being grouped across blank line
+	${EMPTY}
+	context
+	lines
+	to
+	anchor
 	____Indented text to
 	_Q____be further indented by four spaces across
 	____Qseveral lines
@@ -1918,9 +1928,18 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '
 	git commit -m "add text.txt" &&
 
 	tr Q_ "\t " <<-EOF >text.txt &&
+	context
+	lines
+	to
+	anchor
 	QIndented text to
 	QQbe further indented by four spaces across
 	Q____several lines
+	${EMPTY}
+	QQtoo short without
+	${EMPTY}
+	Q_______being grouped across blank line
+	${EMPTY}
 	Q_QThese two lines have had their
 	indentation reduced by four spaces
 	QQdifferent indentation change
@@ -1937,7 +1956,16 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '
 	<BOLD>diff --git a/text.txt b/text.txt<RESET>
 	<BOLD>--- a/text.txt<RESET>
 	<BOLD>+++ b/text.txt<RESET>
-	<CYAN>@@ -1,7 +1,7 @@<RESET>
+	<CYAN>@@ -1,16 +1,16 @@<RESET>
+	<BOLD;MAGENTA>-<RESET>
+	<BOLD;MAGENTA>-<RESET><BOLD;MAGENTA>    too short without<RESET>
+	<BOLD;MAGENTA>-<RESET>
+	<BOLD;MAGENTA>-<RESET><BOLD;MAGENTA>   being grouped across blank line<RESET>
+	<BOLD;MAGENTA>-<RESET>
+	 <RESET>context<RESET>
+	 <RESET>lines<RESET>
+	 <RESET>to<RESET>
+	 <RESET>anchor<RESET>
 	<BOLD;MAGENTA>-<RESET><BOLD;MAGENTA>    Indented text to<RESET>
 	<BOLD;MAGENTA>-<RESET><BRED> <RESET>	<BOLD;MAGENTA>    be further indented by four spaces across<RESET>
 	<BOLD;MAGENTA>-<RESET><BRED>    <RESET>	<BOLD;MAGENTA>several lines<RESET>
@@ -1948,9 +1976,14 @@ test_expect_success 'compare mixed whitespace delta across moved blocks' '
 	<BOLD;CYAN>+<RESET>	<BOLD;CYAN>Indented text to<RESET>
 	<BOLD;CYAN>+<RESET>		<BOLD;CYAN>be further indented by four spaces across<RESET>
 	<BOLD;CYAN>+<RESET>	<BOLD;CYAN>    several lines<RESET>
-	<BOLD;YELLOW>+<RESET>	<BRED> <RESET>	<BOLD;YELLOW>These two lines have had their<RESET>
-	<BOLD;YELLOW>+<RESET><BOLD;YELLOW>indentation reduced by four spaces<RESET>
-	<BOLD;CYAN>+<RESET>		<BOLD;CYAN>different indentation change<RESET>
+	<BOLD;YELLOW>+<RESET>
+	<BOLD;YELLOW>+<RESET>		<BOLD;YELLOW>too short without<RESET>
+	<BOLD;YELLOW>+<RESET>
+	<BOLD;YELLOW>+<RESET>	<BOLD;YELLOW>       being grouped across blank line<RESET>
+	<BOLD;YELLOW>+<RESET>
+	<BOLD;CYAN>+<RESET>	<BRED> <RESET>	<BOLD;CYAN>These two lines have had their<RESET>
+	<BOLD;CYAN>+<RESET><BOLD;CYAN>indentation reduced by four spaces<RESET>
+	<BOLD;YELLOW>+<RESET>		<BOLD;YELLOW>different indentation change<RESET>
 	<GREEN>+<RESET><BRED>  <RESET>	<GREEN>too short<RESET>
 	EOF
 

-- 
2.19.1
Previous: Phillip WoodNext: Stefan Beller
Message 22 of 44 in “diff --color-moved-ws: allow mixed spaces and tabs in indentation change”
  1. 0/3 diff --color-moved-ws: allow mixed spaces and tabs in indentation changePhillip Wood, Sep 24, 2018
  2. 1/3 xdiff-interface: make xdl_blankline() availablePhillip Wood, Sep 24, 2018
  3. Stefan BellerSep 24, 2018
  4. 2/3 diff.c: remove unused variablesPhillip Wood, Sep 24, 2018
  5. 3/3 diff: add --color-moved-ws=allow-mixed-indentation-changePhillip Wood, Sep 24, 2018
  6. Stefan BellerSep 25, 2018
  7. 3/3 diff: add --color-moved-ws=allow-mixed-indentation-changePhillip Wood, Oct 9, 2018
  8. Stefan BellerOct 9, 2018
  9. Phillip WoodOct 10, 2018
  10. Stefan BellerOct 10, 2018
  11. Phillip WoodSep 24, 2018
  12. 0/9 diff --color-moved-ws fixes and enhancmentPhillip Wood, Nov 16, 2018
  13. 1/9 diff: document --no-color-movedPhillip Wood, Nov 16, 2018
  14. 7/9 diff --color-moved-ws: optimize allow-indentation-changePhillip Wood, Nov 16, 2018
  15. Stefan BellerNov 16, 2018
  16. Phillip WoodNov 17, 2018
  17. 4/9 diff --color-moved-ws: demonstrate false positivesPhillip Wood, Nov 16, 2018
  18. 8/9 diff --color-moved-ws: modify allow-indentation-changePhillip Wood, Nov 16, 2018
  19. Stefan BellerNov 16, 2018
  20. Phillip WoodNov 17, 2018
  21. 6/9 diff --color-moved=zebra: be stricter with color alternationPhillip Wood, Nov 16, 2018
  22. 9/9 diff --color-moved-ws: handle blank linesPhillip Wood, Nov 16, 2018
  23. Stefan BellerNov 20, 2018
  24. Phillip WoodNov 21, 2018
  25. 5/9 diff --color-moved-ws: fix false positivesPhillip Wood, Nov 16, 2018
  26. 3/9 diff: allow --no-color-moved-wsPhillip Wood, Nov 16, 2018
  27. 2/9 diff: use whitespace consistentlyPhillip Wood, Nov 16, 2018
  28. Stefan BellerNov 16, 2018
  29. 0/9 diff --color-moved-ws fixes and enhancmentPhillip Wood, Nov 23, 2018
  30. 1/9 diff: document --no-color-movedPhillip Wood, Nov 23, 2018
  31. 5/9 diff --color-moved-ws: fix false positivesPhillip Wood, Nov 23, 2018
  32. 4/9 diff --color-moved-ws: demonstrate false positivesPhillip Wood, Nov 23, 2018
  33. 6/9 diff --color-moved=zebra: be stricter with color alternationPhillip Wood, Nov 23, 2018
  34. 7/9 diff --color-moved-ws: optimize allow-indentation-changePhillip Wood, Nov 23, 2018
  35. 8/9 diff --color-moved-ws: modify allow-indentation-changePhillip Wood, Nov 23, 2018
  36. 9/9 diff --color-moved-ws: handle blank linesPhillip Wood, Nov 23, 2018
  37. 3/9 diff: allow --no-color-moved-wsPhillip Wood, Nov 23, 2018
  38. 2/9 Use "whitespace" consistentlyPhillip Wood, Nov 23, 2018
  39. Stefan BellerNov 26, 2018
  40. Phillip WoodNov 27, 2018
  41. Phillip WoodJan 8, 2019
  42. Junio C HamanoJan 8, 2019
  43. Stefan BellerJan 10, 2019
  44. Junio C HamanoJan 10, 2019

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.