parse-options: allow -h as a short option
cover parse-options: allow -h as a short option
The short option -h is hard-wired in parseopt to list the options of a command together with a short explanation. If -h is to be given a different meaning then the flag PARSE_OPT_NO_INTERNAL_HELP has to be specified, which turns off handling for -h, --help and --help-all (except that --help handling is effectively overridden by git.c, but that's a different story).
Most commands that do that, grep and show-ref in particular, still show the usage when -h is specified as the only parameter and with --help-all by implementing explicit handlers for them. This series makes it easier for them by letting them override -h handling without any flag.
Rene Scharfe (5):
parse-options: deduplicate parse_options_usage() calls
parse-options: inline parse_options_usage() at its only remaining caller
parse-options: allow -h as a short option
grep: stop using PARSE_OPT_NO_INTERNAL_HELP
show-ref: stop using PARSE_OPT_NO_INTERNAL_HELP
builtin/grep.c | 17 +---------------- builtin/show-ref.c | 12 +----------- parse-options.c | 40 ++++++++++++++++++++-------------------- parse-options.h | 2 +- 4 files changed, 23 insertions(+), 48 deletions(-)
Signature · 1 line
2.6.3
PATCH 1/5 parse-options: deduplicate parse_options_usage() calls
Avoid long lines and repeating parse_options_usage() calls with their duplicate parameters by providing labels with speaking names to jump to.
--- parse-options.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
parse-options.c +12 -6
@@ -435,6 +435,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx, const char * const usagestr[]) {int internal_help = !(ctx->flags &PARSE_OPT_NO_INTERNAL_HELP ); +int err =0 ;/* we must reset ->opt, unknown short option leave it dangling */ ctx->opt =NULL ;
@@ -454,10 +455,10 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,if (arg[1 ] !='-' ) { ctx->opt = arg +1 ;if (internal_help && *ctx->opt =='h' ) -return parse_options_usage (ctx, usagestr, options,0 ); + goto show_usage;switch (parse_short_opt (ctx, options)) {case -1 : -return parse_options_usage (ctx, usagestr, options,1 ); + goto show_usage_error;case -2 :if (ctx->opt)check_typos (arg +1 , options);
@@ -467,10 +468,10 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,check_typos (arg +1 , options);while (ctx->opt) {if (internal_help && *ctx->opt =='h' ) -return parse_options_usage (ctx, usagestr, options,0 ); + goto show_usage;switch (parse_short_opt (ctx, options)) {case -1 : -return parse_options_usage (ctx, usagestr, options,1 ); + goto show_usage_error;case -2 :/* fake a short option thing to hide the fact that we may have * started to parse aggregated stuff
@@ -496,10 +497,10 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,if (internal_help && !strcmp (arg +2 ,"help-all" ))return usage_with_options_internal (ctx, usagestr, options,1 ,0 );if (internal_help && !strcmp (arg +2 ,"help" )) -return parse_options_usage (ctx, usagestr, options,0 ); + goto show_usage;switch (parse_long_opt (ctx, arg +2 , options)) {case -1 : -return parse_options_usage (ctx, usagestr, options,1 ); + goto show_usage_error;case -2 : goto unknown; }
@@ -511,6 +512,11 @@ unknown: ctx->opt =NULL ; }return PARSE_OPT_DONE ; + + show_usage_error: + err =1 ; + show_usage: +return parse_options_usage (ctx, usagestr, options, err); }int parse_options_end (struct parse_opt_ctx_t *ctx)
Signature · 1 line
2.6.3
PATCH 2/5 parse-options: inline parse_options_usage() at its only remaining caller
In reply to René Scharfe--- parse-options.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)
parse-options.c +1 -12
@@ -5,10 +5,6 @@#include "color.h" #include "utf8.h" -static int parse_options_usage (struct parse_opt_ctx_t *ctx, -const char *const *usagestr, -const struct option *opts,int err); -#define OPT_SHORT 1#define OPT_UNSET 2
@@ -516,7 +512,7 @@ unknown: show_usage_error: err =1 ; show_usage: -return parse_options_usage (ctx, usagestr, options, err); +return usage_with_options_internal (ctx, usagestr, options,0 , err); }int parse_options_end (struct parse_opt_ctx_t *ctx)
@@ -662,13 +658,6 @@ void NORETURN usage_msg_opt(const char *msg,usage_with_options (usagestr, options); } -static int parse_options_usage (struct parse_opt_ctx_t *ctx, -const char *const *usagestr, -const struct option *opts,int err) -{ -return usage_with_options_internal (ctx, usagestr, opts,0 , err); -} -#undef opterrorint opterror (const struct option *opt,const char *reason,int flags) {
Signature · 1 line
2.6.3
PATCH 3/5 parse-options: allow -h as a short option
In reply to René ScharfeLet callers provide their own handler for the short option -h even without the flag PARSE_OPT_NO_INTERNAL_HELP, but call the internal handler (showing usage information) if that is the only parameter. Implement the first part by checking for -h only if parse_short_opt() can't find it and returns -2.
--- parse-options.c | 15 ++++++++++----- parse-options.h | 2 +- 2 files changed, 11 insertions(+), 6 deletions(-)
parse-options.c +10 -5
@@ -410,7 +410,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx, const struct option *options, int flags) { memset(ctx, 0, sizeof(*ctx)); - ctx->argc = argc -1 ; + ctx->argc = ctx->total = argc -1 ; ctx->argv = argv +1 ; ctx->out = argv; ctx->prefix = prefix;
@@ -448,27 +448,32 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,continue ; } +/* lone -h asks for help */ +if (internal_help && ctx->total ==1 && !strcmp (arg +1 ,"h" )) + goto show_usage; +if (arg[1 ] !='-' ) { ctx->opt = arg +1 ; -if (internal_help && *ctx->opt =='h' ) - goto show_usage;switch (parse_short_opt (ctx, options)) {case -1 : goto show_usage_error;case -2 :if (ctx->opt)check_typos (arg +1 , options); +if (internal_help && *ctx->opt =='h' ) + goto show_usage; goto unknown; }if (ctx->opt)check_typos (arg +1 , options);while (ctx->opt) { -if (internal_help && *ctx->opt =='h' ) - goto show_usage;switch (parse_short_opt (ctx, options)) {case -1 : goto show_usage_error;case -2 : +if (internal_help && *ctx->opt =='h' ) + goto show_usage; +/* fake a short option thing to hide the fact that we may have * started to parse aggregated stuff *
parse-options.h +1 -1
@@ -199,7 +199,7 @@ enum {struct parse_opt_ctx_t {const char **argv;const char **out; -int argc, cpidx; +int argc, cpidx, total;const char *opt;int flags;const char *prefix;
Signature · 1 line
2.6.3
PATCH 4/5 grep: stop using PARSE_OPT_NO_INTERNAL_HELP
In reply to René ScharfeThe flag PARSE_OPT_NO_INTERNAL_HELP is set to allow overriding the option -h, except when it's the only one given. This is the default behavior now, so remove the flag and the hand-rolled --help-all handling. The internal --help-all handler now actually shows hidden options, i.e. --debug in this case.
--- builtin/grep.c | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-)
builtin/grep.c +1 -16
@@ -612,11 +612,6 @@ static int pattern_callback(const struct option *opt, const char *arg,return 0 ; } -static int help_callback (const struct option *opt,const char *arg,int unset) -{ -return -1 ; -} -int cmd_grep (int argc,const char **argv,const char *prefix) {int hit =0 ;
@@ -738,18 +733,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix) PARSE_OPT_OPTARG, NULL, (intptr_t)default_pager }, OPT_BOOL(0, "ext-grep", &external_grep_allowed__ignored, N_("allow calling of grep(1) (ignored by this build)")), - {OPTION_CALLBACK ,0 ,"help-all" ,NULL ,NULL ,N_ ("show usage" ), - PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },OPT_END () }; -/* -* 'git grep -h', unlike 'git grep -h <pattern>', is a request -* to show usage information and exit. -*/ -if (argc ==2 && !strcmp (argv[1 ],"-h" )) -usage_with_options (grep_usage, options); -init_grep_defaults ();git_config (grep_cmd_config,NULL );grep_init (&opt, prefix);
@@ -766,8 +752,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)*/ argc =parse_options (argc, argv, prefix, options, grep_usage,PARSE_OPT_KEEP_DASHDASH | -PARSE_OPT_STOP_AT_NON_OPTION | -PARSE_OPT_NO_INTERNAL_HELP ); +PARSE_OPT_STOP_AT_NON_OPTION );grep_commit_pattern_type (pattern_type_arg, &opt);if (use_index && !startup_info->have_repository)
Signature · 1 line
2.6.3
PATCH 5/5 show-ref: stop using PARSE_OPT_NO_INTERNAL_HELP
In reply to René ScharfeThe flag PARSE_OPT_NO_INTERNAL_HELP is set to allow overriding the option -h, except when it's the only one given. This is the default behavior now, so remove the flag and the hand-rolled --help-all handling. The internal --help-all handler now actually shows hidden options, i.e. -h in this case.
--- builtin/show-ref.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-)
builtin/show-ref.c +1 -11
@@ -161,11 +161,6 @@ static int exclude_existing_callback(const struct option *opt, const char *arg,return 0 ; } -static int help_callback (const struct option *opt,const char *arg,int unset) -{ -return -1 ; -} -static const struct option show_ref_options[] = {OPT_BOOL (0 ,"tags" , &tags_only,N_ ("only show tags (can be combined with heads)" )),OPT_BOOL (0 ,"heads" , &heads_only,N_ ("only show heads (can be combined with tags)" )),
@@ -186,18 +181,13 @@ static const struct option show_ref_options[] = { {OPTION_CALLBACK ,0 ,"exclude-existing" , &exclude_existing_arg,N_ ("pattern" ),N_ ("show refs from stdin that aren't in local repository" ), PARSE_OPT_OPTARG | PARSE_OPT_NONEG, exclude_existing_callback }, - { OPTION_CALLBACK, 0, "help-all", NULL, NULL, N_("show usage"), - PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },OPT_END () };int cmd_show_ref (int argc,const char **argv,const char *prefix) { -if (argc ==2 && !strcmp (argv[1 ],"-h" )) -usage_with_options (show_ref_usage, show_ref_options); - argc =parse_options (argc, argv, prefix, show_ref_options, - show_ref_usage,PARSE_OPT_NO_INTERNAL_HELP ); + show_ref_usage,0 );if (exclude_arg)return exclude_existing (exclude_existing_arg);
Signature · 1 line
2.6.3
Re: [PATCH 5/5] show-ref: stop using PARSE_OPT_NO_INTERNAL_HELP
Quoted 3 lines · René Scharfe
The flag
PARSE_OPT_NO_INTERNAL_HELPis set to allow overriding the option -h, except when it's the only one given.
This is the default behavior now,...
OK, so in the old world order, "-h" used to trigger the internal help even before consulting parse_short_opt() to see if there is a handler supplied by the caller, and NO_INTERNAL_HELP was invented as a kludge to to disable this behaviour. The existing two users (dealt with 4/5 and 5/5) both used this mechanism to let their own handler kick in, but they had to make "-h" without anything else on the command line behave just like the internal one, while handling "-h" with something else on the command line do a custom thing.
In the new world order, internal "-h" handler is called only after seeing that parse_short_opt() decides there is no handler for "-h" as a fallback.
Side note. Not really. Among the three uses of intenal_help in parse_options_step(), the first one ("lone -h asks for help") is used before we ask parse_short_opt().
I wonder if we can/want to further tweak this in a follow-up series. If that is done, I think NO_INTERNAL_HELP can go away, as its only effect would be to make us say "unknown option" when "-h" alone was given from the command line for an options[] array that does not have a handler for "-h".