konevsa/fish
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

builtins: work on error messages

Browse Source 
- Introduce BUILTIN_ERR_COMBO2_EXCLUSIVE
- Distill generally more terse, unambiguous error descriptions.
  Remember English is not everyone's language.
- Do not capitalize sentence fragments
- Use the modality where problem input is in a %s: prefix, then
  is explained.
- Do not address the user (the "You cannot do ..." kraderism)
- Spell out 'arguments' rather than 'args' for consistency
- Mention 'function' as a scope
This commit is contained in:
Aaron Gyes 2021-11-02 03:09:38 -07:00
parent 6858abac04
commit 710639f5d6
11 changed files with 49 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
share/functions/history.fish
View File

@ -4,11 +4,11 @@
function __fish_unexpected_hist_args --no-scope-shadowing
if test -n "$search_mode"
or set -q show_time[1]
printf (_ "%ls: you cannot use any options with the %ls command\n") $cmd $hist_cmd >&2
printf (_ "%ls: %ls: subcommand takes no options\n") $cmd $hist_cmd >&2
return 0
end
if set -q argv[1]
printf (_ "%ls: %ls expected %d args, got %d\n") $cmd $hist_cmd 0 (count $argv) >&2
printf (_ "%ls: %ls: expected %d arguments; got %d") $cmd $hist_cmd 0 (count $argv) >&2
return 0
end
return 1

41
src/builtin.h
View File

@ -29,54 +29,55 @@ struct builtin_data_t {
#define DEFAULT_READ_PROMPT L"set_color green; echo -n read; set_color normal; echo -n \"> \""
/// Error message on missing argument.
#define BUILTIN_ERR_MISSING _(L"%ls: Expected argument for option %ls\n")
#define BUILTIN_ERR_MISSING _(L"%ls: %ls: option requires an argument\n")
/// Error message on missing man page.
#define BUILTIN_ERR_MISSING_HELP \
_(L"fish: Missing man page for '%ls'. Did you install the documentation?\n`help '%ls'` will " \
L"open the online version.\n")
_(L"fish: %ls: missing man page\nDocumentation may not be installed.\n`help %ls` will " \
L"show an online version\n")
/// Error message on invalid combination of options.
#define BUILTIN_ERR_COMBO _(L"%ls: Invalid combination of options\n")
#define BUILTIN_ERR_COMBO _(L"%ls: invalid option combination\n")
/// Error message on invalid combination of options.
#define BUILTIN_ERR_COMBO2 _(L"%ls: Invalid combination of options,\n%ls\n")
#define BUILTIN_ERR_COMBO2 _(L"%ls: invalid option combination, %ls\n")
#define BUILTIN_ERR_COMBO2_EXCLUSIVE _(L"%ls: %ls %ls: options cannot be used together\n")
/// Error message on multiple scope levels for variables.
#define BUILTIN_ERR_GLOCAL \
_(L"%ls: Variable scope can only be one of universal, global and local\n")
_(L"%ls: scope can be only one of: universal function global local\n")
/// Error message for specifying both export and unexport to set/read.
#define BUILTIN_ERR_EXPUNEXP _(L"%ls: Variable can't be both exported and unexported\n")
#define BUILTIN_ERR_EXPUNEXP _(L"%ls: cannot both export and unexport\n")
/// Error message for unknown switch.
#define BUILTIN_ERR_UNKNOWN _(L"%ls: Unknown option '%ls'\n")
#define BUILTIN_ERR_UNKNOWN _(L"%ls: %ls: unknown option\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 %d\n")
#define BUILTIN_ERR_MAX_ARG_COUNT1 _(L"%ls: Expected at most %d args, got %d\n")
#define BUILTIN_ERR_ARG_COUNT0 _(L"%ls: missing argument\n")
#define BUILTIN_ERR_ARG_COUNT1 _(L"%ls: expected %d arguments; got %d\n")
#define BUILTIN_ERR_ARG_COUNT2 _(L"%ls: %ls: expected %d arguments; got %d\n")
#define BUILTIN_ERR_MIN_ARG_COUNT1 _(L"%ls: expected >= %d arguments; got %d\n")
#define BUILTIN_ERR_MAX_ARG_COUNT1 _(L"%ls: expected <= %d arguments; got %d\n")
/// Error message for invalid variable name.
#define BUILTIN_ERR_VARNAME _(L"%ls: Variable name '%ls' is not valid. See `help identifiers`.\n")
#define BUILTIN_ERR_VARNAME _(L"%ls: %ls: invalid variable. See `help identifiers`\n")
/// Error message for invalid bind mode name.
#define BUILTIN_ERR_BIND_MODE _(L"%ls: mode name '%ls' is not valid. See `help identifiers`.\n")
#define BUILTIN_ERR_BIND_MODE _(L"%ls: %ls: invalid mode. See `help identifiers`\n")
/// Error message when too many arguments are supplied to a builtin.
#define BUILTIN_ERR_TOO_MANY_ARGUMENTS _(L"%ls: Too many arguments\n")
#define BUILTIN_ERR_TOO_MANY_ARGUMENTS _(L"%ls: too many arguments\n")
/// Error message when integer expected
#define BUILTIN_ERR_NOT_NUMBER _(L"%ls: Argument '%ls' is not a valid integer\n")
#define BUILTIN_ERR_NOT_NUMBER _(L"%ls: %ls: invalid 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")
#define BUILTIN_ERR_INVALID_SUBCMD _(L"%ls: Subcommand '%ls' is not valid\n")
#define BUILTIN_ERR_MISSING_SUBCMD _(L"%ls: missing subcommand\n")
#define BUILTIN_ERR_INVALID_SUBCMD _(L"%ls: %ls: invalid subcommand\n")
/// The send stuff to foreground message.
#define FG_MSG _(L"Send job %d, '%ls' to foreground\n")
#define FG_MSG _(L"Send job %d (%ls) to foreground\n")
void builtin_init();
bool builtin_exists(const wcstring &cmd);

2
src/builtin_argparse.cpp
View File

@ -110,7 +110,7 @@ static int check_for_mutually_exclusive_flags(const argparse_cmd_opts_t &opts,
std::swap(flag1, flag2);
}
streams.err.append_format(
_(L"%ls: Mutually exclusive flags '%ls' and `%ls` seen\n"),
_(L"%ls: %ls %ls: options cannot be used together\n"),
opts.name.c_str(), flag1.c_str(), flag2.c_str());
return STATUS_CMD_ERROR;
}

