{"thread":{"id":"57002","subject":"[PATCH 0/2] Use the built-in implementation of the interactive add command by default","startedAt":"2021-11-30T14:14:23Z","lastAt":"2021-12-10T22:59:27Z","messageCount":23,"participants":["Johannes Schindelin via GitGitGadget","Jeff King","Junio C Hamano","Phillip Wood","Ævar Arnfjörð Bjarmason","Carlo Arenas","Johannes Schindelin","Ramsay Jones","Philippe Blain"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"442662","messageId":"pull.1087.git.1638281655.gitgitgadget@gmail.com","threadId":"57002","inReplyTo":null,"subject":"[PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-30T14:14:13Z","receivedAt":"2021-11-30T14:14:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Over two years ago, Slavica Đukić participated in the Outreachy project,\nstarting to implement a built-in version of the interactive git add command.\nA little over a year ago, Git turned on that mode whenever users were\nrunning with feature.experimental = true.\n\nIt is time to declare this implementation robust, to use it by default, and\nto start deprecating the scripted implementation.\n\nJohannes Schindelin (2):\n  t2016: require the PERL prereq only when necessary\n  add -i: default to the built-in implementation\n\n Documentation/config/add.txt |  6 +++---\n builtin/add.c                | 15 +++++--------\n ci/run-build-and-tests.sh    |  2 +-\n t/README                     |  2 +-\n t/t2016-checkout-patch.sh    | 42 +++++++++++++++++++-----------------\n 5 files changed, 32 insertions(+), 35 deletions(-)\n\n\nbase-commit: abe6bb3905392d5eb6b01fa6e54d7e784e0522aa\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1087%2Fdscho%2Fdefault-to-builtin-add-p-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1087/dscho/default-to-builtin-add-p-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1087\n-- \ngitgitgadget\n"},{"id":"442663","messageId":"a0cf71031781b481b092d0f501bc4d78376543f3.1638281655.git.gitgitgadget@gmail.com","threadId":"57002","inReplyTo":"pull.1087.git.1638281655.gitgitgadget@gmail.com","subject":"[PATCH 1/2] t2016: require the PERL prereq only when necessary","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-30T14:14:14Z","receivedAt":"2021-11-30T14:14:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nThe scripted version of the interactive mode of `git add` still requires\nPerl, but the built-in version does not. Let's only require the PERL\nprereq if testing the scripted version.\n\nThis addresses a long-standing NEEDSWORK added in 35166b1fb54 (t2016:\nadd a NEEDSWORK about the PERL prerequisite, 2020-10-07).\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n t/t2016-checkout-patch.sh | 42 ++++++++++++++++++++-------------------\n 1 file changed, 22 insertions(+), 20 deletions(-)\n\ndiff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh\nindex abfd586c32b..71c5a15be00 100755\n--- a/t/t2016-checkout-patch.sh\n+++ b/t/t2016-checkout-patch.sh\n@@ -4,7 +4,13 @@ test_description='git checkout --patch'\n \n . ./lib-patch-mode.sh\n \n-test_expect_success PERL 'setup' '\n+if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN false && ! test_have_prereq PERL\n+then\n+\tskip_all='skipping interactive add tests, PERL not set'\n+\ttest_done\n+fi\n+\n+test_expect_success 'setup' '\n \tmkdir dir &&\n \techo parent > dir/foo &&\n \techo dummy > bar &&\n@@ -18,44 +24,40 @@ test_expect_success PERL 'setup' '\n \n # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'\n \n-# NEEDSWORK: Since the builtin add-p is used when $GIT_TEST_ADD_I_USE_BUILTIN\n-# is given, we should replace the PERL prerequisite with an ADD_I prerequisite\n-# which first checks if $GIT_TEST_ADD_I_USE_BUILTIN is defined before checking\n-# PERL.\n-test_expect_success PERL 'saying \"n\" does nothing' '\n+test_expect_success 'saying \"n\" does nothing' '\n \tset_and_save_state dir/foo work head &&\n \ttest_write_lines n n | git checkout -p &&\n \tverify_saved_state bar &&\n \tverify_saved_state dir/foo\n '\n \n-test_expect_success PERL 'git checkout -p' '\n+test_expect_success 'git checkout -p' '\n \ttest_write_lines n y | git checkout -p &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo head head\n '\n \n-test_expect_success PERL 'git checkout -p with staged changes' '\n+test_expect_success 'git checkout -p with staged changes' '\n \tset_state dir/foo work index &&\n \ttest_write_lines n y | git checkout -p &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo index index\n '\n \n-test_expect_success PERL 'git checkout -p HEAD with NO staged changes: abort' '\n+test_expect_success 'git checkout -p HEAD with NO staged changes: abort' '\n \tset_and_save_state dir/foo work head &&\n \ttest_write_lines n y n | git checkout -p HEAD &&\n \tverify_saved_state bar &&\n \tverify_saved_state dir/foo\n '\n \n-test_expect_success PERL 'git checkout -p HEAD with NO staged changes: apply' '\n+test_expect_success 'git checkout -p HEAD with NO staged changes: apply' '\n \ttest_write_lines n y y | git checkout -p HEAD &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo head head\n '\n \n-test_expect_success PERL 'git checkout -p HEAD with change already staged' '\n+test_expect_success 'git checkout -p HEAD with change already staged' '\n \tset_state dir/foo index index &&\n \t# the third n is to get out in case it mistakenly does not apply\n \ttest_write_lines n y n | git checkout -p HEAD &&\n@@ -63,21 +65,21 @@ test_expect_success PERL 'git checkout -p HEAD with change already staged' '\n \tverify_state dir/foo head head\n '\n \n-test_expect_success PERL 'git checkout -p HEAD^...' '\n+test_expect_success 'git checkout -p HEAD^...' '\n \t# the third n is to get out in case it mistakenly does not apply\n \ttest_write_lines n y n | git checkout -p HEAD^... &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo parent parent\n '\n \n-test_expect_success PERL 'git checkout -p HEAD^' '\n+test_expect_success 'git checkout -p HEAD^' '\n \t# the third n is to get out in case it mistakenly does not apply\n \ttest_write_lines n y n | git checkout -p HEAD^ &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo parent parent\n '\n \n-test_expect_success PERL 'git checkout -p handles deletion' '\n+test_expect_success 'git checkout -p handles deletion' '\n \tset_state dir/foo work index &&\n \trm dir/foo &&\n \ttest_write_lines n y | git checkout -p &&\n@@ -90,28 +92,28 @@ test_expect_success PERL 'git checkout -p handles deletion' '\n # dir/foo.  There's always an extra 'n' to reject edits to dir/foo in\n # the failure case (and thus get out of the loop).\n \n-test_expect_success PERL 'path limiting works: dir' '\n+test_expect_success 'path limiting works: dir' '\n \tset_state dir/foo work head &&\n \ttest_write_lines y n | git checkout -p dir &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo head head\n '\n \n-test_expect_success PERL 'path limiting works: -- dir' '\n+test_expect_success 'path limiting works: -- dir' '\n \tset_state dir/foo work head &&\n \ttest_write_lines y n | git checkout -p -- dir &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo head head\n '\n \n-test_expect_success PERL 'path limiting works: HEAD^ -- dir' '\n+test_expect_success 'path limiting works: HEAD^ -- dir' '\n \t# the third n is to get out in case it mistakenly does not apply\n \ttest_write_lines y n n | git checkout -p HEAD^ -- dir &&\n \tverify_saved_state bar &&\n \tverify_state dir/foo parent parent\n '\n \n-test_expect_success PERL 'path limiting works: foo inside dir' '\n+test_expect_success 'path limiting works: foo inside dir' '\n \tset_state dir/foo work head &&\n \t# the third n is to get out in case it mistakenly does not apply\n \ttest_write_lines y n n | (cd dir && git checkout -p foo) &&\n@@ -119,11 +121,11 @@ test_expect_success PERL 'path limiting works: foo inside dir' '\n \tverify_state dir/foo head head\n '\n \n-test_expect_success PERL 'none of this moved HEAD' '\n+test_expect_success 'none of this moved HEAD' '\n \tverify_saved_head\n '\n \n-test_expect_success PERL 'empty tree can be handled' '\n+test_expect_success 'empty tree can be handled' '\n \ttest_when_finished \"git reset --hard\" &&\n \tgit checkout -p $(test_oid empty_tree) --\n '\n-- \ngitgitgadget\n\n"},{"id":"442664","messageId":"84824918ae4564a9194a1a55124ee8694f210437.1638281655.git.gitgitgadget@gmail.com","threadId":"57002","inReplyTo":"pull.1087.git.1638281655.gitgitgadget@gmail.com","subject":"[PATCH 2/2] add -i: default to the built-in implementation","fromName":"Johannes Schindelin via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-11-30T14:14:15Z","receivedAt":"2021-11-30T14:14:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"From: Johannes Schindelin <johannes.schindelin@gmx.de>\n\nIn 9a5315edfdf (Merge branch 'js/patch-mode-in-others-in-c',\n2020-02-05), Git acquired a built-in implementation of `git add`'s\ninteractive mode that could be turned on via the config option\n`add.interactive.useBuiltin`.\n\nThe first official Git version to support this knob was v2.26.0.\n\nIn 2df2d81ddd0 (add -i: use the built-in version when\nfeature.experimental is set, 2020-09-08), this built-in implementation\nwas also enabled via `feature.experimental`. The first version with this\nchange was v2.29.0.\n\nMore than a year (and very few bug reports) later, it is time to declare\nthe built-in implementation mature and to turn it on by default.\n\nWe specifically leave the `add.interactive.useBuiltin` configuration in\nplace, to give users an \"escape hatch\" in the unexpected case should\nthey encounter a previously undetected bug in that implementation.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n Documentation/config/add.txt |  6 +++---\n builtin/add.c                | 15 +++++----------\n ci/run-build-and-tests.sh    |  2 +-\n t/README                     |  2 +-\n t/t2016-checkout-patch.sh    |  2 +-\n 5 files changed, 11 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/config/add.txt b/Documentation/config/add.txt\nindex c9f748f81cb..3e859f34197 100644\n--- a/Documentation/config/add.txt\n+++ b/Documentation/config/add.txt\n@@ -7,6 +7,6 @@ add.ignore-errors (deprecated)::\n \tvariables.\n \n add.interactive.useBuiltin::\n-\t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n-\timplementation of the interactive version of linkgit:git-add[1]\n-\tinstead of the Perl script version. Is `false` by default.\n+\tSet to `false` to fall back to the original Perl implementation of\n+\tthe interactive version of linkgit:git-add[1] instead of the built-in\n+\tversion. Is `true` by default.\ndiff --git a/builtin/add.c b/builtin/add.c\nindex ef6b619c45e..8ef230a345b 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n \tint use_builtin_add_i =\n \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n \n-\tif (use_builtin_add_i < 0) {\n-\t\tint experimental;\n-\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n-\t\t\t\t\t &use_builtin_add_i))\n-\t\t\t; /* ok */\n-\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n-\t\t\t experimental)\n-\t\t\tuse_builtin_add_i = 1;\n-\t}\n+\tif (use_builtin_add_i < 0 &&\n+\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n+\t\t\t\t&use_builtin_add_i))\n+\t\tuse_builtin_add_i = 1;\n \n-\tif (use_builtin_add_i == 1) {\n+\tif (use_builtin_add_i != 0) {\n \t\tenum add_p_mode mode;\n \n \t\tif (!patch_mode)\ndiff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\nindex cc62616d806..660ebe8d108 100755\n--- a/ci/run-build-and-tests.sh\n+++ b/ci/run-build-and-tests.sh\n@@ -29,7 +29,7 @@ linux-gcc)\n \texport GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1\n \texport GIT_TEST_MULTI_PACK_INDEX=1\n \texport GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=1\n-\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n+\texport GIT_TEST_ADD_I_USE_BUILTIN=0\n \texport GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n \texport GIT_TEST_WRITE_REV_INDEX=1\n \texport GIT_TEST_CHECKOUT_WORKERS=2\ndiff --git a/t/README b/t/README\nindex 29f72354bf1..2c22337d6e7 100644\n--- a/t/README\n+++ b/t/README\n@@ -419,7 +419,7 @@ the --sparse command-line argument.\n GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path\n by overriding the minimum number of cache entries required per thread.\n \n-GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when true, enables the\n+GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when false, disables the\n built-in version of git add -i. See 'add.interactive.useBuiltin' in\n git-config(1).\n \ndiff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh\nindex 71c5a15be00..bc3f69b4b1d 100755\n--- a/t/t2016-checkout-patch.sh\n+++ b/t/t2016-checkout-patch.sh\n@@ -4,7 +4,7 @@ test_description='git checkout --patch'\n \n . ./lib-patch-mode.sh\n \n-if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN false && ! test_have_prereq PERL\n+if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN true && ! test_have_prereq PERL\n then\n \tskip_all='skipping interactive add tests, PERL not set'\n \ttest_done\n-- \ngitgitgadget\n"},{"id":"442682","messageId":"YaaP/g74KA63MCmx@coredump.intra.peff.net","threadId":"57002","inReplyTo":"pull.1087.git.1638281655.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-11-30T20:56:30Z","receivedAt":"2021-11-30T20:57:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 30, 2021 at 02:14:13PM +0000, Johannes Schindelin via GitGitGadget wrote:\n\n> Over two years ago, Slavica Đukić participated in the Outreachy project,\n> starting to implement a built-in version of the interactive git add command.\n> A little over a year ago, Git turned on that mode whenever users were\n> running with feature.experimental = true.\n> \n> It is time to declare this implementation robust, to use it by default, and\n> to start deprecating the scripted implementation.\n\nYay. I agree it is time.\n\nIt's still possible there are bugs that feature.experimental folks\nmissed, but at some point we need to flip this switch to get the\nexposure to find those bugs. Doing it early in a cycle makes sense.\n\nThe patches themselves look good to me. I look forward to dropping the\nperl version entirely, just to reduce the duplicated code, but I think\nyour approach of leaving it as an escape hatch for now makes sense in\nthe shorter term.\n\n-Peff\n"},{"id":"442687","messageId":"xmqqy2551h1f.fsf@gitster.g","threadId":"57002","inReplyTo":"pull.1087.git.1638281655.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-30T21:15:40Z","receivedAt":"2021-11-30T21:15:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> Over two years ago, Slavica Đukić participated in the Outreachy project,\n> starting to implement a built-in version of the interactive git add command.\n> A little over a year ago, Git turned on that mode whenever users were\n> running with feature.experimental = true.\n>\n> It is time to declare this implementation robust, to use it by default, and\n> to start deprecating the scripted implementation.\n\nYes, it is time to use it by default to expose any remaining bugs\nthat those with feature.experimental set failed to catch.\nUnfortunately, we do not catch issues real users would encounter in\nany \"opt-in\" feature, until it becomes \"opt-out\" and unconfigured\nusers are exposed to it.  This is true even in the presense of large\ncorporations that expose their internal users to versions based on\n'next' regularly.\n"},{"id":"442765","messageId":"a936a701-c13f-346e-69ee-85441ca20641@gmail.com","threadId":"57002","inReplyTo":"84824918ae4564a9194a1a55124ee8694f210437.1638281655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-12-01T11:20:31Z","receivedAt":"2021-12-01T11:20:46Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Dscho\n\nOn 30/11/2021 14:14, Johannes Schindelin via GitGitGadget wrote:\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> \n> In 9a5315edfdf (Merge branch 'js/patch-mode-in-others-in-c',\n> 2020-02-05), Git acquired a built-in implementation of `git add`'s\n> interactive mode that could be turned on via the config option\n> `add.interactive.useBuiltin`.\n> \n> The first official Git version to support this knob was v2.26.0.\n> \n> In 2df2d81ddd0 (add -i: use the built-in version when\n> feature.experimental is set, 2020-09-08), this built-in implementation\n> was also enabled via `feature.experimental`. The first version with this\n> change was v2.29.0.\n> \n> More than a year (and very few bug reports) later, it is time to declare\n> the built-in implementation mature and to turn it on by default.\n> \n> We specifically leave the `add.interactive.useBuiltin` configuration in\n> place, to give users an \"escape hatch\" in the unexpected case should\n> they encounter a previously undetected bug in that implementation.\n\nThanks for doing this, I agree it is time to switch over - it feels like \nit is quite a while since anyone reported a bug with the C version. Both \npatches look good to me. I've left one minor comment below but it is not \nworth re-rolling just for that. Thanks Slavica for your work on this, \nit's great to have it converted to C.\n\n\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>   Documentation/config/add.txt |  6 +++---\n>   builtin/add.c                | 15 +++++----------\n>   ci/run-build-and-tests.sh    |  2 +-\n>   t/README                     |  2 +-\n>   t/t2016-checkout-patch.sh    |  2 +-\n>   5 files changed, 11 insertions(+), 16 deletions(-)\n> \n> diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt\n> index c9f748f81cb..3e859f34197 100644\n> --- a/Documentation/config/add.txt\n> +++ b/Documentation/config/add.txt\n> @@ -7,6 +7,6 @@ add.ignore-errors (deprecated)::\n>   \tvariables.\n>   \n>   add.interactive.useBuiltin::\n> -\t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n> -\timplementation of the interactive version of linkgit:git-add[1]\n> -\tinstead of the Perl script version. Is `false` by default.\n> +\tSet to `false` to fall back to the original Perl implementation of\n> +\tthe interactive version of linkgit:git-add[1] instead of the built-in\n> +\tversion. Is `true` by default.\n> diff --git a/builtin/add.c b/builtin/add.c\n> index ef6b619c45e..8ef230a345b 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n>   \tint use_builtin_add_i =\n>   \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n>   \n> -\tif (use_builtin_add_i < 0) {\n> -\t\tint experimental;\n> -\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n> -\t\t\t\t\t &use_builtin_add_i))\n> -\t\t\t; /* ok */\n> -\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n> -\t\t\t experimental)\n> -\t\t\tuse_builtin_add_i = 1;\n> -\t}\n> +\tif (use_builtin_add_i < 0 &&\n> +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n> +\t\t\t\t&use_builtin_add_i))\n> +\t\tuse_builtin_add_i = 1;\n>   \n> -\tif (use_builtin_add_i == 1) {\n> +\tif (use_builtin_add_i != 0) {\n\nThis could be simplified to \"if (use_builtin_add_i)\" but don't re-roll \njust for that\n\nBest Wishes\n\nPhillip\n\n>   \t\tenum add_p_mode mode;\n>   \n>   \t\tif (!patch_mode)\n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index cc62616d806..660ebe8d108 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -29,7 +29,7 @@ linux-gcc)\n>   \texport GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1\n>   \texport GIT_TEST_MULTI_PACK_INDEX=1\n>   \texport GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=1\n> -\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> +\texport GIT_TEST_ADD_I_USE_BUILTIN=0\n>   \texport GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n>   \texport GIT_TEST_WRITE_REV_INDEX=1\n>   \texport GIT_TEST_CHECKOUT_WORKERS=2\n> diff --git a/t/README b/t/README\n> index 29f72354bf1..2c22337d6e7 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -419,7 +419,7 @@ the --sparse command-line argument.\n>   GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path\n>   by overriding the minimum number of cache entries required per thread.\n>   \n> -GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when true, enables the\n> +GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when false, disables the\n>   built-in version of git add -i. See 'add.interactive.useBuiltin' in\n>   git-config(1).\n>   \n> diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh\n> index 71c5a15be00..bc3f69b4b1d 100755\n> --- a/t/t2016-checkout-patch.sh\n> +++ b/t/t2016-checkout-patch.sh\n> @@ -4,7 +4,7 @@ test_description='git checkout --patch'\n>   \n>   . ./lib-patch-mode.sh\n>   \n> -if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN false && ! test_have_prereq PERL\n> +if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN true && ! test_have_prereq PERL\n>   then\n>   \tskip_all='skipping interactive add tests, PERL not set'\n>   \ttest_done\n> \n\n"},{"id":"442770","messageId":"211201.86pmqgbful.gmgdl@evledraar.gmail.com","threadId":"57002","inReplyTo":"84824918ae4564a9194a1a55124ee8694f210437.1638281655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-01T13:37:47Z","receivedAt":"2021-12-01T13:43:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 30 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> In 9a5315edfdf (Merge branch 'js/patch-mode-in-others-in-c',\n> 2020-02-05), Git acquired a built-in implementation of `git add`'s\n> interactive mode that could be turned on via the config option\n> `add.interactive.useBuiltin`.\n>\n> The first official Git version to support this knob was v2.26.0.\n>\n> In 2df2d81ddd0 (add -i: use the built-in version when\n> feature.experimental is set, 2020-09-08), this built-in implementation\n> was also enabled via `feature.experimental`. The first version with this\n> change was v2.29.0.\n>\n> More than a year (and very few bug reports) later, it is time to declare\n> the built-in implementation mature and to turn it on by default.\n>\n> We specifically leave the `add.interactive.useBuiltin` configuration in\n> place, to give users an \"escape hatch\" in the unexpected case should\n> they encounter a previously undetected bug in that implementation.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  Documentation/config/add.txt |  6 +++---\n>  builtin/add.c                | 15 +++++----------\n>  ci/run-build-and-tests.sh    |  2 +-\n>  t/README                     |  2 +-\n>  t/t2016-checkout-patch.sh    |  2 +-\n>  5 files changed, 11 insertions(+), 16 deletions(-)\n>\n> diff --git a/Documentation/config/add.txt b/Documentation/config/add.txt\n> index c9f748f81cb..3e859f34197 100644\n> --- a/Documentation/config/add.txt\n> +++ b/Documentation/config/add.txt\n> @@ -7,6 +7,6 @@ add.ignore-errors (deprecated)::\n>  \tvariables.\n>  \n>  add.interactive.useBuiltin::\n> -\t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n> -\timplementation of the interactive version of linkgit:git-add[1]\n> -\tinstead of the Perl script version. Is `false` by default.\n> +\tSet to `false` to fall back to the original Perl implementation of\n> +\tthe interactive version of linkgit:git-add[1] instead of the built-in\n> +\tversion. Is `true` by default.\n\nI think this would be a bit better if we just stole the version you\nadded for stash.useBuiltin entirely. I.e. from your 336ad8424cb (stash:\ndocument stash.useBuiltin, 2019-05-14), with the relevant s/shell\nscript/Perl/g etc. replaced.\n\nI.e. that version encouraged users to report any bugs, because we were\nreally going to remove it soon, as we then did for rebase.useBuiltin in\n9bcde4d5314 (rebase: remove transitory rebase.useBuiltin setting & env,\n2021-03-23).\n\nThe wording in the opening paragraph is also a bit more to the point\nthere, i.e. calling it \"legacy\" rather than \"original [...]\nimplementation\".\n\n(I notice that the stash.useBuiltin is still there in-tree, hrm...)\n\n> diff --git a/builtin/add.c b/builtin/add.c\n> index ef6b619c45e..8ef230a345b 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n>  \tint use_builtin_add_i =\n>  \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n>  \n> -\tif (use_builtin_add_i < 0) {\n> -\t\tint experimental;\n> -\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n> -\t\t\t\t\t &use_builtin_add_i))\n> -\t\t\t; /* ok */\n> -\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n> -\t\t\t experimental)\n> -\t\t\tuse_builtin_add_i = 1;\n> -\t}\n> +\tif (use_builtin_add_i < 0 &&\n> +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n> +\t\t\t\t&use_builtin_add_i))\n> +\t\tuse_builtin_add_i = 1;\n>  \n> -\tif (use_builtin_add_i == 1) {\n> +\tif (use_builtin_add_i != 0) {\n\nStyle/idiom: This should just be \"if (use_builtin_add_i)\".\n\nI.e. before we cared about not catching -1 here, but now that it's true\nby default we don't care about the distinction between -1 or 1 anymore,\nwe just want it not to be 0 here.\n"},{"id":"442771","messageId":"211201.86lf14bfi8.gmgdl@evledraar.gmail.com","threadId":"57002","inReplyTo":"pull.1087.git.1638281655.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-01T13:43:37Z","receivedAt":"2021-12-01T13:53:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 30 2021, Johannes Schindelin via GitGitGadget wrote:\n\n> Over two years ago, Slavica Đukić participated in the Outreachy project,\n> starting to implement a built-in version of the interactive git add command.\n> A little over a year ago, Git turned on that mode whenever users were\n> running with feature.experimental = true.\n>\n> It is time to declare this implementation robust, to use it by default, and\n> to start deprecating the scripted implementation.\n>\n> Johannes Schindelin (2):\n>   t2016: require the PERL prereq only when necessary\n>   add -i: default to the built-in implementation\n\nI'm very happy to see this. I left some minor nits on 2/2[1], but\nwith/without those suggested changes this LGTM.\n\nI was a tad surprised that feature.experimental=false doesn't disable\nthis anymore, but after looking into it a bit that's how we should be\ndoing this. I.e. the life cycle for these has been\n\n    opt in setting [&& experimental] -> opt-out setting [&& !experimental] -> remove opt-out\n\nIf you're intending to re-roll anyway I think a brief mention of that\nbeing intended & correct would be nice.\n\nI.e. I went looking down that rabbit hole since there was no mention of\nit in the commit message, and wondered if it was intentional & correct,\nwhich I then found it is (well, correct, but I'm assuming also\nintentional).\n\nThanks!\n\n1. https://lore.kernel.org/git/211201.86pmqgbful.gmgdl@evledraar.gmail.com/\n"},{"id":"442804","messageId":"CAPUEsphxCcfOgPQP9ci6sSn87g5=w+8kjh21=QywTX=on5vTdw@mail.gmail.com","threadId":"57002","inReplyTo":"pull.1087.git.1638281655.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-12-01T21:24:15Z","receivedAt":"2021-12-01T21:24:29Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Wed, Dec 1, 2021 at 12:40 AM Johannes Schindelin via GitGitGadget\n<gitgitgadget@gmail.com> wrote:\n> It is time to declare this implementation robust, to use it by default, and\n> to start deprecating the scripted implementation.\n>\n> Johannes Schindelin (2):\n>   t2016: require the PERL prereq only when necessary\n>   add -i: default to the built-in implementation\n\nSadly this implementation has a few bugs that still need fixing, with\nat least one IMHO being a showstopper.\n\nThe way macOS implements stdin (through a device) it will always\ntimeout in poll(), so escape keys that are left in the unread buffer\nand that could match some of the entries will result in the wrong\nentry being selected.\n\nI have a series[1] that reimplements this and that seemed to work fine\nin my tests while making the code simpler, but that I didn't\nprioritize (and wanted to clean up further) since I wanted to\nprioritize the EDITOR fixes in the same area.\n\nCarlo\n\n[1] https://github.com/git/git/pull/1150\n"},{"id":"442820","messageId":"xmqqlf14ynco.fsf@gitster.g","threadId":"57002","inReplyTo":"a0cf71031781b481b092d0f501bc4d78376543f3.1638281655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] t2016: require the PERL prereq only when necessary","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-01T22:25:11Z","receivedAt":"2021-12-01T22:25:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> The scripted version of the interactive mode of `git add` still requires\n> Perl, but the built-in version does not. Let's only require the PERL\n> prereq if testing the scripted version.\n>\n> This addresses a long-standing NEEDSWORK added in 35166b1fb54 (t2016:\n> add a NEEDSWORK about the PERL prerequisite, 2020-10-07).\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>  t/t2016-checkout-patch.sh | 42 ++++++++++++++++++++-------------------\n>  1 file changed, 22 insertions(+), 20 deletions(-)\n\nGood.  I suspect there may not be too many developers who lack PERL\nprerequisite around here, so it is not like the built-in version was\nnot sufficiently tested in developers' environment without this\npatch, but it is nice to see us move in this direction.  Of course,\nwhen we remove the non-builtin version, we'd further be able to lose\nthese prerequisites.\n\n> diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh\n> index abfd586c32b..71c5a15be00 100755\n> --- a/t/t2016-checkout-patch.sh\n> +++ b/t/t2016-checkout-patch.sh\n> @@ -4,7 +4,13 @@ test_description='git checkout --patch'\n>  \n>  . ./lib-patch-mode.sh\n>  \n> -test_expect_success PERL 'setup' '\n> +if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN false && ! test_have_prereq PERL\n> +then\n> +\tskip_all='skipping interactive add tests, PERL not set'\n> +\ttest_done\n> +fi\n> +\n> +test_expect_success 'setup' '\n>  \tmkdir dir &&\n>  \techo parent > dir/foo &&\n>  \techo dummy > bar &&\n> @@ -18,44 +24,40 @@ test_expect_success PERL 'setup' '\n>  \n>  # note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar'\n>  \n> -# NEEDSWORK: Since the builtin add-p is used when $GIT_TEST_ADD_I_USE_BUILTIN\n> -# is given, we should replace the PERL prerequisite with an ADD_I prerequisite\n> -# which first checks if $GIT_TEST_ADD_I_USE_BUILTIN is defined before checking\n> -# PERL.\n> -test_expect_success PERL 'saying \"n\" does nothing' '\n> +test_expect_success 'saying \"n\" does nothing' '\n\n"},{"id":"442822","messageId":"xmqqee6wymx2.fsf@gitster.g","threadId":"57002","inReplyTo":"84824918ae4564a9194a1a55124ee8694f210437.1638281655.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-01T22:34:33Z","receivedAt":"2021-12-01T22:34:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> From: Johannes Schindelin <johannes.schindelin@gmx.de>\n>\n> In 9a5315edfdf (Merge branch 'js/patch-mode-in-others-in-c',\n> 2020-02-05), Git acquired a built-in implementation of `git add`'s\n> interactive mode that could be turned on via the config option\n> `add.interactive.useBuiltin`.\n>\n> The first official Git version to support this knob was v2.26.0.\n>\n> In 2df2d81ddd0 (add -i: use the built-in version when\n> feature.experimental is set, 2020-09-08), this built-in implementation\n> was also enabled via `feature.experimental`. The first version with this\n> change was v2.29.0.\n>\n> More than a year (and very few bug reports) later, it is time to declare\n> the built-in implementation mature and to turn it on by default.\n\nI think that declaration is a bit premature.  I do agree that by now\nwe should have killed as many bugs as we could without unleashing it\ntot he general public by keeping it behind the experimental flag, and\nwe should move forward by turn it on by default.  In a month or two\nafter this change hits a stable release of major distros, we can learn\nthat the implementation was mature, but not until then ;-).\n\n\n> We specifically leave the `add.interactive.useBuiltin` configuration in\n> place, to give users an \"escape hatch\" in the unexpected case should\n> they encounter a previously undetected bug in that implementation.\n\nGood thinking.\n\n>  add.interactive.useBuiltin::\n> -\t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n> -\timplementation of the interactive version of linkgit:git-add[1]\n> -\tinstead of the Perl script version. Is `false` by default.\n> +\tSet to `false` to fall back to the original Perl implementation of\n> +\tthe interactive version of linkgit:git-add[1] instead of the built-in\n> +\tversion. Is `true` by default.\n> diff --git a/builtin/add.c b/builtin/add.c\n> index ef6b619c45e..8ef230a345b 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n>  \tint use_builtin_add_i =\n>  \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n>  \n> -\tif (use_builtin_add_i < 0) {\n> -\t\tint experimental;\n> -\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n> -\t\t\t\t\t &use_builtin_add_i))\n> -\t\t\t; /* ok */\n> -\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n> -\t\t\t experimental)\n> -\t\t\tuse_builtin_add_i = 1;\n> -\t}\n> +\tif (use_builtin_add_i < 0 &&\n> +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n> +\t\t\t\t&use_builtin_add_i))\n> +\t\tuse_builtin_add_i = 1;\n>  \n> -\tif (use_builtin_add_i == 1) {\n> +\tif (use_builtin_add_i != 0) {\n\nNit.\n\n\tif (use_builtin_add_i) {\n\nI wondered if these random calls to git_config_get_X() should be\nconsolidated into the existing add_config() callback, but this\nconditional will go away hopefully in a few releases, so it probably\nis not worth it.  Inheriting the way the original code grabbed the\nconfiguration variables is good enough for our purpose here.\n\n> diff --git a/ci/run-build-and-tests.sh b/ci/run-build-and-tests.sh\n> index cc62616d806..660ebe8d108 100755\n> --- a/ci/run-build-and-tests.sh\n> +++ b/ci/run-build-and-tests.sh\n> @@ -29,7 +29,7 @@ linux-gcc)\n>  \texport GIT_TEST_COMMIT_GRAPH_CHANGED_PATHS=1\n>  \texport GIT_TEST_MULTI_PACK_INDEX=1\n>  \texport GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP=1\n> -\texport GIT_TEST_ADD_I_USE_BUILTIN=1\n> +\texport GIT_TEST_ADD_I_USE_BUILTIN=0\n>  \texport GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\n>  \texport GIT_TEST_WRITE_REV_INDEX=1\n>  \texport GIT_TEST_CHECKOUT_WORKERS=2\n\nOK.\n\n> diff --git a/t/README b/t/README\n> index 29f72354bf1..2c22337d6e7 100644\n> --- a/t/README\n> +++ b/t/README\n> @@ -419,7 +419,7 @@ the --sparse command-line argument.\n>  GIT_TEST_PRELOAD_INDEX=<boolean> exercises the preload-index code path\n>  by overriding the minimum number of cache entries required per thread.\n>  \n> -GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when true, enables the\n> +GIT_TEST_ADD_I_USE_BUILTIN=<boolean>, when false, disables the\n>  built-in version of git add -i. See 'add.interactive.useBuiltin' in\n>  git-config(1).\n>  \n> diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh\n> index 71c5a15be00..bc3f69b4b1d 100755\n> --- a/t/t2016-checkout-patch.sh\n> +++ b/t/t2016-checkout-patch.sh\n> @@ -4,7 +4,7 @@ test_description='git checkout --patch'\n>  \n>  . ./lib-patch-mode.sh\n>  \n> -if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN false && ! test_have_prereq PERL\n> +if ! test_bool_env GIT_TEST_ADD_I_USE_BUILTIN true && ! test_have_prereq PERL\n>  then\n>  \tskip_all='skipping interactive add tests, PERL not set'\n>  \ttest_done\n\nLooks good.\n\nThanks.\n"},{"id":"442823","messageId":"xmqqa6hkyml0.fsf@gitster.g","threadId":"57002","inReplyTo":"211201.86pmqgbful.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-01T22:41:47Z","receivedAt":"2021-12-01T22:41:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n>>  add.interactive.useBuiltin::\n>> -\t[EXPERIMENTAL] Set to `true` to use the experimental built-in\n>> -\timplementation of the interactive version of linkgit:git-add[1]\n>> -\tinstead of the Perl script version. Is `false` by default.\n>> +\tSet to `false` to fall back to the original Perl implementation of\n>> +\tthe interactive version of linkgit:git-add[1] instead of the built-in\n>> +\tversion. Is `true` by default.\n>\n> I think this would be a bit better if we just stole the version you\n> added for stash.useBuiltin entirely. I.e. from your 336ad8424cb (stash:\n> document stash.useBuiltin, 2019-05-14), with the relevant s/shell\n> script/Perl/g etc. replaced.\n>\n> I.e. that version encouraged users to report any bugs, because we were\n> really going to remove it soon, as we then did for rebase.useBuiltin in\n> 9bcde4d5314 (rebase: remove transitory rebase.useBuiltin setting & env,\n> 2021-03-23).\n>\n> The wording in the opening paragraph is also a bit more to the point\n> there, i.e. calling it \"legacy\" rather than \"original [...]\n> implementation\".\n\nAfter reading the description on stash.useBuiltin, I'd agree with\nyou, except I do not see the distinction between \"original\" vs\n\"legacy\".  I very much like the way the last paragraph for\n\"stash.useBuiltin\" delivers the right message.\n\n    +If you find some reason to set this option to `false`, other than\n    +one-off testing, you should report the behavior difference as a bug in\n    +Git (see https://git-scm.com/community for details).\n\n\n>> -\tif (use_builtin_add_i == 1) {\n>> +\tif (use_builtin_add_i != 0) {\n>\n> Style/idiom: This should just be \"if (use_builtin_add_i)\".\n>\n> I.e. before we cared about not catching -1 here, but now that it's true\n> by default we don't care about the distinction between -1 or 1 anymore,\n> we just want it not to be 0 here.\n\nYes, if we are redoing this step anyway, we should lose the explicit\ncomparison with zero here.\n\n"},{"id":"442871","messageId":"nycvar.QRO.7.76.6.2112021555020.63@tvgsbejvaqbjf.bet","threadId":"57002","inReplyTo":"xmqqlf14ynco.fsf@gitster.g","subject":"Re: [PATCH 1/2] t2016: require the PERL prereq only when necessary","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-12-02T14:57:39Z","receivedAt":"2021-12-02T14:57:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 1 Dec 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > From: Johannes Schindelin <johannes.schindelin@gmx.de>\n> >\n> > The scripted version of the interactive mode of `git add` still requires\n> > Perl, but the built-in version does not. Let's only require the PERL\n> > prereq if testing the scripted version.\n> >\n> > This addresses a long-standing NEEDSWORK added in 35166b1fb54 (t2016:\n> > add a NEEDSWORK about the PERL prerequisite, 2020-10-07).\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >  t/t2016-checkout-patch.sh | 42 ++++++++++++++++++++-------------------\n> >  1 file changed, 22 insertions(+), 20 deletions(-)\n>\n> Good.  I suspect there may not be too many developers who lack PERL\n> prerequisite around here, so it is not like the built-in version was\n> not sufficiently tested in developers' environment without this\n> patch, but it is nice to see us move in this direction.  Of course,\n> when we remove the non-builtin version, we'd further be able to lose\n> these prerequisites.\n\nIt is probably true that few developers miss the Perl prerequisite, in\nparticular given that you cannot even run Git's tests without having a\nworking Perl.\n\nHowever, we have seen time and time again that developers do not really\nrun the test suite. For some, it takes way too long time (for some, 10\nminutes is too long, but I remember that Jeff Hostetler reported a 5h\nrunning time, and let's not forget about Cygwin and NonStop) and others\nsimply won't bother. So my focus lies more with the CI/PR builds. And\nthere, Windows specifically runs with NO_PERL. And you guessed it, to save\ntime.\n\nCiao,\nDscho\n"},{"id":"442872","messageId":"nycvar.QRO.7.76.6.2112021557530.63@tvgsbejvaqbjf.bet","threadId":"57002","inReplyTo":"a936a701-c13f-346e-69ee-85441ca20641@gmail.com","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-12-02T15:01:53Z","receivedAt":"2021-12-02T15:02:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Wed, 1 Dec 2021, Phillip Wood wrote:\n\n> On 30/11/2021 14:14, Johannes Schindelin via GitGitGadget wrote:\n>\n> > diff --git a/builtin/add.c b/builtin/add.c\n> > index ef6b619c45e..8ef230a345b 100644\n> > --- a/builtin/add.c\n> > +++ b/builtin/add.c\n> > @@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const\n> > char *patch_mode,\n> >    int use_builtin_add_i =\n> >     git_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n> >   -\tif (use_builtin_add_i < 0) {\n> > -\t\tint experimental;\n> > -\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n> > -\t\t\t\t\t &use_builtin_add_i))\n> > -\t\t\t; /* ok */\n> > -\t\telse if (!git_config_get_bool(\"feature.experimental\",\n> > &experimental) &&\n> > -\t\t\t experimental)\n> > -\t\t\tuse_builtin_add_i = 1;\n> > -\t}\n> > +\tif (use_builtin_add_i < 0 &&\n> > +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n> > +\t\t\t\t&use_builtin_add_i))\n> > +\t\tuse_builtin_add_i = 1;\n> >   -\tif (use_builtin_add_i == 1) {\n> > +\tif (use_builtin_add_i != 0) {\n>\n> This could be simplified to \"if (use_builtin_add_i)\" but don't re-roll just\n> for that\n\nI was actually considering this, given that Git's coding practice suggests\nprecisely the form you suggested.\n\nHowever, in this instance I found that form misleading: it would read to\nme as if `use_builtin_add_i` was a Boolean. But it is a tristate, it can\nalso be `-1` (\"undecided\"). And I wanted to express \"if this is not set to\n`false` specifically\", therefore I ended up with my proposal.\n\nCiao,\nDscho\n"},{"id":"442881","messageId":"xmqq5ys7vt8t.fsf@gitster.g","threadId":"57002","inReplyTo":"nycvar.QRO.7.76.6.2112021557530.63@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-02T16:58:26Z","receivedAt":"2021-12-02T16:58:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n>> > +\tif (use_builtin_add_i < 0 &&\n>> > +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n>> > +\t\t\t\t&use_builtin_add_i))\n>> > +\t\tuse_builtin_add_i = 1;\n>> >   -\tif (use_builtin_add_i == 1) {\n>> > +\tif (use_builtin_add_i != 0) {\n>>\n>> This could be simplified to \"if (use_builtin_add_i)\" but don't re-roll just\n>> for that\n>\n> I was actually considering this, given that Git's coding practice suggests\n> precisely the form you suggested.\n>\n> However, in this instance I found that form misleading: it would read to\n> me as if `use_builtin_add_i` was a Boolean. But it is a tristate, it can\n> also be `-1` (\"undecided\"). And I wanted to express \"if this is not set to\n> `false` specifically\", therefore I ended up with my proposal.\n\nI do not think that line of logic is sensible.  The variable starts\nits life as a tristate (i.e. not just bool but can be unknown), and\nthe four new lines above the conditional the patch adds is exactly\nabout getting rid of the unknown-ness and turning it into a known\nboolean.  After that happens, the variable can safely be used as a\nboolean.  In fact, I view the four lines before it is exactly to\nallow us to do so.\n\nWriting \"if not zero\" implies that the variable can have a non-zero\nvalue that is still \"unknown\" at this point in the code that has to\nbe defaulted to \"true\", which would mean that the \"if unset, read\nthe config, and if that fails, default to true\" logic above is not\ndoing its job.  That is a false impression that misleads readers of\nthe code.\n\nSo, I would say this conditional just should treat the variable as a\nsimple boolean.\n\n"},{"id":"442885","messageId":"nycvar.QRO.7.76.6.2112021635330.63@tvgsbejvaqbjf.bet","threadId":"57002","inReplyTo":"xmqqee6wymx2.fsf@gitster.g","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-12-02T17:30:58Z","receivedAt":"2021-12-02T17:31:13Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 1 Dec 2021, Junio C Hamano wrote:\n\n> \"Johannes Schindelin via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> [...]\n> > diff --git a/builtin/add.c b/builtin/add.c\n> > index ef6b619c45e..8ef230a345b 100644\n> > --- a/builtin/add.c\n> > +++ b/builtin/add.c\n> > @@ -237,17 +237,12 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n> >  \tint use_builtin_add_i =\n> >  \t\tgit_env_bool(\"GIT_TEST_ADD_I_USE_BUILTIN\", -1);\n> >\n> > -\tif (use_builtin_add_i < 0) {\n> > -\t\tint experimental;\n> > -\t\tif (!git_config_get_bool(\"add.interactive.usebuiltin\",\n> > -\t\t\t\t\t &use_builtin_add_i))\n> > -\t\t\t; /* ok */\n> > -\t\telse if (!git_config_get_bool(\"feature.experimental\", &experimental) &&\n> > -\t\t\t experimental)\n> > -\t\t\tuse_builtin_add_i = 1;\n> > -\t}\n> > +\tif (use_builtin_add_i < 0 &&\n> > +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n> > +\t\t\t\t&use_builtin_add_i))\n> > +\t\tuse_builtin_add_i = 1;\n> >\n> > -\tif (use_builtin_add_i == 1) {\n> > +\tif (use_builtin_add_i != 0) {\n>\n> Nit.\n>\n> \tif (use_builtin_add_i) {\n>\n> I wondered if these random calls to git_config_get_X() should be\n> consolidated into the existing add_config() callback, but this\n> conditional will go away hopefully in a few releases, so it probably\n> is not worth it.  Inheriting the way the original code grabbed the\n> configuration variables is good enough for our purpose here.\n\nAs I said in my reply to Phillip, I found it misleading to skip the `!= 0`\nbecause we are catching both the `== 1` as well as the `== -1` here.\n\nCiao,\nDscho\n"},{"id":"442886","messageId":"nycvar.QRO.7.76.6.2112021832060.63@tvgsbejvaqbjf.bet","threadId":"57002","inReplyTo":"CAPUEsphxCcfOgPQP9ci6sSn87g5=w+8kjh21=QywTX=on5vTdw@mail.gmail.com","subject":"Re: [PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-12-02T17:33:24Z","receivedAt":"2021-12-02T17:33:30Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Wed, 1 Dec 2021, Carlo Arenas wrote:\n\n> On Wed, Dec 1, 2021 at 12:40 AM Johannes Schindelin via GitGitGadget\n> <gitgitgadget@gmail.com> wrote:\n> > It is time to declare this implementation robust, to use it by default, and\n> > to start deprecating the scripted implementation.\n> >\n> > Johannes Schindelin (2):\n> >   t2016: require the PERL prereq only when necessary\n> >   add -i: default to the built-in implementation\n>\n> Sadly this implementation has a few bugs that still need fixing, with\n> at least one IMHO being a showstopper.\n>\n> The way macOS implements stdin (through a device) it will always\n> timeout in poll(), so escape keys that are left in the unread buffer\n> and that could match some of the entries will result in the wrong\n> entry being selected.\n>\n> I have a series[1] that reimplements this and that seemed to work fine\n> in my tests while making the code simpler, but that I didn't\n> prioritize (and wanted to clean up further) since I wanted to\n> prioritize the EDITOR fixes in the same area.\n>\n> Carlo\n>\n> [1] https://github.com/git/git/pull/1150\n\nThank you for pointing that out. I agree both with prioritizing your macOS\npatches, and with prioritizing the editor patches before that. Please just\nlet me know when would be a good time to move forward with this here patch\nseries.\n\nThank you,\nDscho\n"},{"id":"442896","messageId":"713b6ced-cbdf-9d5d-a722-30e525a3f931@ramsayjones.plus.com","threadId":"57002","inReplyTo":"xmqq5ys7vt8t.fsf@gitster.g","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2021-12-02T17:43:39Z","receivedAt":"2021-12-02T17:43:44Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 02/12/2021 16:58, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n>>>> +\tif (use_builtin_add_i < 0 &&\n>>>> +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n>>>> +\t\t\t\t&use_builtin_add_i))\n>>>> +\t\tuse_builtin_add_i = 1;\n>>>>   -\tif (use_builtin_add_i == 1) {\n>>>> +\tif (use_builtin_add_i != 0) {\n>>>\n>>> This could be simplified to \"if (use_builtin_add_i)\" but don't re-roll just\n>>> for that\n>>\n>> I was actually considering this, given that Git's coding practice suggests\n>> precisely the form you suggested.\n>>\n>> However, in this instance I found that form misleading: it would read to\n>> me as if `use_builtin_add_i` was a Boolean. But it is a tristate, it can\n>> also be `-1` (\"undecided\"). And I wanted to express \"if this is not set to\n>> `false` specifically\", therefore I ended up with my proposal.\n> \n> I do not think that line of logic is sensible.  The variable starts\n> its life as a tristate (i.e. not just bool but can be unknown), and\n> the four new lines above the conditional the patch adds is exactly\n> about getting rid of the unknown-ness and turning it into a known\n> boolean.  After that happens, the variable can safely be used as a\n> boolean.  In fact, I view the four lines before it is exactly to\n> allow us to do so.\n> \n> Writing \"if not zero\" implies that the variable can have a non-zero\n> value that is still \"unknown\" at this point in the code that has to\n> be defaulted to \"true\", which would mean that the \"if unset, read\n> the config, and if that fails, default to true\" logic above is not\n> doing its job.  That is a false impression that misleads readers of\n> the code.\n> \n> So, I would say this conditional just should treat the variable as a\n> simple boolean.\n> \n\nJust an FYI - I had t3701-add-interactive.sh show:\n\n  # 2 known breakage(s) vanished; please update test(s)\n\non Linux tonight (tests #45 and #47).\n\nI assumed, with little (well, any) thought, that these vanishing\nbreakages are due to this 'js/use-builtin-add-i' branch.\n\nJust ignore me (and apologies in advance), if this is not the case! ;-)\n\nATB,\nRamsay Jones\n\n\n"},{"id":"442995","messageId":"2ffbab1a-d5b2-fee5-806b-4e81eb759e11@gmail.com","threadId":"57002","inReplyTo":"pull.1087.git.1638281655.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Philippe Blain","fromEmail":"levraiphilippeblain@gmail.com","sentAt":"2021-12-03T13:58:36Z","receivedAt":"2021-12-03T13:58:41Z","isPatch":true,"sender":{"key":"levraiphilippeblain@gmail.com","avatar":"https://avatars.githubusercontent.com/u/44212482?v=4"},"body":"Hi Dscho,\n\nLe 2021-11-30 à 09:14, Johannes Schindelin via GitGitGadget a écrit :\n> Over two years ago, Slavica Đukić participated in the Outreachy project,\n> starting to implement a built-in version of the interactive git add command.\n> A little over a year ago, Git turned on that mode whenever users were\n> running with feature.experimental = true.\n> \n> It is time to declare this implementation robust, to use it by default, and\n> to start deprecating the scripted implementation.\n> \n> Johannes Schindelin (2):\n>    t2016: require the PERL prereq only when necessary\n>    add -i: default to the built-in implementation\n> \n>   Documentation/config/add.txt |  6 +++---\n>   builtin/add.c                | 15 +++++--------\n>   ci/run-build-and-tests.sh    |  2 +-\n>   t/README                     |  2 +-\n>   t/t2016-checkout-patch.sh    | 42 +++++++++++++++++++-----------------\n>   5 files changed, 32 insertions(+), 35 deletions(-)\n\nI just noticed that 'INSTALL' mentions that Perl is needed for 'git add interactive'\net al, so maybe we would want to tweak the wording a bit in there when switch the default\nto the C version ?\n\nCheers,\nPhilippe.\n"},{"id":"443155","messageId":"nycvar.QRO.7.76.6.2112061659010.27797@tvgsbejvaqbjf.bet","threadId":"57002","inReplyTo":"2ffbab1a-d5b2-fee5-806b-4e81eb759e11@gmail.com","subject":"Re: [PATCH 0/2] Use the built-in implementation of the interactive add command by default","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-12-06T15:59:26Z","receivedAt":"2021-12-06T16:02:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Philippe,\n\nOn Fri, 3 Dec 2021, Philippe Blain wrote:\n\n> Le 2021-11-30 à 09:14, Johannes Schindelin via GitGitGadget a écrit :\n> > Over two years ago, Slavica Đukić participated in the Outreachy project,\n> > starting to implement a built-in version of the interactive git add command.\n> > A little over a year ago, Git turned on that mode whenever users were\n> > running with feature.experimental = true.\n> >\n> > It is time to declare this implementation robust, to use it by default, and\n> > to start deprecating the scripted implementation.\n> >\n> > Johannes Schindelin (2):\n> >    t2016: require the PERL prereq only when necessary\n> >    add -i: default to the built-in implementation\n> >\n> >   Documentation/config/add.txt |  6 +++---\n> >   builtin/add.c                | 15 +++++--------\n> >   ci/run-build-and-tests.sh    |  2 +-\n> >   t/README                     |  2 +-\n> >   t/t2016-checkout-patch.sh    | 42 +++++++++++++++++++-----------------\n> >   5 files changed, 32 insertions(+), 35 deletions(-)\n>\n> I just noticed that 'INSTALL' mentions that Perl is needed for 'git add\n> interactive'\n> et al, so maybe we would want to tweak the wording a bit in there when switch\n> the default\n> to the C version ?\n\nNot yet. Only once we remove `git-add--interactive.perl`.\n\nThanks,\nDscho\n"},{"id":"443563","messageId":"xmqqwnketo0a.fsf@gitster.g","threadId":"57002","inReplyTo":"84824918ae4564a9194a1a55124ee8694f210437.1638281655.git.gitgitgadget@gmail.com","subject":"[PATCH] fixup! add -i: default to the built-in implementation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-09T04:12:37Z","receivedAt":"2021-12-09T04:12:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With the reimplementated \"git add -i\", two test pieces that used to\nexpect failure now succeed.  Mark them as such.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * Yes, I know this is on hold until some issues in the \"add -i\"\n   reimplementation on MacOS are resolved, but as I am getting tired\n   of seeing \"TODO PASSED\" when I rebuild and test 'seen', I'll\n   queue this band-aid at the tip of this topic.\n\n t/t3701-add-interactive.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 207714655f..1effc3f419 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -500,7 +500,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n \t! grep \"^+15\" actual\n '\n \n-test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n+test_expect_success 'split hunk \"add -p (no, yes, edit)\"' '\n \ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n \tgit reset &&\n \t# test sequence is s(plit), n(o), y(es), e(dit)\n@@ -524,7 +524,7 @@ test_expect_success 'split hunk with incomplete line at end' '\n \ttest_must_fail git grep --cached before\n '\n \n-test_expect_failure 'edit, adding lines to the first hunk' '\n+test_expect_success 'edit, adding lines to the first hunk' '\n \ttest_write_lines 10 11 20 30 40 50 51 60 >test &&\n \tgit reset &&\n \ttr _ \" \" >patch <<-EOF &&\n-- \n2.34.1-373-gb739cc97c5\n\n"},{"id":"443846","messageId":"nycvar.QRO.7.76.6.2112102352350.90@tvgsbejvaqbjf.bet","threadId":"57002","inReplyTo":"xmqqwnketo0a.fsf@gitster.g","subject":"Re: [PATCH] fixup! add -i: default to the built-in implementation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-12-10T22:55:10Z","receivedAt":"2021-12-10T22:55:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 8 Dec 2021, Junio C Hamano wrote:\n\n> With the reimplementated \"git add -i\", two test pieces that used to\n> expect failure now succeed.  Mark them as such.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n> ---\n>\n>  * Yes, I know this is on hold until some issues in the \"add -i\"\n>    reimplementation on MacOS are resolved, but as I am getting tired\n>    of seeing \"TODO PASSED\" when I rebuild and test 'seen', I'll\n>    queue this band-aid at the tip of this topic.\n\nAs long as we ship the Perl version as an escape hatch, and as long as\nthat version does not pass those two test cases, your patch is premature.\n\nI would expect the `linux-gcc` job (or whatever it is called for the next\nfew hours) to fail with this fixup!, as a consequence of running t3701\nwith the scripted version of `add -i`.\n\nCiao,\nDscho\n\n>\n>  t/t3701-add-interactive.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index 207714655f..1effc3f419 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -500,7 +500,7 @@ test_expect_success 'split hunk \"add -p (edit)\"' '\n>  \t! grep \"^+15\" actual\n>  '\n>\n> -test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n> +test_expect_success 'split hunk \"add -p (no, yes, edit)\"' '\n>  \ttest_write_lines 5 10 20 21 30 31 40 50 60 >test &&\n>  \tgit reset &&\n>  \t# test sequence is s(plit), n(o), y(es), e(dit)\n> @@ -524,7 +524,7 @@ test_expect_success 'split hunk with incomplete line at end' '\n>  \ttest_must_fail git grep --cached before\n>  '\n>\n> -test_expect_failure 'edit, adding lines to the first hunk' '\n> +test_expect_success 'edit, adding lines to the first hunk' '\n>  \ttest_write_lines 10 11 20 30 40 50 51 60 >test &&\n>  \tgit reset &&\n>  \ttr _ \" \" >patch <<-EOF &&\n> --\n> 2.34.1-373-gb739cc97c5\n>\n>\n"},{"id":"443847","messageId":"nycvar.QRO.7.76.6.2112102356150.90@tvgsbejvaqbjf.bet","threadId":"57002","inReplyTo":"xmqq5ys7vt8t.fsf@gitster.g","subject":"Re: [PATCH 2/2] add -i: default to the built-in implementation","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-12-10T22:59:11Z","receivedAt":"2021-12-10T22:59:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Thu, 2 Dec 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n>\n> >> > +\tif (use_builtin_add_i < 0 &&\n> >> > +\t    git_config_get_bool(\"add.interactive.usebuiltin\",\n> >> > +\t\t\t\t&use_builtin_add_i))\n> >> > +\t\tuse_builtin_add_i = 1;\n> >> >   -\tif (use_builtin_add_i == 1) {\n> >> > +\tif (use_builtin_add_i != 0) {\n> >>\n> >> This could be simplified to \"if (use_builtin_add_i)\" but don't re-roll just\n> >> for that\n> >\n> > I was actually considering this, given that Git's coding practice suggests\n> > precisely the form you suggested.\n> >\n> > However, in this instance I found that form misleading: it would read to\n> > me as if `use_builtin_add_i` was a Boolean. But it is a tristate, it can\n> > also be `-1` (\"undecided\"). And I wanted to express \"if this is not set to\n> > `false` specifically\", therefore I ended up with my proposal.\n>\n> I do not think that line of logic is sensible.  The variable starts\n> its life as a tristate (i.e. not just bool but can be unknown), and\n> the four new lines above the conditional the patch adds is exactly\n> about getting rid of the unknown-ness and turning it into a known\n> boolean.  After that happens, the variable can safely be used as a\n> boolean.  In fact, I view the four lines before it is exactly to\n> allow us to do so.\n>\n> Writing \"if not zero\" implies that the variable can have a non-zero\n> value that is still \"unknown\" at this point in the code that has to\n> be defaulted to \"true\", which would mean that the \"if unset, read\n> the config, and if that fails, default to true\" logic above is not\n> doing its job.  That is a false impression that misleads readers of\n> the code.\n>\n> So, I would say this conditional just should treat the variable as a\n> simple boolean.\n\nThat forces the reader to perform those mental gymnastics to follow the\nreasoning that the tristate now essentially became a Boolean.\n\nI wanted to avoid that unnecessary cognitive load.\n\nCiao,\nDscho\n"}]}