From 1fbf63381782b0badead61d1576ad6a1e29fc3ea Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Thu, 13 Feb 2014 10:08:04 -0800 Subject: [PATCH] Reimplement exec parsing. Instead of special-casing exec as a command, promote it to a decoration (like 'command' or 'builtin'). This makes tab completion and syntax highlighting treat exec's first argument as a command and is otherwise a nice simplification. Fixes #1300 --- complete.cpp | 1 + fish_tests.cpp | 8 ++++++++ highlight.cpp | 2 +- parse_constants.h | 4 +++- parse_execution.cpp | 7 +++---- parse_productions.cpp | 3 +++ parse_tree.cpp | 5 ++++- parse_tree.h | 4 ++-- parse_util.cpp | 17 +++++++++++------ 9 files changed, 36 insertions(+), 15 deletions(-) diff --git a/complete.cpp b/complete.cpp index 4808c4dbd..098a4ad26 100644 --- a/complete.cpp +++ b/complete.cpp @@ -1915,6 +1915,7 @@ void complete(const wcstring &cmd_with_subcmds, std::vector &comps break; case parse_statement_decoration_command: + case parse_statement_decoration_exec: use_command = true; use_function = false; use_builtin = false; diff --git a/fish_tests.cpp b/fish_tests.cpp index 1b66991c8..a9f53911e 100644 --- a/fish_tests.cpp +++ b/fish_tests.cpp @@ -140,10 +140,17 @@ static void err(const wchar_t *blah, ...) va_list va; va_start(va, blah); err_count++; + + // show errors in red + fputs("\x1b[31m", stdout); wprintf(L"Error: "); vwprintf(blah, va); va_end(va); + + // return to normal color + fputs("\x1b[0m", stdout); + wprintf(L"\n"); } @@ -2454,6 +2461,7 @@ static void test_new_parser_ll2(void) { {L"echo hello", L"echo", L"hello", parse_statement_decoration_none}, {L"command echo hello", L"echo", L"hello", parse_statement_decoration_command}, + {L"exec echo hello", L"echo", L"hello", parse_statement_decoration_exec}, {L"command command hello", L"command", L"hello", parse_statement_decoration_command}, {L"builtin command hello", L"command", L"hello", parse_statement_decoration_builtin}, {L"command --help", L"command", L"--help", parse_statement_decoration_none}, diff --git a/highlight.cpp b/highlight.cpp index dc9da1560..55db8a5a9 100644 --- a/highlight.cpp +++ b/highlight.cpp @@ -1236,7 +1236,7 @@ static bool command_is_valid(const wcstring &cmd, enum parse_statement_decoratio { /* Determine which types we check, based on the decoration */ bool builtin_ok = true, function_ok = true, abbreviation_ok = true, command_ok = true, implicit_cd_ok = true; - if (decoration == parse_statement_decoration_command) + if (decoration == parse_statement_decoration_command || decoration == parse_statement_decoration_exec) { builtin_ok = false; function_ok = false; diff --git a/parse_constants.h b/parse_constants.h index 85eabaf9b..32115c017 100644 --- a/parse_constants.h +++ b/parse_constants.h @@ -90,6 +90,7 @@ enum parse_keyword_t parse_keyword_not, parse_keyword_command, parse_keyword_builtin, + parse_keyword_exec, LAST_KEYWORD = parse_keyword_builtin }; @@ -99,7 +100,8 @@ enum parse_statement_decoration_t { parse_statement_decoration_none, parse_statement_decoration_command, - parse_statement_decoration_builtin + parse_statement_decoration_builtin, + parse_statement_decoration_exec }; /* Parse error code list */ diff --git a/parse_execution.cpp b/parse_execution.cpp index 11aa5325f..738e39730 100644 --- a/parse_execution.cpp +++ b/parse_execution.cpp @@ -161,10 +161,9 @@ enum process_type_t parse_execution_context_t::process_type_for_command(const pa /* Determine the process type, which depends on the statement decoration (command, builtin, etc) */ enum parse_statement_decoration_t decoration = tree.decoration_for_plain_statement(plain_statement); - /* Do the "exec hack" */ - if (decoration != parse_statement_decoration_command && cmd == L"exec") + if (decoration == parse_statement_decoration_exec) { - /* Either 'builtin exec' or just plain 'exec', and definitely not 'command exec'. Note we don't allow overriding exec with a function. */ + /* Always exec */ process_type = INTERNAL_EXEC; } else if (decoration == parse_statement_decoration_command) @@ -848,7 +847,7 @@ parse_execution_result_t parse_execution_context_t::populate_plain_process(job_t } wcstring path_to_external_command; - if (process_type == EXTERNAL) + if (process_type == EXTERNAL || process_type == INTERNAL_EXEC) { /* Determine the actual command. This may be an implicit cd. */ bool has_command = path_get_path(cmd, &path_to_external_command); diff --git a/parse_productions.cpp b/parse_productions.cpp index ff473268e..d559a0d93 100644 --- a/parse_productions.cpp +++ b/parse_productions.cpp @@ -357,6 +357,7 @@ PRODUCTIONS(decorated_statement) = {symbol_plain_statement}, {KEYWORD(parse_keyword_command), symbol_plain_statement}, {KEYWORD(parse_keyword_builtin), symbol_plain_statement}, + {KEYWORD(parse_keyword_exec), symbol_plain_statement} }; RESOLVE(decorated_statement) { @@ -374,6 +375,8 @@ RESOLVE(decorated_statement) return 1; case parse_keyword_builtin: return 2; + case parse_keyword_exec: + return 3; } } diff --git a/parse_tree.cpp b/parse_tree.cpp index 0db2a06d3..41ef9cf02 100644 --- a/parse_tree.cpp +++ b/parse_tree.cpp @@ -237,6 +237,8 @@ wcstring keyword_description(parse_keyword_t k) return L"command"; case parse_keyword_builtin: return L"builtin"; + case parse_keyword_exec: + return L"exec"; } return format_string(L"Unknown keyword type %ld", static_cast(k)); } @@ -1049,7 +1051,8 @@ static parse_keyword_t keyword_for_token(token_type tok, const wchar_t *tok_txt) {L"or", parse_keyword_or}, {L"not", parse_keyword_not}, {L"command", parse_keyword_command}, - {L"builtin", parse_keyword_builtin} + {L"builtin", parse_keyword_builtin}, + {L"exec", parse_keyword_exec} }; for (size_t i=0; i < sizeof keywords / sizeof *keywords; i++) diff --git a/parse_tree.h b/parse_tree.h index 255ab467f..77b29fc8a 100644 --- a/parse_tree.h +++ b/parse_tree.h @@ -247,9 +247,9 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse boolean_statement = AND statement | OR statement | NOT statement -# A decorated_statement is a command with a list of arguments_or_redirections, possibly with "builtin" or "command" +# A decorated_statement is a command with a list of arguments_or_redirections, possibly with "builtin" or "command" or "exec" - decorated_statement = plain_statement | COMMAND plain_statement | BUILTIN plain_statement + decorated_statement = plain_statement | COMMAND plain_statement | BUILTIN plain_statement | EXEC plain_statement plain_statement = arguments_or_redirections_list optional_background argument_list = | argument argument_list diff --git a/parse_util.cpp b/parse_util.cpp index 68f652489..230a328c9 100644 --- a/parse_util.cpp +++ b/parse_util.cpp @@ -1067,6 +1067,15 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, pars } else if (node.type == symbol_plain_statement) { + // In a few places below, we want to know if we are in a pipeline + const bool is_in_pipeline = node_tree.statement_is_in_pipeline(node, true /* count first */); + + // Check that we don't try to pipe through exec + if (is_in_pipeline && node_tree.decoration_for_plain_statement(node) == parse_statement_decoration_exec) + { + errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG, L"exec"); + } + wcstring command; if (node_tree.command_for_plain_statement(node, buff_src, &command)) { @@ -1077,13 +1086,9 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, pars } // Check that pipes are sound - if (! errored && parser_is_pipe_forbidden(command)) + if (! errored && parser_is_pipe_forbidden(command) && is_in_pipeline) { - // forbidden commands cannot be in a pipeline at all - if (node_tree.statement_is_in_pipeline(node, true /* count first */)) - { - errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG, command.c_str()); - } + errored = append_syntax_error(&parse_errors, node, EXEC_ERR_MSG, command.c_str()); } // Check that we don't return from outside a function