Re: [PATCH 5/6] help: correct logic error in combining --all and --config

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Sep 8, 2021, 16:39 UTC
Message-ID: <CAPig+cSK+wLPUDuGf1d41J_F5jQS+J=a=7kHQLV07-ZKZW9GsA@mail.gmail.com>
In-Reply-To: <patch-5.6-bcc640d32a1-20210908T151949Z-avarab@gmail.com>

On Wed, Sep 8, 2021 at 11:24 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 21 quoted lines

> Fix a bug in the --config option that's been there ever since its
> introduction in 3ac68a93fd2 (help: add --config to list all available
> config, 2018-05-26). Die when --all and --config are combined,
> combining them doesn't make sense.
>
> The code for the --config option when combined with an earlier
> refactoring done to support the --guide option in
> 65f98358c0c (builtin/help.c: add --guide option, 2013-04-02) would
> cause us to take the "--all" branch early and ignore the --config
> option.
>
> Let's instead list these as incompatible, both in the synopsis and
> help output, and enforce it in the code itself.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/builtin/help.c b/builtin/help.c
> @@ -549,18 +550,26 @@ int cmd_help(int argc, const char **argv, const char *prefix)
> +       /* Incompatible options */
> +       if (show_all + !!show_config + show_guides > 1)
> +               usage_with_options(builtin_help_usage, builtin_help_options);

I personally find it highly frustrating when a program merely dumps the usage statement without any explanation of what exactly it doesn't like about the command-line. Printing out a simple:

    --all, --guides, --config are mutually exclusive

message would go a long way toward reducing the frustration level.

(Aside: I also find it more hostile than helpful when programs dump the usage statement for a command-line invocation error -- even if preceded by an explanation of the error -- since the explanation usually gets drowned-out by the often multi-page usage text, and the user has to go spelunking around the wall of output to try to figure out what actually went wrong. It's much more helpful and easy to figure out what went wrong with the invocation when only a simple error message is printed -- without usage statement. However, that's a separate battle, as Git already has plenty of places which dump the usage statement in response to an invocation error.)