Re: [PATCH 07/15] blame: handle NULL values for color.blame.* config

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Thomas

On 07/11/2026 08:42, Thomas Bachem via GitGitGadget wrote:
> 
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 48d5251c6d..653449c0c9 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -782,12 +782,16 @@ static int git_blame_config(const char *var, const char *value,
>   		return 0;
>   	}
>   	if (!strcmp(var, "color.blame.repeatedlines")) {
> +		if (!value)
> +			return config_error_nonbool(var);
>   		if (color_parse_mem(value, strlen(value), repeated_meta_color))
>   			warning(_("invalid value for '%s': '%s'"),
>   				"color.blame.repeatedLines", value);
>   		return 0;

We ignore an invalid value, so does it make sense to error out on a 
missing value? The existing warning does get a bit lost though as it is 
printed before the blame output.

>   	}
>   	if (!strcmp(var, "color.blame.highlightrecent")) {
> +		if (!value)
> +			return config_error_nonbool(var);
>   		parse_color_fields(value);
>   		return 0;

Same here

Thanks

Phillip


    	}
> diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh
> index d2653b3c31..5eeb7338a3 100755
> --- a/t/t8012-blame-colors.sh
> +++ b/t/t8012-blame-colors.sh
> @@ -69,4 +69,11 @@ test_expect_success 'blame color by age: new code is different' '
>   	test_grep qfunc colored
>   '
>   
> +test_expect_success 'color.blame.repeatedLines and highlightRecent need a value' '
> +	test_must_fail git -c color.blame.repeatedLines blame hello.c 2>err &&
> +	test_grep "missing value for .color.blame.repeatedlines." err &&
> +	test_must_fail git -c color.blame.highlightRecent blame hello.c 2>err &&
> +	test_grep "missing value for .color.blame.highlightrecent." err
> +'
> +
>   test_done