From 61f0756fe606ca927e5fe493cc32e746462fa72f Mon Sep 17 00:00:00 2001 From: Aaron Gyes Date: Tue, 17 Sep 2019 22:00:08 -0700 Subject: [PATCH] builtins: Use standard builtin.h error macros more --- src/builtin.h | 7 ++++--- src/builtin_commandline.cpp | 2 +- src/builtin_exit.cpp | 3 +-- src/builtin_functions.cpp | 6 +++--- src/builtin_history.cpp | 3 +-- src/builtin_random.cpp | 4 ++-- src/builtin_read.cpp | 3 +-- src/builtin_return.cpp | 2 +- src/builtin_string.cpp | 11 +++++------ tests/checks/argparse.fish | 2 +- tests/checks/math.fish | 4 ++-- tests/checks/string.fish | 6 +++--- tests/functions.in | 2 +- tests/random.err | 16 ++++++++-------- 14 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/builtin.h b/src/builtin.h index e5edfc2d4..ab4865057 100644 --- a/src/builtin.h +++ b/src/builtin.h @@ -52,9 +52,10 @@ enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION }; #define BUILTIN_ERR_UNKNOWN _(L"%ls: Unknown option '%ls'\n") /// Error message for unexpected args. +#define BUILTIN_ERR_ARG_COUNT0 _(L"%ls: Expected an argument\n") #define BUILTIN_ERR_ARG_COUNT1 _(L"%ls: Expected %d args, got %d\n") #define BUILTIN_ERR_ARG_COUNT2 _(L"%ls %ls: Expected %d args, got %d\n") -#define BUILTIN_ERR_MIN_ARG_COUNT1 _(L"%ls: Expected at least %d args, got only %d\n") +#define BUILTIN_ERR_MIN_ARG_COUNT1 _(L"%ls: Expected at least %d args, got %d\n") #define BUILTIN_ERR_MAX_ARG_COUNT1 _(L"%ls: Expected at most %d args, got %d\n") /// Error message for invalid variable name. @@ -66,8 +67,8 @@ enum { COMMAND_NOT_BUILTIN, BUILTIN_REGULAR, BUILTIN_FUNCTION }; /// Error message when too many arguments are supplied to a builtin. #define BUILTIN_ERR_TOO_MANY_ARGUMENTS _(L"%ls: Too many arguments\n") -/// Error message when number expected -#define BUILTIN_ERR_NOT_NUMBER _(L"%ls: Argument '%ls' is not a number\n") +/// Error message when integer expected +#define BUILTIN_ERR_NOT_NUMBER _(L"%ls: Argument '%ls' is not a valid integer\n") /// Command that requires a subcommand was invoked without a recognized subcommand. #define BUILTIN_ERR_MISSING_SUBCMD _(L"%ls: Expected a subcommand to follow the command\n") diff --git a/src/builtin_commandline.cpp b/src/builtin_commandline.cpp index 7b623e451..9e7db57e0 100644 --- a/src/builtin_commandline.cpp +++ b/src/builtin_commandline.cpp @@ -322,7 +322,7 @@ int builtin_commandline(parser_t &parser, io_streams_t &streams, wchar_t **argv) // Check for invalid switch combinations. if ((search_mode || line_mode || cursor_mode || paging_mode) && (argc - w.woptind > 1)) { - streams.err.append_format(L"%ls: Too many arguments", argv[0]); + streams.err.append_format(BUILTIN_ERR_TOO_MANY_ARGUMENTS, argv[0]); builtin_print_error_trailer(parser, streams.err, cmd); return STATUS_INVALID_ARGS; } diff --git a/src/builtin_exit.cpp b/src/builtin_exit.cpp index 7b375f59d..853b83cf0 100644 --- a/src/builtin_exit.cpp +++ b/src/builtin_exit.cpp @@ -83,8 +83,7 @@ int builtin_exit(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } else { retval = fish_wcstoi(argv[optind]); if (errno) { - streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), cmd, - argv[optind]); + streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, argv[optind]); builtin_print_error_trailer(parser, streams.err, cmd); return STATUS_INVALID_ARGS; } diff --git a/src/builtin_functions.cpp b/src/builtin_functions.cpp index 1edd67c3b..919b1f131 100644 --- a/src/builtin_functions.cpp +++ b/src/builtin_functions.cpp @@ -287,7 +287,7 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) { // Erase, desc, query, copy and list are mutually exclusive. bool describe = opts.description ? true : false; if (describe + opts.erase + opts.list + opts.query + opts.copy > 1) { - streams.err.append_format(_(L"%ls: Invalid combination of options\n"), cmd); + streams.err.append_format(BUILTIN_ERR_COMBO, cmd); builtin_print_error_trailer(parser, streams.err, cmd); return STATUS_INVALID_ARGS; } @@ -319,8 +319,8 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (opts.report_metadata) { if (argc - optind != 1) { - streams.err.append_format(_(L"%ls: Expected exactly one function name for --details\n"), - cmd); + streams.err.append_format(BUILTIN_ERR_ARG_COUNT2, cmd, argv[optind -1], 1, + argc - optind); return STATUS_INVALID_ARGS; } diff --git a/src/builtin_history.cpp b/src/builtin_history.cpp index 877e88e1c..661dc50b0 100644 --- a/src/builtin_history.cpp +++ b/src/builtin_history.cpp @@ -161,8 +161,7 @@ static int parse_cmd_opts(history_cmd_opts_t &opts, int *optind, //!OCLINT(high case 'n': { long x = fish_wcstol(w.woptarg); if (errno) { - streams.err.append_format(_(L"%ls: max value '%ls' is not a valid number\n"), - cmd, w.woptarg); + streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, w.woptarg); return STATUS_INVALID_ARGS; } opts.max_items = static_cast(x); diff --git a/src/builtin_random.cpp b/src/builtin_random.cpp index e20f22718..d6cfbb602 100644 --- a/src/builtin_random.cpp +++ b/src/builtin_random.cpp @@ -64,7 +64,7 @@ int builtin_random(parser_t &parser, io_streams_t &streams, wchar_t **argv) { auto parse_ll = [&](const wchar_t *str) { long long ll = fish_wcstoll(str); if (errno) { - streams.err.append_format(L"%ls: %ls is not a valid integer\n", cmd, str); + streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, str); parse_error = true; } return ll; @@ -72,7 +72,7 @@ int builtin_random(parser_t &parser, io_streams_t &streams, wchar_t **argv) { auto parse_ull = [&](const wchar_t *str) { unsigned long long ull = fish_wcstoull(str); if (errno) { - streams.err.append_format(L"%ls: %ls is not a valid integer\n", cmd, str); + streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, str); parse_error = true; } return ull; diff --git a/src/builtin_read.cpp b/src/builtin_read.cpp index 04a161b4b..94e831f2d 100644 --- a/src/builtin_read.cpp +++ b/src/builtin_read.cpp @@ -127,8 +127,7 @@ static int parse_cmd_opts(read_cmd_opts_t &opts, int *optind, //!OCLINT(high nc return STATUS_INVALID_ARGS; } - streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), cmd, - w.woptarg); + streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, w.woptarg); builtin_print_error_trailer(parser, streams.err, cmd); return STATUS_INVALID_ARGS; } diff --git a/src/builtin_return.cpp b/src/builtin_return.cpp index c8873e2bc..ec6fd5739 100644 --- a/src/builtin_return.cpp +++ b/src/builtin_return.cpp @@ -82,7 +82,7 @@ int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **argv) { } else { retval = fish_wcstoi(argv[1]); if (errno) { - streams.err.append_format(_(L"%ls: Argument '%ls' must be an integer\n"), cmd, argv[1]); + streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, argv[1]); builtin_print_error_trailer(parser, streams.err, cmd); return STATUS_INVALID_ARGS; } diff --git a/src/builtin_string.cpp b/src/builtin_string.cpp index 697b288ef..3b4b17b6a 100644 --- a/src/builtin_string.cpp +++ b/src/builtin_string.cpp @@ -37,8 +37,6 @@ class parser_t; -#define STRING_ERR_MISSING _(L"%ls: Expected argument\n") - // How many bytes we read() at once. // Bash uses 128 here, so we do too (see READ_CHUNK_SIZE). // This should be about the size of a line. @@ -462,7 +460,7 @@ static int parse_opts(options_t *opts, int *optind, int n_req_args, int argc, wc int retval = fn->second(argv, parser, streams, w, opts); if (retval != STATUS_CMD_OK) return retval; } else if (opt == ':') { - string_error(streams, STRING_ERR_MISSING, cmd); + string_error(streams, BUILTIN_ERR_MISSING, cmd, argv[w.woptind - 1]); return STATUS_INVALID_ARGS; } else if (opt == '?') { string_unknown_option(parser, streams, cmd, argv[w.woptind - 1]); @@ -477,15 +475,16 @@ static int parse_opts(options_t *opts, int *optind, int n_req_args, int argc, wc // If the caller requires one or two mandatory args deal with that here. if (n_req_args) { opts->arg1 = string_get_arg_argv(optind, argv); - if (!opts->arg1) { - string_error(streams, STRING_ERR_MISSING, cmd); + if (!opts->arg1 && n_req_args == 1) { + string_error(streams, BUILTIN_ERR_ARG_COUNT0, cmd); return STATUS_INVALID_ARGS; } } if (n_req_args > 1) { opts->arg2 = string_get_arg_argv(optind, argv); if (!opts->arg2) { - string_error(streams, STRING_ERR_MISSING, cmd); + string_error(streams, BUILTIN_ERR_MIN_ARG_COUNT1, cmd, n_req_args, + !!opts->arg2 + !!opts->arg1); return STATUS_INVALID_ARGS; } } diff --git a/tests/checks/argparse.fish b/tests/checks/argparse.fish index 4c25a3fbc..62352b7ae 100644 --- a/tests/checks/argparse.fish +++ b/tests/checks/argparse.fish @@ -29,7 +29,7 @@ argparse h-help=x # --max-args and --min-args work begin argparse --name min-max --min-args 1 h/help -- - #CHECKERR: min-max: Expected at least 1 args, got only 0 + #CHECKERR: min-max: Expected at least 1 args, got 0 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 diff --git a/tests/checks/math.fish b/tests/checks/math.fish index 5a278baf4..3513f26bf 100644 --- a/tests/checks/math.fish +++ b/tests/checks/math.fish @@ -103,9 +103,9 @@ not math '2 + 2 4' # CHECKERR: '2 + 2 4' # CHECKERR: ^ not math -# CHECKERR: math: Expected at least 1 args, got only 0 +# CHECKERR: math: Expected at least 1 args, got 0 not math -s 12 -# CHECKERR: math: Expected at least 1 args, got only 0 +# CHECKERR: math: Expected at least 1 args, got 0 not math 2^999999 # CHECKERR: math: Error: Result is infinite # CHECKERR: '2^999999' diff --git a/tests/checks/string.fish b/tests/checks/string.fish index 6e2ee8ba9..f1e598434 100644 --- a/tests/checks/string.fish +++ b/tests/checks/string.fish @@ -321,16 +321,16 @@ string repeat -m-1 "foo"; and echo "exit 0" # CHECKERR: string repeat: Invalid max value '-1' string repeat -n notanumber "foo"; and echo "exit 0" -# CHECKERR: string repeat: Argument 'notanumber' is not a number +# CHECKERR: string repeat: Argument 'notanumber' is not a valid integer string repeat -m notanumber "foo"; and echo "exit 0" -# CHECKERR: string repeat: Argument 'notanumber' is not a number +# CHECKERR: string repeat: Argument 'notanumber' is not a valid integer echo "stdin" | string repeat -n1 "and arg"; and echo "exit 0" # CHECKERR: string repeat: Too many arguments string repeat -n; and echo "exit 0" -# CHECKERR: string repeat: Expected argument +# CHECKERR: string repeat: Expected argument for option -n # FIXME: Also triggers usage # string repeat -l fakearg diff --git a/tests/functions.in b/tests/functions.in index 1e63ba59d..0cd4def92 100644 --- a/tests/functions.in +++ b/tests/functions.in @@ -8,7 +8,7 @@ end # ========== # Verify that `functions --details` works as expected when given too many args. set x (functions --details f1 f2 2>&1) -if test "$x" != "functions: Expected exactly one function name for --details" +if test "$x" != "functions --details: Expected 1 args, got 2" echo "Unexpected output for 'functions --details f1 f2': $x" >&2 end diff --git a/tests/random.err b/tests/random.err index d6f1ea51d..0a9572bdf 100644 --- a/tests/random.err +++ b/tests/random.err @@ -1,14 +1,14 @@ -random: a is not a valid integer -random: 18446744073709551614 is not a valid integer +random: Argument 'a' is not a valid integer +random: Argument '18446744073709551614' is not a valid integer random: Too many arguments random: END must be greater than START -random: 18446744073709551614 is not a valid integer -random: 1d is not a valid integer -random: 1c is not a valid integer +random: Argument '18446744073709551614' is not a valid integer +random: Argument '1d' is not a valid integer +random: Argument '1c' is not a valid integer random: END must be greater than START -random: - is not a valid integer -random: -1 is not a valid integer -random: -9223372036854775807 is not a valid integer +random: Argument '-' is not a valid integer +random: Argument '-1' is not a valid integer +random: Argument '-9223372036854775807' is not a valid integer random: STEP must be a positive integer random: range contains only one possible value random: range contains only one possible value