Re: [PATCH v3 01/11] Start to implement a built-in version of `git add --interactive`

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 31, 2019, 17:52 UTC
Message-ID: <xmqq1ry6qeg2.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <0a5ec9345d2f9cc6cd348231219d4af428a28e94.1563289115.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +add.interactive.useBuiltin::

I am not sure if three-level name is a good thing to use here.

If we have end-user controllable (like branch or remote names) unbounded number of subcommand/submode to "add", and "interactive" is merely one of it, then three-level name is a perfect fit, but otherwise, not.

Show 12 quoted lines

> @@ -185,6 +186,14 @@ int run_add_interactive(const char *revision, const char *patch_mode,
>  {
>  	int status, i;
>  	struct argv_array argv = ARGV_ARRAY_INIT;
> +	int use_builtin_add_i =
> +		git_env_bool("GIT_TEST_ADD_I_USE_BUILTIN", -1);
> +	if (use_builtin_add_i < 0)
> +		git_config_get_bool("add.interactive.usebuiltin",
> +				    &use_builtin_add_i);
> +
> +	if (use_builtin_add_i == 1 && !patch_mode)
> +		return !!run_add_i(the_repository, pathspec);

I am hoping that eventually "add -p" will also be routed to the new codepath. Would it make sense to have "&& !patch_mode" here, especially at this step where run_add_i() won't do anything useful anyway yet?

Show 7 quoted lines

> @@ -319,6 +328,7 @@ static int add_config(const char *var, const char *value, void *cb)
>  		ignore_add_errors = git_config_bool(var, value);
>  		return 0;
>  	}
> +
>  	return git_default_config(var, value, cb);
>  }

Good addition while at it.