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

[PATCH 3/4] add-interactive: manually fall back color config to color.ui

From
Jeff King <peff@peff.net>
Date
Aug 21, 2025, 07:22 UTC
Message-ID
<20250821072224.GC1839835@coredump.intra.peff.net>
In-Reply-To
<20250821070740.GA3356411@coredump.intra.peff.net>

Color options like color.interactive and color.diff should fall back to the value of color.ui if they aren't set. In add-interactive, we check the specific options (e.g., color.diff) via repo_config_get_value(), which does not depend on the main command having loaded any color config via the git_config() callback mechanism.

But then we call want_color() on the result; if our specific config is unset then that function uses the value of git_use_color_default. That variable is typically set from color.ui by the git_color_config() callback, which is called by the main command in its own git_config() callback function.

This works fine for "add -p", whose add_config() callback calls into git_color_config(). But it doesn't work for other commands like "checkout -p", which is otherwise unaware of color at all. People tend not to notice because the default is "auto", and that's what they'd set color.ui to as well. But something like:

  git -c color.ui=false checkout -p
should disable color, and it doesn't.

This regression goes back to 0527ccb1b5 (add -i: default to the built-in implementation, 2021-11-30). In the perl version we got the color config from "git config --get-colorbool", which did the full lookup for us.

The obvious fix is for git-checkout to add a call to git_color_config() to its own config callback. But we'd have to do so for every command with this problem, which is error-prone. Let's see if we can fix it more centrally.

It is tempting to teach want_color() to look up the value of repo_config_get_value("color.ui") itself. But I think that would have disastrous consequences. Plumbing commands, especially older ones, avoid porcelain config like color. by simply not parsing it in their config callbacks. Looking up the value of color.ui under the hood would undermine that.

Instead, let's do that lookup in the add-interactive setup code. We're already demand-loading other color config there, which is probably fine (even in a plumbing command like "git reset", the interactive mode is inherently porcelain-ish). That catches all commands that use the interactive code, whether they were calling git_color_config() themselves or not.

Reported-by: Isaac Oscar Gariano <isaacoscar@live.com.au>
Signed-off-by: Jeff King <peff@peff.net>
---
 add-interactive.c          |  9 +++++++++
 t/t3701-add-interactive.sh | 15 +++++++++++++++
 2 files changed, 24 insertions(+)
diff --git a/add-interactive.c b/add-interactive.c
index 95ab251963..db7e6a81a8 100644
--- a/add-interactive.c
+++ b/add-interactive.c
@@ -45,6 +45,15 @@ static int check_color_config(struct repository *r, const char *var)
 		ret = -1;
 	else
 		ret = git_config_colorbool(var, value);
+
+	/*
+	 * Do not rely on want_color() to fall back to color.ui for us. It uses
+	 * the value parsed by git_color_config(), which may not have been
+	 * called by the main command.
+	 */
+	if (ret < 0 && !repo_config_get_value(r, "color.ui", &value))
+		ret = git_config_colorbool("color.ui", value);
+
 	return want_color(ret);
 }
 
diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
index 3f9cb9453f..0024991257 100755
--- a/t/t3701-add-interactive.sh
+++ b/t/t3701-add-interactive.sh
@@ -1319,6 +1319,12 @@ test_expect_success 'stash accepts -U and --inter-hunk-context' '
 	test_grep "@@ -2,20 +2,20 @@" actual
 '
 
+test_expect_success 'set up base for -p color tests' '
+	echo commit >file &&
+	git commit -am "commit state" &&
+	git tag patch-base
+'
+
 for cmd in add checkout commit reset restore "stash save" "stash push"
 do
 	test_expect_success "$cmd rejects invalid context options" '
@@ -1335,6 +1341,15 @@ do
 		test_must_fail git $cmd --inter-hunk-context 2 2>actual &&
 		test_grep -E ".--inter-hunk-context. requires .(--interactive/)?--patch." actual
 	'
+
+	test_expect_success "$cmd falls back to color.ui" '
+		git reset --hard patch-base &&
+		echo working-tree >file &&
+		test_write_lines y |
+		force_color git -c color.ui=false $cmd -p >output.raw 2>&1 &&
+		test_decode_color <output.raw >output &&
+		test_cmp output.raw output
+	'
 done
 
 test_done
-- 
2.51.0.356.g99d8374de0
Previous: Patrick SteinhardtNext: Junio C Hamano
Message 13 of 23 in “[BUG] Some subcommands ignore color.diff and color.ui in --patch mode”
  1. Isaac Oscar GarianoAug 20, 2025
  2. Jeff KingAug 20, 2025
  3. Isaac Oscar GarianoAug 20, 2025
  4. Jeff KingAug 21, 2025
  5. 0/4 oddities around add-interactive and colorJeff King, Aug 21, 2025
  6. 1/4 stash: pass --no-color to diff-tree child processesJeff King, Aug 21, 2025
  7. Patrick SteinhardtSep 3, 2025
  8. Jeff KingSep 8, 2025
  9. 2/4 add-interactive: respect color.diff for diff coloringJeff King, Aug 21, 2025
  10. Patrick SteinhardtSep 3, 2025
  11. Jeff KingSep 8, 2025
  12. Patrick SteinhardtSep 9, 2025
  13. 3/4 add-interactive: manually fall back color config to color.uiJeff King, Aug 21, 2025
  14. Junio C HamanoAug 21, 2025
  15. Patrick SteinhardtSep 3, 2025
  16. Jeff KingSep 8, 2025
  17. 4/4 contrib/diff-highlight: mention interactive.diffFilterJeff King, Aug 21, 2025
  18. 0/4 oddities around add-interactive and colorJeff King, Sep 8, 2025
  19. 1/4 stash: pass --no-color to diff plumbing child processesJeff King, Sep 8, 2025
  20. 2/4 add-interactive: respect color.diff for diff coloringJeff King, Sep 8, 2025
  21. 3/4 add-interactive: manually fall back color config to color.uiJeff King, Sep 8, 2025
  22. 4/4 contrib/diff-highlight: mention interactive.diffFilterJeff King, Sep 8, 2025
  23. Patrick SteinhardtSep 9, 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.