From 3e226f0a5ea2d848cd57b094333f5d68c5fff3f0 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sun, 16 Jul 2017 18:27:41 -0700 Subject: [PATCH] implement a new implicit int option spec While updating the `history` function to use `argparse` I realized it is useful to define an option that can be used in three ways. First by using the short flag; e.g., `-n NNN`. Second by using the long flag; e.g., `--max NNN`. Third, as an implicit int flag; e.g., `-NNN`. This use case is now supported by a spec of the form `n#max`. --- doc_src/argparse.txt | 6 ++++-- share/functions/history.fish | 3 ++- src/builtin_argparse.cpp | 35 +++++++++++++++++++++++++++++++---- tests/argparse.err | 10 +++++++++- tests/argparse.in | 24 +++++++++++++++++++++++- tests/argparse.out | 10 +++++++++- 6 files changed, 78 insertions(+), 10 deletions(-) diff --git a/doc_src/argparse.txt b/doc_src/argparse.txt index ce9c2d435..b33198af1 100644 --- a/doc_src/argparse.txt +++ b/doc_src/argparse.txt @@ -62,11 +62,11 @@ Each option specification is a string composed of - 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. +- 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. You can also specify a '#' to indicate the short and long flag names can be used and the value can be specified as an implicit int; i.e., a flag of the form `-NNN`. - A long flag name which is optional. If not present then only the short flag letter can be used. -- Nothing if the flag is a boolean that takes no argument, else +- Nothing if the flag is a boolean that takes no argument or is an implicit int flag, else - `=` if it requires a value and only the last instance of the flag is saved, else @@ -100,6 +100,8 @@ Some OPTION_SPEC examples: - `#-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. +- `n#max` means that flags matching the regex "^--?\d+$" are valid. When seen they are assigned to the variables `_flag_n` and `_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. You can also specify the value using either flag: `-n NNN` or `--max NNN` in this example. + 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/share/functions/history.fish b/share/functions/history.fish index 9581b9fbf..0545aae23 100644 --- a/share/functions/history.fish +++ b/share/functions/history.fish @@ -20,7 +20,7 @@ function history --description "display or manipulate interactive command histor set -l options --exclusive 'c,e,p' --exclusive 'S,D,M,V,C' --exclusive 't,T' set options $options 'h/help' 'c/contains' 'e/exact' 'p/prefix' - set options $options 'C/case-sensitive' 'z/null' 't/show-time=?' 'n/max=' '#-max' + set options $options 'C/case-sensitive' 'z/null' 't/show-time=?' 'n#max' # This long option is deprecated and here solely for legacy compatibility. People should use # -t or --show-time now. set options $options 'T-with-time=?' @@ -37,6 +37,7 @@ function history --description "display or manipulate interactive command histor set -l hist_cmd set -l show_time + set -l max_count $_flag_max set -q _flag_with_time diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp index c0c5e51e0..f414a44dc 100644 --- a/src/builtin_argparse.cpp +++ b/src/builtin_argparse.cpp @@ -52,6 +52,7 @@ class argparse_cmd_opts_t { bool stop_nonopt = false; size_t min_args = 0; size_t max_args = SIZE_MAX; + wchar_t implicit_int_flag = L'\0'; wcstring name = L"argparse"; wcstring_list_t raw_exclusive_flags; wcstring_list_t argv; @@ -173,9 +174,9 @@ 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); + if (opt_spec->short_flag == opts.implicit_int_flag && *s) { + streams.err.append_format(_(L"%ls: Implicit int short flag '%lc' does not allow modifiers like '%lc'\n"), + opts.name.c_str(), opt_spec->short_flag, *s); return false; } @@ -198,6 +199,12 @@ static bool parse_flag_modifiers(argparse_cmd_opts_t &opts, option_spec_t *opt_s return false; } + if (opts.options.find(opt_spec->short_flag) != opts.options.end()) { + streams.err.append_format(L"%ls: Short flag '%lc' already defined\n", opts.name.c_str(), + opt_spec->short_flag); + return false; + } + opts.options.emplace(opt_spec->short_flag, opt_spec); return true; } @@ -226,6 +233,12 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, opts.name.c_str()); return false; } + if (opts.implicit_int_flag) { + streams.err.append_format( + _(L"%ls: Implicit int flag '%lc' already defined\n"), opts.name.c_str(), opts.implicit_int_flag); + return false; + } + opts.implicit_int_flag = opt_spec->short_flag; opt_spec->short_flag_valid = false; s++; } else if (*s == L'-') { @@ -233,6 +246,15 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, s++; } else if (*s == L'/') { s++; // the struct is initialized assuming short_flag_valid should be true + } else if (*s == L'#') { + if (opts.implicit_int_flag) { + streams.err.append_format( + _(L"%ls: Implicit int flag '%lc' already defined\n"), opts.name.c_str(), opts.implicit_int_flag); + return false; + } + opts.implicit_int_flag = opt_spec->short_flag; + opt_spec->num_allowed = 1; // mandatory arg and can appear only once + 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. @@ -248,6 +270,11 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, return false; } opt_spec->long_flag = wcstring(s, e - s); + if (opts.long_to_short_flag.find(opt_spec->long_flag) != opts.long_to_short_flag.end()) { + streams.err.append_format(L"%ls: Long flag '%ls' already defined\n", opts.name.c_str(), + opt_spec->long_flag.c_str()); + return false; + } 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); @@ -402,7 +429,7 @@ 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'#'); + auto found = opts.options.find(opts.implicit_int_flag); if (found != opts.options.end()) { // Try to parse it as a number; e.g., "-123". long x = fish_wcstol(argv[w.woptind - 1] + 1); diff --git a/tests/argparse.err b/tests/argparse.err index 4816f3953..c5b5d9d92 100644 --- a/tests/argparse.err +++ b/tests/argparse.err @@ -15,9 +15,17 @@ 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 '=' +argparse: Implicit int 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 ^ +# Defining a short flag more than once +argparse: Short flag 's' already defined +# Defining a long flag more than once +argparse: Long flag 'short' already defined +# Defining an implicit int flag more than once +argparse: Implicit int flag '#' already defined +# Defining an implicit int flag with modifiers +argparse: Implicit int short flag 'v' does not allow modifiers like '=' diff --git a/tests/argparse.in b/tests/argparse.in index de9c5cd00..cf467fc9d 100644 --- a/tests/argparse.in +++ b/tests/argparse.in @@ -37,6 +37,18 @@ argparse '#-val=' -- abc -x def echo '# Invalid arg in the face of a "#-val" spec' >&2 argparse '#-val' -- abc -x def +echo '# Defining a short flag more than once' >&2 +argparse 's/short' 'x/xray' 's/long' -- -s -x --long + +echo '# Defining a long flag more than once' >&2 +argparse 's/short' 'x/xray' 'l/short' -- -s -x --long + +echo '# Defining an implicit int flag more than once' >&2 +argparse '#-val' 'x/xray' 'v#val' -- -s -x --long + +echo '# Defining an implicit int flag with modifiers' >&2 +argparse 'v#val=' -- + ########## # Now verify that validly formed invocations work as expected. @@ -71,8 +83,18 @@ begin show $argv end -echo '# A "#-val" spec works' +echo '# Implicit int flags work' +for v in (set -l -n); set -e $v; end argparse '#-val' -- abc -123 def set -l +for v in (set -l -n); set -e $v; end argparse 'v/verbose' '#-val' 't/token=' -- -123 a1 --token woohoo --234 -v a2 --verbose set -l +echo '# Should be set to 987' +for v in (set -l -n); set -e $v; end +argparse 'm#max' -- argle -987 bargle +set -l +echo '# Should be set to 765' +for v in (set -l -n); set -e $v; end +argparse 'm#max' -- argle -987 bargle --max 765 +set -l diff --git a/tests/argparse.out b/tests/argparse.out index 922c3ef20..2031ae09a 100644 --- a/tests/argparse.out +++ b/tests/argparse.out @@ -34,7 +34,7 @@ count=3 |non-opt| |second non-opt| |--help| -# A "#-val" spec works +# Implicit int flags work _flag_val 123 argv 'abc' 'def' _flag_t woohoo @@ -43,3 +43,11 @@ _flag_v 2 _flag_val -234 _flag_verbose 2 argv 'a1' 'a2' +# Should be set to 987 +_flag_m 987 +_flag_max 987 +argv 'argle' 'bargle' +# Should be set to 765 +_flag_m 765 +_flag_max 765 +argv 'argle' 'bargle'