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

Ævar Arnfjörð Bjarmason [off-list ref] writes:

quoted

 add.interactive.useBuiltin::
-	[EXPERIMENTAL] Set to `true` to use the experimental built-in
-	implementation of the interactive version of linkgit:git-add[1]
-	instead of the Perl script version. Is `false` by default.
+	Set to `false` to fall back to the original Perl implementation of
+	the interactive version of linkgit:git-add[1] instead of the built-in
+	version. Is `true` by default.

I think this would be a bit better if we just stole the version you
added for stash.useBuiltin entirely. I.e. from your 336ad8424cb (stash:
document stash.useBuiltin, 2019-05-14), with the relevant s/shell
script/Perl/g etc. replaced.

I.e. that version encouraged users to report any bugs, because we were
really going to remove it soon, as we then did for rebase.useBuiltin in
9bcde4d5314 (rebase: remove transitory rebase.useBuiltin setting & env,
2021-03-23).

The wording in the opening paragraph is also a bit more to the point
there, i.e. calling it "legacy" rather than "original [...]
implementation".

After reading the description on stash.useBuiltin, I'd agree with
you, except I do not see the distinction between "original" vs
"legacy".  I very much like the way the last paragraph for
"stash.useBuiltin" delivers the right message.

    +If you find some reason to set this option to `false`, other than
    +one-off testing, you should report the behavior difference as a bug in
    +Git (see https://git-scm.com/community for details).

quoted

-	if (use_builtin_add_i == 1) {
+	if (use_builtin_add_i != 0) {

Style/idiom: This should just be "if (use_builtin_add_i)".

I.e. before we cared about not catching -1 here, but now that it's true
by default we don't care about the distinction between -1 or 1 anymore,
we just want it not to be 0 here.

Yes, if we are redoing this step anyway, we should lose the explicit
comparison with zero here.