Re: [PATCH 2/2] add -i: default to the built-in implementation

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Junio,

On Wed, 1 Dec 2021, Junio C Hamano wrote:

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> [...]
> > diff --git a/builtin/add.c b/builtin/add.c
> > index ef6b619c45e..8ef230a345b 100644
> > --- a/builtin/add.c
> > +++ b/builtin/add.c
> > @@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,
> >  	int use_builtin_add_i =
> >  		git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1);
> >
> > -	if (use_builtin_add_i < 0) {
> > -		int experimental;
> > -		if (!git_config_get_bool("add.interactive.usebuiltin",
> > -					 &use_builtin_add_i))
> > -			; /* ok */
> > -		else if (!git_config_get_bool("feature.experimental", &experimental) &&
> > -			 experimental)
> > -			use_builtin_add_i = 1;
> > -	}
> > +	if (use_builtin_add_i < 0 &&
> > +	    git_config_get_bool("add.interactive.usebuiltin",
> > +				&use_builtin_add_i))
> > +		use_builtin_add_i = 1;
> >
> > -	if (use_builtin_add_i == 1) {
> > +	if (use_builtin_add_i != 0) {
>
> Nit.
>
> 	if (use_builtin_add_i) {
>
> I wondered if these random calls to git_config_get_X() should be
> consolidated into the existing add_config() callback, but this
> conditional will go away hopefully in a few releases, so it probably
> is not worth it.  Inheriting the way the original code grabbed the
> configuration variables is good enough for our purpose here.

As I said in my reply to Phillip, I found it misleading to skip the `!= 0`
because we are catching both the `== 1` as well as the `== -1` here.

Ciao,
Dscho