"Johannes Schindelin via GitGitGadget" <gitgitgad...@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.

> @@ -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?

> @@ -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.

Reply via email to