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

> 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

> 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);

> 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
> @@ -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
> +	'