diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp index 91193e181..edf7d4210 100644 --- a/src/builtin_argparse.cpp +++ b/src/builtin_argparse.cpp @@ -173,12 +173,12 @@ static int parse_exclusive_args(argparse_cmd_opts_t &opts, io_streams_t &streams static bool parse_flag_modifiers(argparse_cmd_opts_t &opts, option_spec_t *opt_spec, const wcstring &option_spec, const wchar_t *s, io_streams_t &streams) { - if (*s == '=') { + if (*s == L'=') { s++; - if (*s == '?') { + if (*s == L'?') { opt_spec->num_allowed = -1; // optional arg s++; - } else if (*s == '+') { + } else if (*s == L'+') { opt_spec->num_allowed = 2; // mandatory arg and can appear more than once s++; } else { @@ -208,9 +208,9 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, const wchar_t *s = option_spec.c_str(); option_spec_t *opt_spec = new option_spec_t(*s++); - if (*s == '/') { + if (*s == L'/') { s++; // the struct is initialized assuming short_flag_valid should be true - } else if (*s == '-') { + } else if (*s == L'-') { opt_spec->short_flag_valid = false; s++; } else { @@ -219,16 +219,17 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, return parse_flag_modifiers(opts, opt_spec, option_spec, s, streams); } + // Collect the long flag name. const wchar_t *e = s; - while (*e && *e != '+' && *e != '=') e++; + while (*e && (*e == L'-' || *e == L'_' || iswalnum(*e))) e++; if (e == s) { streams.err.append_format(BUILTIN_ERR_INVALID_OPT_SPEC, opts.name.c_str(), option_spec.c_str(), *(s - 1)); return false; } - opt_spec->long_flag = wcstring(s, e - s); opts.long_to_short_flag.emplace(opt_spec->long_flag, opt_spec->short_flag); + return parse_flag_modifiers(opts, opt_spec, option_spec, e, streams); } @@ -236,7 +237,7 @@ static int collect_option_specs(argparse_cmd_opts_t &opts, int *optind, int argc io_streams_t &streams) { wchar_t *cmd = argv[0]; - while (*optind < argc) { + while (true) { if (wcscmp(L"--", argv[*optind]) == 0) { ++*optind; break; @@ -246,7 +247,10 @@ static int collect_option_specs(argparse_cmd_opts_t &opts, int *optind, int argc return STATUS_CMD_ERROR; } - ++*optind; + if (++*optind == argc) { + streams.err.append_format(_(L"%ls: Missing -- separator\n"), cmd); + return STATUS_INVALID_ARGS; + } } if (opts.options.empty()) { @@ -317,6 +321,12 @@ static int parse_cmd_opts(argparse_cmd_opts_t &opts, int *optind, //!OCLINT(hig } } + if (argc == w.woptind || wcscmp(L"--", argv[w.woptind - 1]) == 0) { + // The user didn't specify any option specs. + streams.err.append_format(_(L"%ls: No option specs were provided\n"), cmd); + return STATUS_INVALID_ARGS; + } + *optind = w.woptind; return collect_option_specs(opts, optind, argc, argv, streams); } @@ -417,9 +427,8 @@ static int argparse_parse_args(argparse_cmd_opts_t &opts, const wcstring_list_t const wchar_t *cmd = opts.name.c_str(); int argc = static_cast(args.size()); - // This is awful but we need to convert our wcstring_list_t to a that can be passed - // to w.wgetopt_long(). Furthermore, because we're dynamically allocating the array of pointers - // we need to ensure the memory for the data structure is freed when we leave this scope. + // We need to convert our wcstring_list_t to a that can be used by wgetopt_long(). + // This ensures the memory for the data structure is freed when we leave this scope. null_terminated_array_t argv_container(args); auto argv = (wchar_t **)argv_container.get(); @@ -433,7 +442,17 @@ static int argparse_parse_args(argparse_cmd_opts_t &opts, const wcstring_list_t update_bool_flag_counts(opts); for (int i = optind; argv[i]; i++) opts.argv.push_back(argv[i]); - if (opts.min_args > 0 && opts.argv.size() < opts.min_args) { + + return STATUS_CMD_OK; +} + +static int check_min_max_args_constraints(argparse_cmd_opts_t &opts, const wcstring_list_t &args, + parser_t &parser, io_streams_t &streams) { + UNUSED(parser); + const wchar_t *cmd = opts.name.c_str(); + int argc = static_cast(args.size()); + + if (opts.argv.size() < opts.min_args) { streams.err.append_format(BUILTIN_ERR_MIN_ARG_COUNT1, cmd, opts.min_args, opts.argv.size()); return STATUS_CMD_ERROR; } @@ -466,11 +485,13 @@ int builtin_argparse(parser_t &parser, io_streams_t &streams, wchar_t **argv) { return STATUS_CMD_OK; } +#if 0 if (optind == argc) { // Apparently we weren't handed any arguments to be parsed according to the option specs we // just collected. So there isn't anything for us to do. return STATUS_CMD_OK; } +#endif wcstring_list_t args; args.push_back(opts.name); @@ -482,6 +503,9 @@ int builtin_argparse(parser_t &parser, io_streams_t &streams, wchar_t **argv) { retval = argparse_parse_args(opts, args, parser, streams); if (retval != STATUS_CMD_OK) return retval; + retval = check_min_max_args_constraints(opts, args, parser, streams); + if (retval != STATUS_CMD_OK) return retval; + for (auto it : opts.options) { option_spec_t *opt_spec = it.second; if (!opt_spec->num_seen) continue; diff --git a/tests/argparse.err b/tests/argparse.err new file mode 100644 index 000000000..24b07730a --- /dev/null +++ b/tests/argparse.err @@ -0,0 +1,16 @@ +# No args is an error +argparse: No option specs were provided +# Missing -- is an error +argparse: Missing -- separator +# Flags but no option specs is an error +argparse: No option specs were provided +# Invalid option specs +argparse: Invalid option spec 'h-' at char '-' +argparse: Invalid option spec '+help' at char 'h' +argparse: Invalid option spec 'h/help:' at char ':' +argparse: Invalid option spec 'h-help::' at char ':' +argparse: Invalid option spec 'h-help=x' at char 'x' +# --max-args and --min-args work +min-max: Expected at least 1 args, got only 0 +min-max: Expected at most 3 args, got 4 +min-max: Expected at most 1 args, got 2 diff --git a/tests/argparse.in b/tests/argparse.in new file mode 100644 index 000000000..9eb565248 --- /dev/null +++ b/tests/argparse.in @@ -0,0 +1,66 @@ +# Test the `argparse` builtin. + +########## +# Start by verifying a bunch of error conditions. + +echo '# No args is an error' >&2 +argparse + +echo '# Missing -- is an error' >&2 +argparse h/help + +echo '# Flags but no option specs is an error' >&2 +argparse -s -- hello + +echo '# Invalid option specs' >&2 +argparse h- +argparse +help +argparse h/help: +argparse h-help:: +argparse h-help=x + +echo '# --max-args and --min-args work' >&2 +begin + argparse --name min-max --min-args 1 h/help -- + argparse --name min-max --min-args 1 --max-args 3 h/help -- arg1 + argparse --name min-max --min-args 1 --max-args 3 h/help -- arg1 arg2 + argparse --name min-max --min-args 1 --max-args 3 h/help -- --help arg1 arg2 arg3 + argparse --name min-max --min-args 1 --max-args 3 h/help -- arg1 arg2 -h arg3 arg4 + argparse --name min-max --max-args 1 h/help -- + argparse --name min-max --max-args 1 h/help -- arg1 + argparse --name min-max --max-args 1 h/help -- arg1 arg2 +end + +########## +# Now verify that validly formed invocations work as expected. + +echo '# No args' +argparse h/help -- + +echo '# One arg and no matching flags' +begin + argparse h/help -- help + set -l + show $argv +end + +echo '# Five args with two matching a flag' +begin + argparse h/help -- help --help me -h 'a lot more' + set -l + show $argv +end + +echo '# Required, optional, and multiple flags' +begin + argparse 'h/help' 'a/abc=' 'd/def=?' 'g/ghk=+' -- help --help me --ghk=g1 --abc=ABC --ghk g2 --d -g g3 + set -l + show $argv +end + +echo '# --stop-nonopt works' +begin + argparse --stop-nonopt 'h/help' 'a/abc=' -- -a A1 -h --abc A2 non-opt 'second non-opt' --help + set -l + show $argv +end diff --git a/tests/argparse.out b/tests/argparse.out new file mode 100644 index 000000000..d508230d6 --- /dev/null +++ b/tests/argparse.out @@ -0,0 +1,36 @@ +# No args +# One arg and no matching flags +argv help +count=1 +|help| +# Five args with two matching a flag +_flag_h 2 +_flag_help 2 +argv 'help' 'me' 'a lot more' +count=3 +|help| +|me| +|a lot more| +# Required, optional, and multiple flags +_flag_a ABC +_flag_abc ABC +_flag_d +_flag_def +_flag_g 'g1' 'g2' 'g3' +_flag_ghk 'g1' 'g2' 'g3' +_flag_h 1 +_flag_help 1 +argv 'help' 'me' +count=2 +|help| +|me| +# --stop-nonopt works +_flag_a A2 +_flag_abc A2 +_flag_h 1 +_flag_help 1 +argv 'non-opt' 'second non-opt' '--help' +count=3 +|non-opt| +|second non-opt| +|--help| diff --git a/tests/test_functions/show.fish b/tests/test_functions/show.fish new file mode 100644 index 000000000..d644bb3eb --- /dev/null +++ b/tests/test_functions/show.fish @@ -0,0 +1,4 @@ +function show + echo count=(count $argv) + printf '|%s|\n' $argv +end