Re: [PATCH v16 6/6] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C

From: Christian Couder <christian.couder@gmail.com>
Date: Nov 20, 2017, 07:03 UTC
Message-ID: <CAP8UFD30RWeXbRDhrxzcrT9_88WhBt-prcNMxC=XwPd7+4UgBA@mail.gmail.com>
In-Reply-To: <xmqqlgj1myyj.fsf@gitster.mtv.corp.google.com>

On Mon, Nov 20, 2017 at 4:05 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 11 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> On Fri, Sep 29, 2017 at 8:49 AM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
>>> Reimplement `is_expected_rev` & `check_expected_revs` shell function in
>>> C and add a `--check-expected-revs` subcommand to `git bisect--helper` to
>>> call it from git-bisect.sh .
>>
>> It looks like this patch forgot to add the `--check-expected-revs`
>> subcommand to the usage string.
>
> Usage string of what?  "git bisect"?

bisect--helper

Show 5 quoted lines

> I do not think bisect--helper even needs to have end-user facing
> "usage string", but "git bisect--helper -h" still gets the new thing
> listed in its output for free thanks to parse-options API, so that
> is not what you found "this patch forgot to add" and you are making
> me scratch my head.

Right now in master the usage string for bisect--helper is:

static const char * const git_bisect_helper_usage[] = {
    N_("git bisect--helper --next-all [--no-checkout]"),
    N_("git bisect--helper --write-terms <bad_term> <good_term>"),
    N_("git bisect--helper --bisect-clean-state"),
    NULL
};

So it lists 3 of the command modes out of 4. The missing one is for --check-expected-revs. For consistency I think it would be nice to add the missing one.