diff --git a/doc_src/argparse.txt b/doc_src/argparse.txt index 289c8e78e..82e78bce1 100644 --- a/doc_src/argparse.txt +++ b/doc_src/argparse.txt @@ -60,7 +60,7 @@ The first `--` seen is what allows the `argparse` command to reliably seperate t Each option specification is a string composed of -- A short flag letter (which is mandatory). +- A short flag letter (which is mandatory). It must be an alphanumeric or "#". The "#" character is special and means that a flag of the form `-123` is valid. The short flag "#" must be followed by "-" (since the short name isn't otherwise valid since `_flag_#` is not a valid var name) and must but followed by a long flag name with no modifiers. - A `/` if the short flag can be used by someone invoking your command else `-` if it should not be exposed as a valid short flag. If there is no long flag name these characters should be omitted. @@ -98,7 +98,9 @@ Some OPTION_SPEC examples: - `x-` is not valid since there is no long flag name and therefore the short flag, `-x`, has to be usable. This is obviously true whether or not the specification also includes one of `:`, `::`, `+`. -After parsing the arguments the argv var is set (with local scope) to any values not already consumed during flag processing. +- `#-max` means that flags matching the regex "^--?\d+$" are valid. When seen they are assigned to the variable `_flag_max`. This allows any valid positive or negative integer to be specified by prefixing it with a single "-". Many commands support this idiom. For example `head -3 /a/file` to emit only the first three lines of /a/file. + +After parsing the arguments the `argv` var is set with local scope to any values not already consumed during flag processing. If there are not unbound values the var is set but `count $argv` will be zero. If an error occurs during argparse processing it will exit with a non-zero status and appropriate error messages are written to stderr. diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp index edf7d4210..c0c5e51e0 100644 --- a/src/builtin_argparse.cpp +++ b/src/builtin_argparse.cpp @@ -173,6 +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 (opt_spec->short_flag == L'#' && *s) { + streams.err.append_format(_(L"%ls: Short flag '#' does not allow modifiers like '%lc'\n"), + opts.name.c_str(), *s); + return false; + } + if (*s == L'=') { s++; if (*s == L'?') { @@ -200,19 +206,33 @@ static bool parse_flag_modifiers(argparse_cmd_opts_t &opts, option_spec_t *opt_s static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, io_streams_t &streams) { if (option_spec.empty()) { - streams.err.append_format(_(L"%s: An option spec must have a short flag letter\n"), + streams.err.append_format(_(L"%ls: An option spec must have a short flag letter\n"), opts.name.c_str()); return false; } const wchar_t *s = option_spec.c_str(); + if (!iswalnum(*s) && *s != L'#') { + streams.err.append_format(_(L"%ls: Short flag '%lc' invalid, must be alphanum or '#'\n"), + opts.name.c_str(), *s); + return false; + } option_spec_t *opt_spec = new option_spec_t(*s++); - if (*s == L'/') { - s++; // the struct is initialized assuming short_flag_valid should be true + if (*(s - 1) == L'#') { + if (*s != L'-') { + streams.err.append_format( + _(L"%ls: Short flag '#' must be followed by '-' and a long name\n"), + opts.name.c_str()); + return false; + } + opt_spec->short_flag_valid = false; + s++; } else if (*s == L'-') { opt_spec->short_flag_valid = false; s++; + } else if (*s == L'/') { + s++; // the struct is initialized assuming short_flag_valid should be true } else { // Long flag name not allowed if second char isn't '/' or '-' so just check for // behavior modifier chars. @@ -361,6 +381,8 @@ static void populate_option_strings( static void update_bool_flag_counts(argparse_cmd_opts_t &opts) { for (auto it : opts.options) { auto opt_spec = it.second; + // The '#' short flag is special. It doesn't take any values but isn't a boolean arg. + if (opt_spec->short_flag == L'#') continue; if (opt_spec->num_allowed != 0 || opt_spec->num_seen == 0) continue; wchar_t count[20]; swprintf(count, sizeof count / sizeof count[0], L"%d", opt_spec->num_seen); @@ -380,6 +402,21 @@ static int argparse_parse_flags(argparse_cmd_opts_t &opts, const wchar_t *short_ return STATUS_INVALID_ARGS; } if (opt == '?') { + auto found = opts.options.find(L'#'); + if (found != opts.options.end()) { + // Try to parse it as a number; e.g., "-123". + long x = fish_wcstol(argv[w.woptind - 1] + 1); + if (!errno) { + wchar_t val[20]; + swprintf(val, sizeof val / sizeof val[0], L"%ld", x); + auto opt_spec = found->second; + opt_spec->vals.clear(); + opt_spec->vals.push_back(wcstring(val)); + opt_spec->num_seen++; + w.nextchar = NULL; + continue; + } + } builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]); return STATUS_INVALID_ARGS; } @@ -446,11 +483,10 @@ static int argparse_parse_args(argparse_cmd_opts_t &opts, const wcstring_list_t 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) { +static int check_min_max_args_constraints(argparse_cmd_opts_t &opts, 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()); @@ -464,6 +500,34 @@ static int check_min_max_args_constraints(argparse_cmd_opts_t &opts, const wcstr return STATUS_CMD_OK; } +/// Put the result of parsing the supplied args into the caller environment as local vars. +static void set_argparse_result_vars(argparse_cmd_opts_t &opts) { + for (auto it : opts.options) { + option_spec_t *opt_spec = it.second; + if (!opt_spec->num_seen) continue; + + wcstring var_name_prefix = L"_flag_"; + auto val = list_to_array_val(opt_spec->vals); + if (opt_spec->short_flag_valid) { + env_set(var_name_prefix + opt_spec->short_flag, *val == ENV_NULL ? NULL : val->c_str(), + ENV_LOCAL); + } + if (!opt_spec->long_flag.empty()) { + // We do a simple replacement of all non alphanum chars rather than calling + // escape_string(long_flag, 0, STRING_STYLE_VAR). + wcstring long_flag = opt_spec->long_flag; + for (size_t pos = 0; pos < long_flag.size(); pos++) { + if (!iswalnum(long_flag[pos])) long_flag[pos] = L'_'; + } + env_set(var_name_prefix + long_flag, *val == ENV_NULL ? NULL : val->c_str(), + ENV_LOCAL); + } + } + + auto val = list_to_array_val(opts.argv); + env_set(L"argv", *val == ENV_NULL ? NULL : val->c_str(), ENV_LOCAL); +} + /// The argparse builtin. This is explicitly not compatible with the BSD or GNU version of this /// command. That's because fish doesn't have the weird quoting problems of POSIX shells. So we /// don't need to support flags like `--unquoted`. Similarly we don't want to support introducing @@ -485,14 +549,6 @@ 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); while (optind < argc) args.push_back(argv[optind++]); @@ -503,31 +559,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); + retval = check_min_max_args_constraints(opts, 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; - - wcstring var_name_prefix = L"_flag_"; - auto val = list_to_array_val(opt_spec->vals); - env_set(var_name_prefix + opt_spec->short_flag, *val == ENV_NULL ? NULL : val->c_str(), - ENV_LOCAL); - if (!opt_spec->long_flag.empty()) { - // We do a simple replacement of all non alphanum chars rather than calling - // escape_string(long_flag, 0, STRING_STYLE_VAR). - wcstring long_flag = opt_spec->long_flag; - for (size_t pos = 0; pos < long_flag.size(); pos++) { - if (!iswalnum(long_flag[pos])) long_flag[pos] = L'_'; - } - env_set(var_name_prefix + long_flag, *val == ENV_NULL ? NULL : val->c_str(), - ENV_LOCAL); - } - } - - auto val = list_to_array_val(opts.argv); - env_set(L"argv", *val == ENV_NULL ? NULL : val->c_str(), ENV_LOCAL); - + set_argparse_result_vars(opts); return retval; } diff --git a/tests/argparse.err b/tests/argparse.err index 24b07730a..4816f3953 100644 --- a/tests/argparse.err +++ b/tests/argparse.err @@ -6,7 +6,7 @@ argparse: Missing -- separator 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: Short flag '+' invalid, must be alphanum or '#' 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' @@ -14,3 +14,10 @@ argparse: Invalid option spec 'h-help=x' at char 'x' 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 +# Invalid "#-val" spec +argparse: Short flag '#' does not allow modifiers like '=' +# Invalid arg in the face of a "#-val" spec +argparse: Unknown option '-x' +Standard input (line 38): +argparse '#-val' -- abc -x def +^ diff --git a/tests/argparse.in b/tests/argparse.in index 9eb565248..de9c5cd00 100644 --- a/tests/argparse.in +++ b/tests/argparse.in @@ -31,6 +31,12 @@ begin argparse --name min-max --max-args 1 h/help -- arg1 arg2 end +echo '# Invalid "#-val" spec' >&2 +argparse '#-val=' -- abc -x def + +echo '# Invalid arg in the face of a "#-val" spec' >&2 +argparse '#-val' -- abc -x def + ########## # Now verify that validly formed invocations work as expected. @@ -64,3 +70,9 @@ begin set -l show $argv end + +echo '# A "#-val" spec works' +argparse '#-val' -- abc -123 def +set -l +argparse 'v/verbose' '#-val' 't/token=' -- -123 a1 --token woohoo --234 -v a2 --verbose +set -l diff --git a/tests/argparse.out b/tests/argparse.out index d508230d6..922c3ef20 100644 --- a/tests/argparse.out +++ b/tests/argparse.out @@ -34,3 +34,12 @@ count=3 |non-opt| |second non-opt| |--help| +# A "#-val" spec works +_flag_val 123 +argv 'abc' 'def' +_flag_t woohoo +_flag_token woohoo +_flag_v 2 +_flag_val -234 +_flag_verbose 2 +argv 'a1' 'a2' diff --git a/tests/string.err b/tests/string.err index 7092f30c2..291095371 100644 --- a/tests/string.err +++ b/tests/string.err @@ -5,7 +5,7 @@ string match: ^ # string invalidarg string: Subcommand 'invalidarg' is not valid -Standard input (line 258): +Standard input (line 251): string invalidarg; and echo "unexpected exit 0" >&2 ^ @@ -29,6 +29,6 @@ string repeat: Expected argument # string repeat -l fakearg 2>&1 string repeat: Unknown option '-l' -Standard input (line 359): +Standard input (line 352): string repeat -l fakearg ^ diff --git a/tests/string.in b/tests/string.in index 4e66b7950..da9e7be29 100644 --- a/tests/string.in +++ b/tests/string.in @@ -1,12 +1,5 @@ # Tests for string builtin. Mostly taken from man page examples. -# We don't want syntax errors to emit command usage help. This makes the -# stderr output considerably shorter and makes it easier to updates the tests -# and documentation without having to make pointless changes to the test -# output files. -function __fish_print_help -end - echo '# string match -r -v "c.*" dog can cat diz' string match -r -v "c.*" dog can cat diz; and echo "exit 0" diff --git a/tests/test_functions/__fish_print_help.fish b/tests/test_functions/__fish_print_help.fish new file mode 100644 index 000000000..cd2da5512 --- /dev/null +++ b/tests/test_functions/__fish_print_help.fish @@ -0,0 +1,6 @@ +# We don't want syntax errors to emit command usage help. This makes the +# stderr output considerably shorter and makes it easier to updates the tests +# and documentation without having to make pointless changes to the test +# output files. +function __fish_print_help +end