Re: [PATCH v1] checkout: optionally speed up "git checkout -b foo"

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jul 26, 2018, 18:59 UTC
Message-ID: <CAPig+cRtUeSCUnUFw0xgXEV7nyBiORZSLfLYfhKWjXxcgHvQmg@mail.gmail.com>
In-Reply-To: <xmqqeffqrqb3.fsf@gitster-ct.c.googlers.com>

On Thu, Jul 26, 2018 at 11:04 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

> Ben Peart <peartben@gmail.com> writes:
> > I'm not thrilled with the long list either (the plethora of comments
> > probably makes it appear worse than it is) but I don't see how...
>
> If there were a simple and futureproof way to tell the option
> parsing loop to notice any feature other than "-b newbranch" was
> used, then such a whitelisting may be a viable way, but there is
> not, and the whitelist condition can become (over time---we are
> talking about futureproofing and not "a filter that happens to match
> today's feature set") just as complex as this blacklisting function
> is (e.g. feature A and feature B when used alone may be compatible
> with the optimization but not when used both), so if we were to use
> this optimization, I think this long list of special-case checks is
> the best we could do.

I'm wondering if a two-stage parse-options invocations could make this potential maintenance problem more manageable by altogether eliminating needs_working_tree_merge(). Something very roughly along the lines of:

    new_branch_and_passive_options = {
        OPT_STRING('b', NULL, ...),
        ...options which can't impact "optimization" decision...
    };
    argc = parse_options(argc, argv, prefix,
        new_branch_and_passive_options, NULL,
        PARSE_OPT_KEEP_UNKNOWN | PARSE_OPT_KEEP_DASHDASH);

    can_optimize_new_branch = 1;
    for (i = 0; i < argc; i++)
        if (argv[i][0] == '-') {
            can_optimize_new_branch = 0;
            break;
        }

    options = {
        ...all other options...
    }
    argc = parse_options(argc, argv, prefix, options,
        checkout_usage, PARSE_OPT_KEEP_DASHDASH);

    ...as before...

The can_optimize_new_branch check could, of course, be fooled by a non-option which starts with a "-", but that would err toward safety of not optimizing, so shouldn't be a worry.