From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 30 Sep 2025 15:26:36 GMT
Subject: Re: [PATCH v2 2/3] config: values of pathname type can be prefixed with :(optional)
Message-ID: <a687ec17-8ee4-428e-bae5-063716d59a08@gmail.com>
In-Reply-To: <5c97f580a9e77c464bc6bf4ed9ea8546711c6637.1759094936.git.ben.knoble+github@gmail.com>

Hi Ben

On 28/09/2025 22:29, D. Ben Knoble wrote:
> From: Junio C Hamano <gitster@pobox.com>
> 
> Sometimes people want to specify additional configuration data
> as "best effort" basis.  Maybe commit.template configuration file points
> at somewhere in ~/template/ but on a particular system, the file may not
> exist and the user may be OK without using the template in such a case.
> 
> When the value given to a configuration variable whose type is
> pathname wants to signal such an optional file, it can be marked by
> prepending ":(optional)" in front of it.  Such a setting that is
> marked optional would avoid getting the command barf for a missing
> file, as an optional configuration setting that names a missing
> file is not even seen.

I think this would be a useful addition, we've had several people 
wanting to make blame.ignoreRevsFile optional and this provides a 
general way to do that.

> --- a/config.c
> +++ b/config.c
> @@ -1279,11 +1279,23 @@ int git_config_string(char **dest, const char *var, const char *value)
>   
>   int git_config_pathname(char **dest, const char *var, const char *value)
>   {
> +	int is_optional;

This could be bool rather than int, the rest of the implementation looks 
good.

> --- a/t/t7500-commit-template-squash-signoff.sh
> +++ b/t/t7500-commit-template-squash-signoff.sh
> @@ -46,6 +46,15 @@ commit_msg_is ()
>   	)
>   '
>   
> +test_expect_success 'nonexistent optional template file in config' '
> +	test_config commit.template ":(optional)$PWD"/notexist &&
> +	(
> +		GIT_EDITOR="echo hello >\"\$1\"" &&

when git runs the editor this will be expanded to

     sh -c 'echo hello >"$1" "$@"' 'echo hello >"$1"' path/to/file

I think it should be

     GIT_EDITOR="echo hello >"

instead
> +		export GIT_EDITOR &&
> +		git commit --allow-empty

Maybe I'm missing something but don't we want to ensure that we have a 
non-empty message here? Also as it is a single command we can avoid the 
subshell with

     GIT_EDITOR="echo hello >" git commit

Thanks

Phillip