builtins: Use standard builtin.h error macros more

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
+/// Error message when integer expected
-#define BUILTIN_ERR_NOT_NUMBER _(L"%ls: Argument '%ls' is not a number\n")
+#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")

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

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,
+            streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, argv[optind]);
-                                      argv[optind]);
             builtin_print_error_trailer(parser, streams.err, cmd);
             return STATUS_INVALID_ARGS;
         }

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"),
+            streams.err.append_format(BUILTIN_ERR_ARG_COUNT2, cmd, argv[optind -1], 1,
-                                      cmd);
+                                      argc - optind);
             return STATUS_INVALID_ARGS;
         }
 

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"),
+                    streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, w.woptarg);
-                                              cmd, w.woptarg);
                     return STATUS_INVALID_ARGS;
                 }
                 opts.max_items = static_cast<size_t>(x);

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;

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,
+                    streams.err.append_format(BUILTIN_ERR_NOT_NUMBER, cmd, w.woptarg);
-                                              w.woptarg);
                     builtin_print_error_trailer(parser, streams.err, cmd);
                     return STATUS_INVALID_ARGS;
                 }

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

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) {
+        if (!opts->arg1 && n_req_args == 1) {
-            string_error(streams, STRING_ERR_MISSING, cmd);
+            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;
         }
     }

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

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'

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

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
 

tests/random.err

@@ -1,14 +1,14 @@
-random: a is not a valid integer
+random: Argument 'a' is not a valid integer
-random: 18446744073709551614 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: Argument '18446744073709551614' is not a valid integer
-random: 1d is not a valid integer
+random: Argument '1d' is not a valid integer
-random: 1c 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: Argument '-' is not a valid integer
-random: -1 is not a valid integer
+random: Argument '-1' is not a valid integer
-random: -9223372036854775807 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