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

[PATCH 04/13] diff: simplify color_moved check when flushing

From
Jeff King <peff@peff.net>
Date
Sep 16, 2025, 20:17 UTC
Message-ID
<20250916201719.GD612873@coredump.intra.peff.net>
In-Reply-To
<20250916201036.GA612463@coredump.intra.peff.net>

In diff_flush_patch_all_file_pairs(), we set o->emitted_symbols if and only if o->color_moved is true. That causes the lower-level routines to fill up o->emitted_symbols, which we then analyze in order to do the actual colorizing.

But in that final step, we do:
  if (o->emitted_symbols) {
          if (o->color_moved) {
	     ...actual coloring...
	  }
	  ...clean up of emitted_symbols...
  }

The inner "if" will always trigger, since we set emitted_symbols only when doing color_moved (it is a little confusing that it is set inside the diff_options struct, but that is for convenience of passing it to the lower-level routines; we always clear it at the end of flushing, since 48edf3a02a (diff: clear emitted_symbols flag after use, 2019-01-24)).

Let's simplify the code a bit by just dropping the inner "if" and running its block unconditionally.

In theory the current code might be useful if another feature besides color_moved setup and used emitted_symbols, but it would be easy to refactor later to handle that. And in the meantime, this makes further work in this area easier.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/diff.c b/diff.c
index 64a4bd23ea..e6c85c8491 100644
--- a/diff.c
+++ b/diff.c
@@ -6746,20 +6746,17 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
 	}
 
 	if (o->emitted_symbols) {
-		if (o->color_moved) {
-			struct mem_pool entry_pool;
-			struct moved_entry_list *entry_list;
-
-			mem_pool_init(&entry_pool, 1024 * 1024);
-			entry_list = add_lines_to_move_detection(o,
-								 &entry_pool);
-			mark_color_as_moved(o, entry_list);
-			if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
-				dim_moved_lines(o);
-
-			mem_pool_discard(&entry_pool, 0);
-			free(entry_list);
-		}
+		struct mem_pool entry_pool;
+		struct moved_entry_list *entry_list;
+
+		mem_pool_init(&entry_pool, 1024 * 1024);
+		entry_list = add_lines_to_move_detection(o, &entry_pool);
+		mark_color_as_moved(o, entry_list);
+		if (o->color_moved == COLOR_MOVED_ZEBRA_DIM)
+			dim_moved_lines(o);
+
+		mem_pool_discard(&entry_pool, 0);
+		free(entry_list);
 
 		for (i = 0; i < esm.nr; i++)
 			emit_diff_symbol_from_struct(o, &esm.buf[i]);
-- 
2.51.0.527.g34bc42dacd
Previous: Jeff KingNext: Jeff King
Message 6 of 18 in “unraveling the mysteries of color variables”
  1. 0/13 unraveling the mysteries of color variablesJeff King, Sep 16, 2025
  2. Jeff KingSep 16, 2025
  3. 01/13 color: use GIT_COLOR_* instead of numeric constantsJeff King, Sep 16, 2025
  4. 02/13 color: return enum from git_config_colorbool()Jeff King, Sep 16, 2025
  5. 03/13 grep: don't treat grep_opt.color as a strict boolJeff King, Sep 16, 2025
  6. 04/13 diff: simplify color_moved check when flushingJeff King, Sep 16, 2025
  7. 05/13 diff: don't use diff_options.use_color as a strict boolJeff King, Sep 16, 2025
  8. 06/13 diff: pass o->use_color directly to fill_metainfo()Jeff King, Sep 16, 2025
  9. 07/13 diff: stop passing ecbdata->use_color as booleanJeff King, Sep 16, 2025
  10. 08/13 pretty: use format_commit_context.auto_color as colorboolJeff King, Sep 16, 2025
  11. 09/13 color: use git_colorbool enum to type to store colorboolsJeff King, Sep 16, 2025
  12. 9/13 color: use git_colorbool enum type to store colorboolsJeff King, Sep 16, 2025
  13. 10/13 color: return bool from want_color()Jeff King, Sep 16, 2025
  14. 11/13 add-interactive: retain colorbool values longerJeff King, Sep 16, 2025
  15. 12/13 config: store want_color() result in a separate boolJeff King, Sep 16, 2025
  16. 13/13 color: convert git_colorbool into a structJeff King, Sep 16, 2025
  17. Junio C HamanoSep 16, 2025
  18. Jeff KingSep 16, 2025

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.