Re: [PATCH 2/2] add -i: default to the built-in implementation
From: Johannes Schindelin <hidden>
Date: 2021-12-02 15:02:48
Hi Phillip, On Wed, 1 Dec 2021, Phillip Wood wrote:
On 30/11/2021 14:14, Johannes Schindelin via GitGitGadget wrote: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, constchar *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) {This could be simplified to "if (use_builtin_add_i)" but don't re-roll just for that
I was actually considering this, given that Git's coding practice suggests
precisely the form you suggested.
However, in this instance I found that form misleading: it would read to
me as if `use_builtin_add_i` was a Boolean. But it is a tristate, it can
also be `-1` ("undecided"). And I wanted to express "if this is not set to
`false` specifically", therefore I ended up with my proposal.
Ciao,
Dscho