Re: [PATCH v6 05/16] bisect--helper: make `terms` an explicit singleton

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

From: Johannes Schindelin <redacted>

We tried very hard to keep code in `builtin/bisect--helper.c` in as
libifyable a state as possible.

However, we are about to migrate this built-in to the `OPT_SUBCOMMAND()`
API, which does not allow for passing any context (e.g. via a `void
*data` parameters as they are used in the config API).

Therefore, we _have_ to move the `terms` variable outside of
`cmd_bisect__helper()` and explicitly make it a singleton (as it
currently is, anyway).

Well, I do not find the above all that convincing.

I can see that the top-level functions that are called from
OPT_SUBCOMMAND(), because the only thing they get may be argc, argv,
and prefix, may need to access a global variable to keep track of
other states.  But all the helper functions that are called by them,
which are already "kept as libifyable as possible", can be called
with a pointer to that global variable by these subcommand functions
like cmd_bisect_{terms,start,...}().  As long as the libification
boundary is left relatively clean, I do not see the need to churn
this much code to change even the lowest level of helpers to access
the global.

I was actually imagining that these functions that (used to) take a
pointer to "struct bisect_terms" can be moved to the top-level
"bisect.c", together with the code already libified that drive the
"git bisect" program, as part of the "libified part of bisect
engine".

For example, at the tip of 'seen', free_term() are called by
get_terms() and cmd_bisect() and nothing else.  get_terms(), before
this patch deliberately made it less libifiable, used to take a
pointer to "struct bisect_terms", so it is just the matter of
passing what it got from its callers down to free_terms().

get_terms() in turn are called from many places, but as far as I can
see, all the callers are either

 - functions that are OPT_SUBCOMMAND() handler, which should take
   the pointer to the global &global_terms and pass it down to the
   callchain that are "kept libifiable" until this step, or

 - functions that used to take a pointer to "struct bisect_terms"
   before this step, and can just pass it down the callchain.

That is the impression after I tentatively modified the file at the
tip of 'seen' to

    static struct bisect_terms {
            char *term_good;
            char *term_bad;
    } global_terms;

and tried resurrect the "as libified as possible" state for some
call graphs.

Is there a function I missed that MUST access the global *and* also
access pointer given as a parameter?  If there is such a codepath,
that would indeed cause a confusion for future developers and leave
a maintenance burden because developers do not have a clear guidance
which one to use, but I do not think the current (i.e. the state
before this step) "as libifiable as possible" state is not that bad.

Thanks.