diff --git a/doc_src/argparse.txt b/doc_src/argparse.txt
index b33198af1..ae763382d 100644
--- a/doc_src/argparse.txt
+++ b/doc_src/argparse.txt
@@ -74,10 +74,28 @@ Each option specification is a string composed of
- `=+` if it requires a value each instance of the flag is saved.
+- Optionally a `!` followed by fish script to validate the value. Typically this will be a function to run. If the return status is zero the value for the flag is valid. If non-zero the value is invalid. Any error messages should be written to stdout (not stderr). See the section on Flag Value Validation for more information.
+
See the `fish_opt` command for a friendlier but more verbose way to create option specifications.
In the following examples if a flag is not seen when parsing the arguments then the corresponding _flag_X var(s) will not be set.
+\subsection argparse-validation Flag Value Validation
+
+It is common to want to validate the the value provided for an option satisfies some criteria. For example, that it is a valid integer within a specific range. You can always do this after `argparse` returns but you can also request that `argparse` perform the validation by executing arbitrary fish script. To do so simply append an `!` (exclamation-mark) then the fish script to be run. When that code is executed three vars will be defined:
+
+- `_argparse_cmd` will be set to the value of the value of the `argparse --name` value.
+
+- `_flag_name` will be set to the short or long flag that being processed.
+
+- `_flag_value` will be set to the value associated with the flag being processed.
+
+If you do this via a function it should be defined with the `--no-scope-shadowing` flag. Otherwise it won't have access to those variables.
+
+The script should write any error messages to stdout, not stderr. It should return a status of zero if the flag value is valid otherwise a non-zero status to indicate it is invalid.
+
+Fish ships with a `_validate_int` function that accepts a `--min` and `--max` flag. Let's say your command accepts a `-m` or `--max` flag and the minimum allowable value is zero and the maximum is 5. You would define the option like this: `m/max=!_validate_int --min 0 --max 5`. The default if you just call `_validate_int` without those flags is to simply check that the value is a valid integer with no limits on the min or max value allowed.
+
\subsection argparse-optspec-examples Example OPTION_SPECs
Some OPTION_SPEC examples:
diff --git a/share/functions/_validate_int.fish b/share/functions/_validate_int.fish
new file mode 100644
index 000000000..629cf4c73
--- /dev/null
+++ b/share/functions/_validate_int.fish
@@ -0,0 +1,32 @@
+# This function is intended to be used as a validation command for individual option specifications
+# given to the `argparse` command. It checks that the argument is a valid integer and optionally
+# whether it is in a reasonable range.
+function _validate_int --no-scope-shadowing
+ # Note that we can't use ourself to validate the arguments to the --min and --max flags this
+ # function recognizes.
+ set -l options 'm-min=' 'x-max='
+ argparse -n _argparse_validate_int $options -- $argv
+ or return
+
+ if not string match -qr '^-?\d+$' -- $_flag_value
+ set -l msg (_ "%s: Value '%s' for flag '%s' is not an integer\n")
+ printf $msg $_argparse_cmd $_flag_value $_flag_name
+ return 1
+ end
+
+ if set -q _flag_min
+ and test $_flag_value -lt $_flag_min
+ set -l msg (_ "%s: Value '%s' for flag '%s' less than min allowed of '%s'\n")
+ printf $msg $_argparse_cmd $_flag_value $_flag_name $_flag_min
+ return 1
+ end
+
+ if set -q _flag_max
+ and test $_flag_value -gt $_flag_max
+ set -l msg (_ "%s: Value '%s' for flag '%s' greater than max allowed of '%s'\n")
+ printf $msg $_argparse_cmd $_flag_value $_flag_name $_flag_max
+ return 1
+ end
+
+ return 0
+end
diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp
index f414a44dc..363328d8c 100644
--- a/src/builtin_argparse.cpp
+++ b/src/builtin_argparse.cpp
@@ -24,12 +24,14 @@
#include "builtin_argparse.h"
#include "common.h"
#include "env.h"
+#include "exec.h"
#include "fallback.h" // IWYU pragma: keep
#include "io.h"
#include "wgetopt.h" // IWYU pragma: keep
#include "wutil.h" // IWYU pragma: keep
class parser_t;
+const wcstring var_name_prefix = L"_flag_";
#define BUILTIN_ERR_INVALID_OPT_SPEC _(L"%ls: Invalid option spec '%ls' at char '%lc'\n")
@@ -37,13 +39,20 @@ class option_spec_t {
public:
wchar_t short_flag;
wcstring long_flag;
+ wcstring validation_command;
wcstring_list_t vals;
bool short_flag_valid;
int num_allowed;
int num_seen;
option_spec_t(wchar_t s)
- : short_flag(s), long_flag(), vals(), short_flag_valid(true), num_allowed(0), num_seen(0) {}
+ : short_flag(s),
+ long_flag(),
+ validation_command(),
+ vals(),
+ short_flag_valid(true),
+ num_allowed(0),
+ num_seen(0) {}
};
class argparse_cmd_opts_t {
@@ -174,9 +183,10 @@ 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 == 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);
+ if (opt_spec->short_flag == opts.implicit_int_flag && *s && *s != L'!') {
+ 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;
}
@@ -193,12 +203,20 @@ static bool parse_flag_modifiers(argparse_cmd_opts_t &opts, option_spec_t *opt_s
}
}
- if (*s) {
+ if (*s == L'!') {
+ s++;
+ opt_spec->validation_command = wcstring(s);
+ } else if (*s) {
streams.err.append_format(BUILTIN_ERR_INVALID_OPT_SPEC, opts.name.c_str(),
option_spec.c_str(), *s);
return false;
}
+ // Make sure we have some validation for implicit int flags.
+ if (opt_spec->short_flag == opts.implicit_int_flag && opt_spec->validation_command.empty()) {
+ opt_spec->validation_command = L"_validate_int";
+ }
+
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);
@@ -234,8 +252,8 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec,
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);
+ 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;
@@ -248,8 +266,8 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec,
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);
+ 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;
@@ -417,37 +435,91 @@ static void update_bool_flag_counts(argparse_cmd_opts_t &opts) {
}
}
+static int validate_arg(argparse_cmd_opts_t &opts, option_spec_t *opt_spec, bool is_long_flag,
+ const wchar_t *woptarg, io_streams_t &streams) {
+ // Obviously if there is no arg validation command we assume the arg is okay.
+ if (opt_spec->validation_command.empty()) return STATUS_CMD_OK;
+
+ wcstring_list_t cmd_output;
+
+ env_push(true);
+ env_set(L"_argparse_cmd", opts.name.c_str(), ENV_LOCAL);
+ if (is_long_flag) {
+ env_set(var_name_prefix + L"name", opt_spec->long_flag.c_str(), ENV_LOCAL);
+ } else {
+ env_set(var_name_prefix + L"name", wcstring(1, opt_spec->short_flag).c_str(), ENV_LOCAL);
+ }
+ env_set(var_name_prefix + L"value", woptarg, ENV_LOCAL);
+
+ int retval = exec_subshell(opt_spec->validation_command, cmd_output, false);
+ for (auto it : cmd_output) {
+ streams.err.append(it);
+ streams.err.push_back(L'\n');
+ }
+ env_pop();
+ return retval;
+}
+
+// Check if it is an implicit integer option. Try to parse and validate it as a valid integer. The
+// code that parses option specs has already ensured that implicit integers have either an explicit
+// or implicit validation function.
+static int check_for_implicit_int(argparse_cmd_opts_t &opts, const wchar_t *val, const wchar_t *cmd,
+ const wchar_t *const *argv, wgetopter_t &w, int opt, int long_idx,
+ parser_t &parser, io_streams_t &streams) {
+ if (opts.implicit_int_flag == L'\0') {
+ // There is no implicit integer option so report an error.
+ builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]);
+ return STATUS_INVALID_ARGS;
+ }
+
+ if (opt == '?') {
+ // It's a flag that might be an implicit integer flag. Try to parse it as an integer to
+ // decide if it is in fact something like `-NNN` or an invalid flag.
+ (void)fish_wcstol(val);
+ if (errno) {
+ builtin_unknown_option(parser, streams, cmd, argv[w.woptind - 1]);
+ return STATUS_INVALID_ARGS;
+ }
+ }
+
+ // Okay, it looks like an integer. See if it passes the validation checks.
+ auto found = opts.options.find(opts.implicit_int_flag);
+ assert(found != opts.options.end());
+ auto opt_spec = found->second;
+ int retval = validate_arg(opts, opt_spec, long_idx != -1, val, streams);
+ if (retval != STATUS_CMD_OK) return retval;
+
+ // It's a valid integer so store it and return success.
+ opt_spec->vals.clear();
+ opt_spec->vals.push_back(wcstring(val));
+ opt_spec->num_seen++;
+ w.nextchar = NULL;
+ return STATUS_CMD_OK;
+}
+
static int argparse_parse_flags(argparse_cmd_opts_t &opts, const wchar_t *short_options,
const woption *long_options, const wchar_t *cmd, int argc,
wchar_t **argv, int *optind, parser_t &parser,
io_streams_t &streams) {
int opt;
+ int long_idx = -1;
wgetopter_t w;
- while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, NULL)) != -1) {
+ while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, &long_idx)) != -1) {
if (opt == ':') {
builtin_missing_argument(parser, streams, cmd, argv[w.woptind - 1]);
return STATUS_INVALID_ARGS;
}
+
if (opt == '?') {
- 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);
- 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;
+ // It's not a recognized flag. See if it's an implicit int flag.
+ int retval = check_for_implicit_int(opts, argv[w.woptind - 1] + 1, cmd, argv, w, opt,
+ long_idx, parser, streams);
+ if (retval != STATUS_CMD_OK) return retval;
+ long_idx = -1;
+ continue;
}
+ // It's a recognized flag.
auto found = opts.options.find(opt);
assert(found != opts.options.end());
@@ -455,7 +527,16 @@ static int argparse_parse_flags(argparse_cmd_opts_t &opts, const wchar_t *short_
opt_spec->num_seen++;
if (opt_spec->num_allowed == 0) {
assert(!w.woptarg);
- } else if (opt_spec->num_allowed == -1 || opt_spec->num_allowed == 1) {
+ long_idx = -1;
+ continue;
+ }
+
+ if (w.woptarg) {
+ int retval = validate_arg(opts, opt_spec, long_idx != -1, w.woptarg, streams);
+ if (retval != STATUS_CMD_OK) return retval;
+ }
+
+ if (opt_spec->num_allowed == -1 || opt_spec->num_allowed == 1) {
// We're depending on `wgetopt_long()` to report that a mandatory value is missing if
// `opt_spec->num_allowed == 1` and thus return ':' so that we don't take this branch if
// the mandatory arg is missing.
@@ -467,6 +548,8 @@ static int argparse_parse_flags(argparse_cmd_opts_t &opts, const wchar_t *short_
assert(w.woptarg);
opt_spec->vals.push_back(w.woptarg);
}
+
+ long_idx = -1;
}
*optind = w.woptind;
@@ -533,7 +616,6 @@ static void set_argparse_result_vars(argparse_cmd_opts_t &opts) {
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(),
diff --git a/tests/argparse.err b/tests/argparse.err
index c5b5d9d92..37bcf54ca 100644
--- a/tests/argparse.err
+++ b/tests/argparse.err
@@ -29,3 +29,9 @@ argparse: Long flag 'short' already defined
argparse: Implicit int flag '#' already defined
# Defining an implicit int flag with modifiers
argparse: Implicit int short flag 'v' does not allow modifiers like '='
+# Implicit int flag validation fails
+argparse: Value '765x' for flag 'max' is not an integer
+argparse: Value 'a1' for flag 'm' is not an integer
+# Explicit int flag validation
+argparse: Value '2' for flag 'm' greater than max allowed of '1'
+argparse: Value '-1' for flag 'max' less than min allowed of '0'
diff --git a/tests/argparse.in b/tests/argparse.in
index cf467fc9d..24b9f7979 100644
--- a/tests/argparse.in
+++ b/tests/argparse.in
@@ -98,3 +98,30 @@ 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
+
+##########
+# Verify that flag value validation works.
+
+echo '# Implicit int flag validation fails' >&2
+argparse 'm#max' -- argle --max 765x bargle
+and echo unxpected argparse return status >&2
+argparse 'm#max' -- argle -ma1 bargle
+and echo unxpected argparse return status >&2
+
+echo '# Check the exit status from argparse validation'
+argparse 'm#max!set | grep _flag_; function x; return 57; end; x' -- argle --max=83 bargle 2>&1
+set -l saved_status $status
+test $saved_status -eq 57
+and echo expected argparse return status $saved_status
+
+echo '# Explicit int flag validation' >&2
+# These should fail
+argparse 'm#max!_validate_int --min 1 --max 1' -- argle -m2 bargle
+and echo unexpected argparse return status $status >&2
+argparse 'm#max!_validate_int --min 0 --max 1' -- argle --max=-1 bargle
+and echo unexpected argparse return status $status >&2
+# These should succeed
+argparse 'm/max=!_validate_int --min 0 --max 1' -- argle --max=0 bargle
+or echo unexpected argparse return status $status >&2
+argparse 'm/max=!_validate_int --min 0 --max 1' -- argle --max=1 bargle
+or echo unexpected argparse return status $status >&2
diff --git a/tests/argparse.out b/tests/argparse.out
index 2031ae09a..79c13b377 100644
--- a/tests/argparse.out
+++ b/tests/argparse.out
@@ -51,3 +51,7 @@ argv 'argle' 'bargle'
_flag_m 765
_flag_max 765
argv 'argle' 'bargle'
+# Check the exit status from argparse validation
+_flag_name max
+_flag_value 83
+expected argparse return status 57