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

@@ -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.

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

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);

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 '='

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

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'