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

Hi Junio,

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

"Johannes Schindelin via GitGitGadget" [off-list ref]
writes:

[...]

quoted

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