On Wed, Sep 08 2021, Eric Sunshine wrote: > On Wed, Sep 8, 2021 at 11:24 AM Ævar Arnfjörð Bjarmason > <avarab@xxxxxxxxx> wrote: >> 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@xxxxxxxxx> >> --- >> 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.) I'll make it emit something more helpful. More generally I've got quite a bit of parse_options() improvements queued up locally that I've been trying to trickle in at the rate I can get them through the list, review over at [1] would be much appreciated. I wonder if we can do this automatically, we already have the builtin_help_usage, we could parse that in the general case and find that certain options are mutually exclusive per the examples there. We'd then discover what option we parsed when usage_with_options() was called, and automatically emit a useful message in these sorts of cases. Of course the usage strings might be incomplete or wrong, but part of the point would be to find those cases & a test mode to die() if a command was called with some option combinations not suggested as working according to its documented usage... https://lore.kernel.org/git/cover-0.2-00000000000-20210901T110917Z-avarab@xxxxxxxxx/
