Re: [PATCH v3 0/3] Convert git-bisect--helper to OPT_SUBCOMMAND

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Thu, Nov 10 2022, Đoàn Trần Công Danh wrote:

> Despite this series has been merged into next.
> I'm sending this because Taylor said he wanted a (combined) reroll?
> <Y2riRSL+NprJt278@nand.local>

Since Taylor hasn't chimed in: I think that "combined reroll" was
referring to whether we could combine the version I sent out, and the
one you sent out.

I think we should (obviously) go for your version, and thanks for
picking this up and running with it. But at the time in that "What's
Cooking" the branch name was "dd/git-bisect-builtin" (i.e. your
version), but the note was still saying the source was
"<cover-00.13-00000000000-20221104T132117Z-avarab@gmail.com>" (i.e. my
version).

In any case, the patches you're re-rolling in this 1-3 are already in
next, so a "what it would have looked like" isn't going to be useful to
apply at this point. So I think just...

> Range-diff against v2:
> -:  ---------- > 1:  6b80fd9398 bisect--helper: remove unused options
> 1:  9d8a3cdd7a ! 2:  248ed392ea bisect--helper: move all subcommands into their own functions
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__terms(int argc, const char **argv, const char *prefix UNUSED)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	if (argc > 1)
>      +		return error(_("--bisect-terms requires 0 or 1 argument"));
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__start(int argc, const char **argv, const char *prefix UNUSED)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	set_terms(&terms, "bad", "good");
>      +	res = bisect_start(&terms, argv, argc);
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__next(int argc, const char **argv UNUSED, const char *prefix)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	if (argc)
>      +		return error(_("--bisect-next requires 0 arguments"));
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__state(int argc, const char **argv, const char *prefix UNUSED)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	set_terms(&terms, "bad", "good");
>      +	get_terms(&terms);
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__replay(int argc, const char **argv, const char *prefix UNUSED)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	if (argc != 1)
>      +		return error(_("no logfile given"));
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__skip(int argc, const char **argv, const char *prefix UNUSED)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	set_terms(&terms, "bad", "good");
>      +	get_terms(&terms);
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__visualize(int argc, const char **argv, const char *prefix UNUSED)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	get_terms(&terms);
>      +	res = bisect_visualize(&terms, argv, argc);
>     @@ builtin/bisect--helper.c: static int bisect_run(struct bisect_terms *terms, cons
>      +static int cmd_bisect__run(int argc, const char **argv, const char *prefix UNUSED)
>      +{
>      +	int res;
>     -+	struct bisect_terms terms = { .term_good = NULL, .term_bad = NULL };
>     ++	struct bisect_terms terms = { 0 };
>      +
>      +	if (!argc)
>      +		return error(_("bisect run failed: no command provided."));

...a stand-alone two-patch series with these (and possibly tying any
other loose ends) would be nice, I think (but haven't looked carefully)
that it could probably be done in such a way that it applies
concurrently with the migration to builtin/bisect.c, i.e. to the extent
that the hunks don't conflict textually or semantically.