diff --git a/parse_execution.cpp b/parse_execution.cpp index 38b543c46..338a513da 100644 --- a/parse_execution.cpp +++ b/parse_execution.cpp @@ -219,7 +219,7 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(const if (unmatched_wildcard != NULL) { - append_unmatched_wildcard_error(*unmatched_wildcard); + report_unmatched_wildcard_error(*unmatched_wildcard); result = parse_execution_errored; } @@ -232,7 +232,7 @@ parse_execution_result_t parse_execution_context_t::run_function_statement(const if (! error_str.empty()) { - this->append_error(header, L"%ls", error_str.c_str()); + this->report_error(header, L"%ls", error_str.c_str()); result = parse_execution_errored; } } @@ -285,8 +285,13 @@ parse_execution_result_t parse_execution_context_t::run_for_statement(const pars const parse_node_t &var_name_node = *get_child(header, 1, parse_token_type_string); const wcstring for_var_name = get_source(var_name_node); - /* Get the contents to iterate over. Here we could do something with unmatched_wildcard. However it seems nicer to not make for loops complain about this, i.e. just iterate over a potentially empty list */ - wcstring_list_t argument_list = this->determine_arguments(header, NULL); + /* Get the contents to iterate over. */ + const parse_node_t *unmatched_wildcard = NULL; + wcstring_list_t argument_list = this->determine_arguments(header, &unmatched_wildcard); + if (unmatched_wildcard != NULL) + { + return report_unmatched_wildcard_error(*unmatched_wildcard); + } parse_execution_result_t ret = parse_execution_success; @@ -303,6 +308,7 @@ parse_execution_result_t parse_execution_context_t::run_for_statement(const pars if (should_cancel_execution(fb)) { ret = parse_execution_cancelled; + break; } const wcstring &for_variable = fb->variable; @@ -353,7 +359,7 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(const p { case EXPAND_ERROR: { - result = append_error(switch_value_node, + result = report_error(switch_value_node, _(L"Could not expand string '%ls'"), switch_value.c_str()); break; @@ -362,7 +368,7 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(const p case EXPAND_WILDCARD_NO_MATCH: { /* Store the node that failed to expand */ - append_error(switch_value_node, WILDCARD_ERR_MSG, switch_value.c_str()); + report_error(switch_value_node, WILDCARD_ERR_MSG, switch_value.c_str()); ret = parse_execution_errored; break; } @@ -376,7 +382,7 @@ parse_execution_result_t parse_execution_context_t::run_switch_statement(const p if (result == parse_execution_success && switch_values_expanded.size() != 1) { - result = append_error(switch_value_node, + result = report_error(switch_value_node, _(L"switch: Expected exactly one argument, got %lu\n"), switch_values_expanded.size()); } @@ -509,8 +515,8 @@ parse_execution_result_t parse_execution_context_t::run_while_statement(const pa return ret; } -/* Appends an error to the error list. Always returns parse_execution_errored, so you can assign the result to an 'errored' variable */ -parse_execution_result_t parse_execution_context_t::append_error(const parse_node_t &node, const wchar_t *fmt, ...) +/* Reports an error. Always returns parse_execution_errored, so you can assign the result to an 'errored' variable */ +parse_execution_result_t parse_execution_context_t::report_error(const parse_node_t &node, const wchar_t *fmt, ...) { parse_error_t error; error.source_start = node.source_start; @@ -522,19 +528,29 @@ parse_execution_result_t parse_execution_context_t::append_error(const parse_nod error.text = vformat_string(fmt, va); va_end(va); - //this->errors.push_back(error); - /* Output the error */ - fprintf(stderr, "%ls\n", error.describe(this->src).c_str()); + const wcstring desc = error.describe(this->src); + if (! desc.empty()) + { + fprintf(stderr, "%ls\n", desc.c_str()); + } return parse_execution_errored; } -/* Appends an unmatched wildcard error to the error list, and returns true. */ -parse_execution_result_t parse_execution_context_t::append_unmatched_wildcard_error(const parse_node_t &unmatched_wildcard) +/* Reoports an unmatched wildcard error and returns parse_execution_errored */ +parse_execution_result_t parse_execution_context_t::report_unmatched_wildcard_error(const parse_node_t &unmatched_wildcard) { proc_set_last_status(STATUS_UNMATCHED_WILDCARD); - return append_error(unmatched_wildcard, WILDCARD_ERR_MSG, get_source(unmatched_wildcard).c_str()); + /* For reasons I cannot explain, unmatched wildcards are only reported in interactive use. */ + if (get_is_interactive()) + { + return report_error(unmatched_wildcard, WILDCARD_ERR_MSG, get_source(unmatched_wildcard).c_str()); + } + else + { + return parse_execution_errored; + } } /* Handle the case of command not found */ @@ -565,7 +581,7 @@ void parse_execution_context_t::handle_command_not_found(const wcstring &cmd_str ellipsis_str = L"..."; /* Looks like a command */ - this->append_error(statement_node, + this->report_error(statement_node, _(L"Unknown command '%ls'. Did you mean to run %ls with a modified environment? Try 'env %ls=%ls %ls%ls'. See the help section on the set command by typing 'help set'."), cmd, argument.c_str(), @@ -576,7 +592,7 @@ void parse_execution_context_t::handle_command_not_found(const wcstring &cmd_str } else { - this->append_error(statement_node, + this->report_error(statement_node, COMMAND_ASSIGN_ERR_MSG, cmd, name_str.c_str(), @@ -590,7 +606,7 @@ void parse_execution_context_t::handle_command_not_found(const wcstring &cmd_str const wchar_t *val = val_wstr.missing() ? NULL : val_wstr.c_str(); if (val) { - this->append_error(statement_node, + this->report_error(statement_node, _(L"Variables may not be used as commands. Instead, define a function like 'function %ls; %ls $argv; end' or use the eval builtin instead, like 'eval %ls'. See the help section for the function command by typing 'help function'."), cmd+1, val, @@ -599,7 +615,7 @@ void parse_execution_context_t::handle_command_not_found(const wcstring &cmd_str } else { - this->append_error(statement_node, + this->report_error(statement_node, _(L"Variables may not be used as commands. Instead, define a function or use the eval builtin instead, like 'eval %ls'. See the help section for the function command by typing 'help function'."), cmd, cmd); @@ -607,14 +623,14 @@ void parse_execution_context_t::handle_command_not_found(const wcstring &cmd_str } else if (wcschr(cmd, L'$')) { - this->append_error(statement_node, + this->report_error(statement_node, _(L"Commands may not contain variables. Use the eval builtin instead, like 'eval %ls'. See the help section for the eval command by typing 'help eval'."), cmd, cmd); } else if (err_code!=ENOENT) { - this->append_error(statement_node, + this->report_error(statement_node, _(L"The file '%ls' is not executable by this user"), cmd?cmd:L"UNKNOWN"); } @@ -631,7 +647,7 @@ void parse_execution_context_t::handle_command_not_found(const wcstring &cmd_str event_fire_generic(L"fish_command_not_found", &event_args); /* Here we want to report an error (so it shows a backtrace), but with no text */ - this->append_error(statement_node, L""); + this->report_error(statement_node, L""); } /* Set the last proc status appropriately */ @@ -657,7 +673,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(job_t bool expanded = expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES); if (! expanded) { - append_error(statement, ILLEGAL_CMD_ERR_MSG, cmd.c_str()); + report_error(statement, ILLEGAL_CMD_ERR_MSG, cmd.c_str()); return parse_execution_errored; } @@ -718,7 +734,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(job_t if (unmatched_wildcard != NULL) { job_set_flag(job, JOB_WILDCARD_ERROR, 1); - append_unmatched_wildcard_error(*unmatched_wildcard); + report_unmatched_wildcard_error(*unmatched_wildcard); return parse_execution_errored; } @@ -770,7 +786,7 @@ wcstring_list_t parse_execution_context_t::determine_arguments(const parse_node_ { case EXPAND_ERROR: { - this->append_error(arg_node, + this->report_error(arg_node, _(L"Could not expand string '%ls'"), arg_str.c_str()); break; @@ -838,7 +854,7 @@ bool parse_execution_context_t::determine_io_chain(const parse_node_t &statement if (! target_expanded || target.empty()) { /* Should improve this error message */ - errored = append_error(redirect_node, + errored = report_error(redirect_node, _(L"Invalid redirection target: %ls"), target.c_str()); } @@ -862,7 +878,7 @@ bool parse_execution_context_t::determine_io_chain(const parse_node_t &statement int old_fd = fish_wcstoi(target.c_str(), &end, 10); if (old_fd < 0 || errno || *end) { - errored = append_error(redirect_node, + errored = report_error(redirect_node, _(L"Requested redirection to something that is not a file descriptor %ls"), target.c_str()); } diff --git a/parse_execution.h b/parse_execution.h index d22da442d..feeb7404a 100644 --- a/parse_execution.h +++ b/parse_execution.h @@ -60,9 +60,9 @@ class parse_execution_context_t execution_cancellation_reason_t cancellation_reason(const block_t *block) const; /* Report an error. Always returns true. */ - parse_execution_result_t append_error(const parse_node_t &node, const wchar_t *fmt, ...); + parse_execution_result_t report_error(const parse_node_t &node, const wchar_t *fmt, ...); /* Wildcard error helper */ - parse_execution_result_t append_unmatched_wildcard_error(const parse_node_t &unmatched_wildcard); + parse_execution_result_t report_unmatched_wildcard_error(const parse_node_t &unmatched_wildcard); void handle_command_not_found(const wcstring &cmd, const parse_node_t &statement_node, int err_code); diff --git a/parse_tree.cpp b/parse_tree.cpp index b624cf375..222af8af9 100644 --- a/parse_tree.cpp +++ b/parse_tree.cpp @@ -1,6 +1,7 @@ #include "parse_productions.h" #include "tokenizer.h" #include "fallback.h" +#include "proc.h" #include #include @@ -41,37 +42,47 @@ wcstring parse_error_t::describe(const wcstring &src, bool skip_caret) const assert(line_end >= line_start); assert(source_start >= line_start); - // Append the line of text. - result.push_back(L'\n'); - result.append(src, line_start, line_end - line_start); + // Don't include the caret and line if we're interactive this is the first line, because then it's obvious + bool skip_caret = (get_is_interactive() && source_start == 0); - // Append the caret line. The input source may include tabs; for that reason we construct a "caret line" that has tabs in corresponding positions - wcstring caret_space_line; - caret_space_line.reserve(source_start - line_start); - for (size_t i=line_start; i < source_start; i++) + if (! skip_caret) { - wchar_t wc = src.at(i); - if (wc == L'\t') + // Append the line of text. + if (! result.empty()) { - caret_space_line.push_back(L'\t'); + result.push_back(L'\n'); } - else if (wc == L'\n') + result.append(src, line_start, line_end - line_start); + + + // Append the caret line. The input source may include tabs; for that reason we construct a "caret line" that has tabs in corresponding positions + wcstring caret_space_line; + caret_space_line.reserve(source_start - line_start); + for (size_t i=line_start; i < source_start; i++) { - /* It's possible that the source_start points at a newline itself. In that case, pretend it's a space. We only expect this to be at the end of the string. */ - caret_space_line.push_back(L' '); - } - else - { - int width = fish_wcwidth(wc); - if (width > 0) + wchar_t wc = src.at(i); + if (wc == L'\t') { - caret_space_line.append(static_cast(width), L' '); + caret_space_line.push_back(L'\t'); + } + else if (wc == L'\n') + { + /* It's possible that the source_start points at a newline itself. In that case, pretend it's a space. We only expect this to be at the end of the string. */ + caret_space_line.push_back(L' '); + } + else + { + int width = fish_wcwidth(wc); + if (width > 0) + { + caret_space_line.append(static_cast(width), L' '); + } } } + result.push_back(L'\n'); + result.append(caret_space_line); + result.push_back(L'^'); } - result.push_back(L'\n'); - result.append(caret_space_line); - result.push_back(L'^'); } return result; } @@ -249,6 +260,36 @@ wcstring parse_token_t::describe() const return result; } +/** A string description appropriate for presentation to the user */ +wcstring parse_token_t::user_presentable_description() const +{ + if (keyword != parse_keyword_none) + { + return format_string(L"keyword %ls", keyword_description(keyword).c_str()); + } + + switch (type) + { + /* Hackish. We only support the */ + case parse_token_type_string: + return L"a string"; + + case parse_token_type_pipe: + return L"a pipe"; + + case parse_token_type_redirection: + return L"a redirection"; + + case parse_token_type_background: + return L"a '&'"; + + case parse_token_type_end: + return L"statement terminator"; + + default: + return format_string(L"a %ls", this->describe().c_str()); + } +} /* Convert from tokenizer_t's token type to a parse_token_t type */ static inline parse_token_type_t parse_token_type_from_tokenizer_token(enum token_type tokenizer_token_type) @@ -382,6 +423,25 @@ struct parse_stack_element_t } return result; } + + /* Returns a name that we can show to the user, e.g. "a command" */ + wcstring user_presentable_description(void) const + { + if (keyword != parse_keyword_none) + { + return format_string(L"keyword %ls", keyword_description(keyword).c_str()); + } + + switch (type) + { + /* Hackish, the only one we support now */ + case symbol_statement: + return L"a command"; + + default: + return format_string(L"a %ls", this->describe().c_str()); + } + } }; /* The parser itself, private implementation of class parse_t. This is a hand-coded table-driven LL parser. Most hand-coded LL parsers are recursive descent, but recursive descent parsers are difficult to "pause", unlike table-driven parsers. */ @@ -407,6 +467,7 @@ class parse_ll_t void parse_error(const wchar_t *expected, parse_token_t token); void parse_error(parse_token_t token, parse_error_code_t code, const wchar_t *format, ...); + void parse_error_failed_production(struct parse_stack_element_t &elem, parse_token_t token); void parse_error_unbalancing_token(parse_token_t token); void append_error_callout(wcstring &error_message, parse_token_t token); @@ -671,6 +732,12 @@ void parse_ll_t::parse_error_unbalancing_token(parse_token_t token) } } +// This is a 'generic' parse error when we can't match the top of the stack element +void parse_ll_t::parse_error_failed_production(struct parse_stack_element_t &stack_elem, parse_token_t token) +{ + this->parse_error(token, parse_error_generic, L"Expected %ls, but instead found %ls", stack_elem.user_presentable_description().c_str(), token.user_presentable_description().c_str()); +} + void parse_ll_t::report_tokenizer_error(parse_token_t token, const wchar_t *tok_error) { assert(tok_error != NULL); @@ -861,7 +928,7 @@ void parse_ll_t::accept_tokens(parse_token_t token1, parse_token_t token2) { if (should_generate_error_messages) { - this->parse_error(token1, parse_error_generic, L"Unable to produce a '%ls' from input '%ls'", stack_elem.describe().c_str(), token1.describe().c_str()); + parse_error_failed_production(stack_elem, token1); } else { diff --git a/parse_tree.h b/parse_tree.h index aa7a0d984..d8874b119 100644 --- a/parse_tree.h +++ b/parse_tree.h @@ -51,6 +51,7 @@ struct parse_token_t size_t source_length; wcstring describe() const; + wcstring user_presentable_description() const; };