4
src/builtin_bind.cpp
View File

@ -257,9 +257,7 @@ bool builtin_bind_t::insert(int optind, int argc, const wchar_t **argv, io_strea
} else {
// Inserting both on the other hand makes no sense.
if (opts->have_preset && opts->have_user) {
streams.err.append_format(
BUILTIN_ERR_COMBO2, cmd,
L"--preset and --user can not be used together when inserting bindings.");
streams.err.append_format(BUILTIN_ERR_COMBO2_EXCLUSIVE, cmd, L"--preset", "--user");
return true;
}
}

14
src/builtin_function.cpp
View File

@ -81,7 +81,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
opts.named_arguments.push_back(w.woptarg);
break;
} else {
streams.err.append_format(_(L"%ls: Unexpected positional argument '%ls'"), cmd,
streams.err.append_format(_(L"%ls: %ls: unexpected positional argument"), cmd,
w.woptarg);
return STATUS_INVALID_ARGS;
}
@ -121,7 +121,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
parser.libdata().is_subshell ? parser.libdata().caller_id : 0;
if (caller_id == 0) {
streams.err.append_format(
_(L"%ls: Cannot find calling job for event handler"), cmd);
_(L"%ls: calling job for event handler not found"), cmd);
return STATUS_INVALID_ARGS;
}
e.type = event_type_t::caller_exit;
@ -132,7 +132,7 @@ static int parse_cmd_opts(function_cmd_opts_t &opts, int *optind, //!OCLINT(hig
} else {
pid_t pid = fish_wcstoi(w.woptarg);
if (errno || pid < 0) {
streams.err.append_format(_(L"%ls: Invalid process id '%ls'"), cmd,
streams.err.append_format(_(L"%ls: %ls: invalid process id"), cmd,
w.woptarg);
return STATUS_INVALID_ARGS;
}
@ -194,20 +194,20 @@ static int validate_function_name(int argc, const wchar_t *const *argv, wcstring
const wchar_t *cmd, io_streams_t &streams) {
if (argc < 2) {
// This is currently impossible but let's be paranoid.
streams.err.append_format(_(L"%ls: Expected function name"), cmd);
streams.err.append_format(_(L"%ls: function name required"), cmd);
return STATUS_INVALID_ARGS;
}
function_name = argv[1];
if (!valid_func_name(function_name)) {
streams.err.append_format(_(L"%ls: Illegal function name '%ls'"), cmd,
streams.err.append_format(_(L"%ls: %ls: invalid function name"), cmd,
function_name.c_str());
return STATUS_INVALID_ARGS;
}
if (parser_keywords_is_reserved(function_name)) {
streams.err.append_format(
_(L"%ls: The name '%ls' is reserved, and cannot be used as a function name"), cmd,
_(L"%ls: %ls: cannot use reserved keyword as function name"), cmd,
function_name.c_str());
return STATUS_INVALID_ARGS;
}
@ -259,7 +259,7 @@ maybe_t<int> builtin_function(parser_t &parser, io_streams_t &streams,
opts.named_arguments.push_back(argv[i]);
}
} else {
streams.err.append_format(_(L"%ls: Unexpected positional argument '%ls'"), cmd,
streams.err.append_format(_(L"%ls: %ls: unexpected positional argument"), cmd,
argv[optind]);
return STATUS_INVALID_ARGS;
}

12
src/builtin_history.cpp
View File

@ -68,13 +68,9 @@ static const struct woption long_options[] = {{L"prefix", no_argument, nullptr,
static bool set_hist_cmd(const wchar_t *cmd, hist_cmd_t *hist_cmd, hist_cmd_t sub_cmd,
io_streams_t &streams) {
if (*hist_cmd != HIST_UNDEF) {
wchar_t err_text[1024];
const wchar_t *subcmd_str1 = enum_to_str(*hist_cmd, hist_enum_map);
const wchar_t *subcmd_str2 = enum_to_str(sub_cmd, hist_enum_map);
std::swprintf(err_text, sizeof(err_text) / sizeof(wchar_t),
_(L"you cannot do both '%ls' and '%ls' in the same invocation"), subcmd_str1,
subcmd_str2);
streams.err.append_format(BUILTIN_ERR_COMBO2, cmd, err_text);
streams.err.append_format(BUILTIN_ERR_COMBO2_EXCLUSIVE, cmd,
enum_to_str(*hist_cmd, hist_enum_map),
enum_to_str(sub_cmd, hist_enum_map));
return false;
}
@ -86,7 +82,7 @@ static bool check_for_unexpected_hist_args(const history_cmd_opts_t &opts, const
const wcstring_list_t &args, io_streams_t &streams) {
if (opts.history_search_type_defined || opts.show_time_format || opts.null_terminate) {
const wchar_t *subcmd_str = enum_to_str(opts.hist_cmd, hist_enum_map);
streams.err.append_format(_(L"%ls: you cannot use any options with the %ls command\n"), cmd,
streams.err.append_format(_(L"%ls: %ls: subcommand takes no options\n"), cmd,
subcmd_str);
return true;
}

11
src/builtin_math.cpp
View File

@ -57,7 +57,7 @@ static int parse_cmd_opts(math_cmd_opts_t &opts, int *optind, //!OCLINT(high nc
} else {
opts.scale = fish_wcstoi(w.woptarg);
if (errno || opts.scale < 0 || opts.scale > 15) {
streams.err.append_format(_(L"%ls: '%ls' is not a valid scale value\n"),
streams.err.append_format(_(L"%ls: %ls: invalid scale value\n"),
cmd, w.woptarg);
return STATUS_INVALID_ARGS;
}
@ -72,7 +72,7 @@ static int parse_cmd_opts(math_cmd_opts_t &opts, int *optind, //!OCLINT(high nc
} else {
opts.base = fish_wcstoi(w.woptarg);
if (errno || (opts.base != 8 && opts.base != 16)) {
streams.err.append_format(_(L"%ls: '%ls' is not a valid base value\n"), cmd,
streams.err.append_format(_(L"%ls: %ls: invalid base value\n"), cmd,
w.woptarg);
return STATUS_INVALID_ARGS;
}
@ -99,8 +99,7 @@ static int parse_cmd_opts(math_cmd_opts_t &opts, int *optind, //!OCLINT(high nc
}
}
if (opts.have_scale && opts.scale != 0 && opts.base != 10) {
streams.err.append_format(
_(L"%ls: Bases other than 10 can only do scale=0 output currently\n"), cmd, w.woptarg);
streams.err.append_format(BUILTIN_ERR_COMBO2, cmd, L"non-zero scale value only valid for base 10");
return STATUS_INVALID_ARGS;
}
@ -157,7 +156,7 @@ static const wchar_t *math_get_arg(int *argidx, const wchar_t **argv, wcstring *
}
static const wchar_t *math_describe_error(const te_error_t &error) {
if (error.position == 0) return L"NO ERROR?!?";
if (error.position == 0) return L"NO ERROR";
switch (error.type) {
case TE_ERROR_NONE:
@ -286,7 +285,7 @@ maybe_t<int> builtin_math(parser_t &parser, io_streams_t &streams, const wchar_t
}
if (expression.empty()) {
streams.err.append_format(BUILTIN_ERR_MIN_ARG_COUNT1, L"math", 1, 0);
streams.err.append_format(BUILTIN_ERR_MIN_ARG_COUNT1, cmd, 1, 0);
return STATUS_CMD_ERROR;
}
return evaluate_expression(cmd, parser, streams, opts, expression);

6
src/builtin_read.cpp
View File

@ -416,14 +416,12 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg
}
if (opts.tokenize && opts.have_delimiter) {
streams.err.append_format(BUILTIN_ERR_COMBO2, cmd,
L"--delimiter and --tokenize can not be used together");
streams.err.append_format(BUILTIN_ERR_COMBO2_EXCLUSIVE, cmd, L"--delimiter", L"--tokenize");
return STATUS_INVALID_ARGS;
}
if (opts.tokenize && opts.one_line) {
streams.err.append_format(BUILTIN_ERR_COMBO2, cmd,
L"--line and --tokenize can not be used together");
streams.err.append_format(BUILTIN_ERR_COMBO2_EXCLUSIVE, cmd, L"--line", L"--tokenize");
return STATUS_INVALID_ARGS;
}

10
src/builtin_status.cpp
View File

@ -135,13 +135,9 @@ static const struct woption long_options[] = {
static bool set_status_cmd(const wchar_t *cmd, status_cmd_opts_t &opts, status_cmd_t sub_cmd,
io_streams_t &streams) {
if (opts.status_cmd != STATUS_UNDEF) {
wchar_t err_text[1024];
const wchar_t *subcmd_str1 = enum_to_str(opts.status_cmd, status_enum_map);
const wchar_t *subcmd_str2 = enum_to_str(sub_cmd, status_enum_map);
std::swprintf(err_text, sizeof(err_text) / sizeof(wchar_t),
_(L"you cannot do both '%ls' and '%ls' in the same invocation"), subcmd_str1,
subcmd_str2);
streams.err.append_format(BUILTIN_ERR_COMBO2, cmd, err_text);
streams.err.append_format(BUILTIN_ERR_COMBO2_EXCLUSIVE, cmd,
enum_to_str(opts.status_cmd, status_enum_map),
enum_to_str(sub_cmd, status_enum_map));
return false;
}

2
src/parse_execution.cpp
View File

@ -454,7 +454,7 @@ end_execution_reason_t parse_execution_context_t::run_for_statement(
auto var = parser->vars().get(for_var_name, ENV_DEFAULT);
if (env_var_t::flags_for(for_var_name.c_str()) & env_var_t::flag_read_only) {
return report_error(STATUS_INVALID_ARGS, header.var_name,
L"You cannot use read-only variable '%ls' in a for loop",
L"for: %ls: cannot overwrite read-only variable",
for_var_name.c_str());
}

4
src/path.cpp
View File

@ -392,7 +392,7 @@ static const base_directory_t &get_config_directory() {
void path_emit_config_directory_messages(env_stack_t &vars) {
const auto &data = get_data_directory();
if (!data.success()) {
maybe_issue_path_warning(L"data", _(L"Your history will not be saved."), data.used_xdg,
maybe_issue_path_warning(L"data", _(L"can not save history"), data.used_xdg,
L"XDG_DATA_HOME", data.path, data.err, vars);
}
if (data.is_remote > 0) {
@ -401,7 +401,7 @@ void path_emit_config_directory_messages(env_stack_t &vars) {
const auto &config = get_config_directory();
if (!config.success()) {
maybe_issue_path_warning(L"config", _(L"Your personal settings will not be saved."),
maybe_issue_path_warning(L"config", _(L"can not save universal variables or functions"),
config.used_xdg, L"XDG_CONFIG_HOME", config.path, config.err,
vars);
}