From 594b460ba2d8dca59a3bfd282397c5f33aa9da6f Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sat, 19 Dec 2015 14:45:45 -0800 Subject: Allow and/or statements to attach to the if/while header For example: if false; or true; echo hello; end will output 'hello' now. Fixes #1428 --- src/fish_indent.cpp | 3 ++- src/parse_constants.h | 2 ++ src/parse_execution.cpp | 37 ++++++++++++++++++++++++------------- src/parse_productions.cpp | 28 ++++++++++++++++++++++++++-- src/parse_tree.cpp | 2 ++ src/parse_tree.h | 12 ++++++++---- src/parse_util.cpp | 4 ++-- tests/test8.in | 4 ++-- 8 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/fish_indent.cpp b/src/fish_indent.cpp index 59efbd1e..60ed2334 100644 --- a/src/fish_indent.cpp +++ b/src/fish_indent.cpp @@ -92,7 +92,8 @@ static void prettify_node_recursive(const wcstring &source, const parse_node_tre /* Increment the indent if we are either a root job_list, or root case_item_list, or in an if or while header (#1665) */ const bool is_root_job_list = (node_type == symbol_job_list && parent_type != symbol_job_list); const bool is_root_case_item_list = (node_type == symbol_case_item_list && parent_type != symbol_case_item_list); - const bool is_if_while_header = (node_type == symbol_job && (parent_type == symbol_if_clause || parent_type == symbol_while_header)); + const bool is_if_while_header = ((node_type == symbol_job || node_type == symbol_andor_job_list) && + (parent_type == symbol_if_clause || parent_type == symbol_while_header)); if (is_root_job_list || is_root_case_item_list || is_if_while_header) { node_indent += 1; diff --git a/src/parse_constants.h b/src/parse_constants.h index ce188d65..e7c6399f 100644 --- a/src/parse_constants.h +++ b/src/parse_constants.h @@ -41,6 +41,8 @@ enum parse_token_type_t symbol_plain_statement, symbol_arguments_or_redirections_list, symbol_argument_or_redirection, + + symbol_andor_job_list, symbol_argument_list, diff --git a/src/parse_execution.cpp b/src/parse_execution.cpp index 7946d959..0857ec42 100644 --- a/src/parse_execution.cpp +++ b/src/parse_execution.cpp @@ -295,18 +295,24 @@ parse_execution_result_t parse_execution_context_t::run_if_statement(const parse result = parse_execution_cancelled; break; } - + + /* An if condition has a job and a "tail" of andor jobs, e.g. "foo ; and bar; or baz" */ assert(if_clause != NULL && else_clause != NULL); - const parse_node_t &condition = *get_child(*if_clause, 1, symbol_job); + const parse_node_t &condition_head = *get_child(*if_clause, 1, symbol_job); + const parse_node_t &condition_boolean_tail = *get_child(*if_clause, 3, symbol_andor_job_list); - /* Check the condition. We treat parse_execution_errored here as failure, in accordance with historic behavior */ - parse_execution_result_t cond_ret = run_1_job(condition, ib); - bool take_branch = (cond_ret == parse_execution_success) && proc_get_last_status() == EXIT_SUCCESS; + /* Check the condition and the tail. We treat parse_execution_errored here as failure, in accordance with historic behavior */ + parse_execution_result_t cond_ret = run_1_job(condition_head, ib); + if (cond_ret == parse_execution_success) + { + cond_ret = run_job_list(condition_boolean_tail, ib); + } + const bool take_branch = (cond_ret == parse_execution_success) && proc_get_last_status() == EXIT_SUCCESS; if (take_branch) { /* condition succeeded */ - job_list_to_execute = get_child(*if_clause, 3, symbol_job_list); + job_list_to_execute = get_child(*if_clause, 4, symbol_job_list); break; } else if (else_clause->child_count == 0) @@ -651,17 +657,22 @@ parse_execution_result_t parse_execution_context_t::run_while_statement(const pa parse_execution_result_t ret = parse_execution_success; - /* The condition and contents of the while loop, as a job and job list respectively */ - const parse_node_t &while_condition = *get_child(header, 1, symbol_job); + /* The conditions of the while loop */ + const parse_node_t &condition_head = *get_child(header, 1, symbol_job); + const parse_node_t &condition_boolean_tail = *get_child(header, 3, symbol_andor_job_list); /* Run while the condition is true */ for (;;) { /* Check the condition */ - parse_execution_result_t cond_result = this->run_1_job(while_condition, wb); - + parse_execution_result_t cond_ret = this->run_1_job(condition_head, wb); + if (cond_ret == parse_execution_success) + { + cond_ret = run_job_list(condition_boolean_tail, wb); + } + /* We only continue on successful execution and EXIT_SUCCESS */ - if (cond_result != parse_execution_success || proc_get_last_status() != EXIT_SUCCESS) + if (cond_ret != parse_execution_success || proc_get_last_status() != EXIT_SUCCESS) { break; } @@ -1460,13 +1471,13 @@ parse_execution_result_t parse_execution_context_t::run_1_job(const parse_node_t parse_execution_result_t parse_execution_context_t::run_job_list(const parse_node_t &job_list_node, const block_t *associated_block) { - assert(job_list_node.type == symbol_job_list); + assert(job_list_node.type == symbol_job_list || job_list_node.type == symbol_andor_job_list); parse_execution_result_t result = parse_execution_success; const parse_node_t *job_list = &job_list_node; while (job_list != NULL && ! should_cancel_execution(associated_block)) { - assert(job_list->type == symbol_job_list); + assert(job_list->type == symbol_job_list || job_list_node.type == symbol_andor_job_list); // Try pulling out a job const parse_node_t *job = tree.next_node_in_node_list(*job_list, symbol_job, &job_list); diff --git a/src/parse_productions.cpp b/src/parse_productions.cpp index 4d1ec227..1a44ff48 100644 --- a/src/parse_productions.cpp +++ b/src/parse_productions.cpp @@ -173,7 +173,7 @@ RESOLVE(statement) } RESOLVE_ONLY(if_statement) = {symbol_if_clause, symbol_else_clause, symbol_end_command, symbol_arguments_or_redirections_list}; -RESOLVE_ONLY(if_clause) = { KEYWORD(parse_keyword_if), symbol_job, parse_token_type_end, symbol_job_list }; +RESOLVE_ONLY(if_clause) = { KEYWORD(parse_keyword_if), symbol_job, parse_token_type_end, symbol_andor_job_list, symbol_job_list }; RESOLVE(else_clause) { @@ -216,6 +216,29 @@ RESOLVE(case_item_list) RESOLVE_ONLY(case_item) = {KEYWORD(parse_keyword_case), symbol_argument_list, parse_token_type_end, symbol_job_list}; +RESOLVE(andor_job_list) +{ + P list_end = {}; + P andor_job = {symbol_job, symbol_andor_job_list}; + P empty_line = {parse_token_type_end, symbol_andor_job_list}; + + if (token1.type == parse_token_type_end) + { + return &empty_line; + } + else if (token1.keyword == parse_keyword_and || token1.keyword == parse_keyword_or) + { + // Check that the argument to and/or is a string that's not help + // Otherwise it's either 'and --help' or a naked 'and', and not part of this list + if (token2.type == parse_token_type_string && !token2.is_help_argument) + { + return &andor_job; + } + } + // All other cases end the list + return &list_end; +} + RESOLVE(argument_list) { P empty = {}; @@ -272,7 +295,7 @@ RESOLVE(block_header) RESOLVE_ONLY(for_header) = {KEYWORD(parse_keyword_for), parse_token_type_string, KEYWORD (parse_keyword_in), symbol_argument_list, parse_token_type_end}; -RESOLVE_ONLY(while_header) = {KEYWORD(parse_keyword_while), symbol_job, parse_token_type_end}; +RESOLVE_ONLY(while_header) = {KEYWORD(parse_keyword_while), symbol_job, parse_token_type_end, symbol_andor_job_list}; RESOLVE_ONLY(begin_header) = {KEYWORD(parse_keyword_begin)}; RESOLVE_ONLY(function_header) = {KEYWORD(parse_keyword_function), symbol_argument, symbol_argument_list, parse_token_type_end}; @@ -415,6 +438,7 @@ const production_t *parse_productions::production_for_token(parse_token_type_t n TEST(begin_header) TEST(function_header) TEST(plain_statement) + TEST(andor_job_list) TEST(arguments_or_redirections_list) TEST(argument_or_redirection) TEST(argument) diff --git a/src/parse_tree.cpp b/src/parse_tree.cpp index 39c3c6d6..c1f1f80f 100644 --- a/src/parse_tree.cpp +++ b/src/parse_tree.cpp @@ -168,6 +168,8 @@ wcstring token_type_description(parse_token_type_t type) case symbol_case_item: return L"case_item"; + case symbol_andor_job_list: + return L"andor_job_list"; case symbol_argument_list: return L"argument_list"; case symbol_freestanding_argument_list: diff --git a/src/parse_tree.h b/src/parse_tree.h index 55bb9c1a..b4c4b8ba 100644 --- a/src/parse_tree.h +++ b/src/parse_tree.h @@ -222,7 +222,7 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse # A job_list is a list of jobs, separated by semicolons or newlines job_list = | - job job_list + job job_list | job_list # A job is a non-empty list of statements, separated by pipes. (Non-empty is useful for cases like if statements, where we require a command). To represent "non-empty", we require a statement, followed by a possibly empty job_continuation, and then optionally a background specifier '&' @@ -238,7 +238,7 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse # A block is a conditional, loop, or begin/end if_statement = if_clause else_clause end_command arguments_or_redirections_list - if_clause = job job_list + if_clause = job andor_job_list job_list else_clause = | else_continuation else_continuation = if_clause else_clause | @@ -254,15 +254,19 @@ bool parse_tree_from_string(const wcstring &str, parse_tree_flags_t flags, parse block_statement = block_header job_list end_command arguments_or_redirections_list block_header = for_header | while_header | function_header | begin_header for_header = FOR var_name IN argument_list - while_header = WHILE job + while_header = WHILE job andor_job_list begin_header = BEGIN # Functions take arguments, and require at least one (the name). No redirections allowed. function_header = FUNCTION argument argument_list # A boolean statement is AND or OR or NOT - boolean_statement = AND statement | OR statement | NOT statement + +# An andor_job_list is zero or more job lists, where each starts with an `and` or `or` boolean statement + andor_job_list = | + job andor_job_list | + andor_job_list # A decorated_statement is a command with a list of arguments_or_redirections, possibly with "builtin" or "command" or "exec" diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 87468e54..a528c898 100644 --- a/src/parse_util.cpp +++ b/src/parse_util.cpp @@ -759,13 +759,13 @@ static void compute_indents_recursive(const parse_node_tree_t &tree, node_offset if (node_idx > *max_visited_node_idx) *max_visited_node_idx = node_idx; - /* We could implement this by utilizing the fish grammar. But there's an easy trick instead: almost everything that wraps a job list should be indented by 1. So just find all of the job lists. One exception is switch, which wraps a case_item_list instead of a job_list. The other exception is job_list itself: a job_list is a job and a job_list, and we want that child list to be indented the same as the parent. So just find all job_lists whose parent is not a job_list, and increment their indent by 1. */ + /* We could implement this by utilizing the fish grammar. But there's an easy trick instead: almost everything that wraps a job list should be indented by 1. So just find all of the job lists. One exception is switch, which wraps a case_item_list instead of a job_list. The other exception is job_list itself: a job_list is a job and a job_list, and we want that child list to be indented the same as the parent. So just find all job_lists whose parent is not a job_list, and increment their indent by 1. We also want to treat andor_job_list like job_lists */ const parse_node_t &node = tree.at(node_idx); const parse_token_type_t node_type = node.type; /* Increment the indent if we are either a root job_list, or root case_item_list */ - const bool is_root_job_list = (node_type == symbol_job_list && parent_type != symbol_job_list); + const bool is_root_job_list = node_type != parent_type && (node_type == symbol_job_list || node_type == symbol_andor_job_list); const bool is_root_case_item_list = (node_type == symbol_case_item_list && parent_type != symbol_case_item_list); if (is_root_job_list || is_root_case_item_list) { diff --git a/tests/test8.in b/tests/test8.in index 22cb3b12..5e918fbe 100644 --- a/tests/test8.in +++ b/tests/test8.in @@ -32,8 +32,8 @@ if false ; end ; echo $status if false ; or true ; echo "success1" ; end if false ; and false ; echo "failure1" ; end while false ; and false ; or true ; echo "success2"; break ; end -while false; or begin ; false; or true; end; echo "success3"; end -if false ; else if false ; and true ; else if false ; and false ; else if false; or true; echo "success 4"; end +while false; or begin ; false; or true; end; echo "success3"; break ; end +if false ; else if false ; and true ; else if false ; and false ; else if false; or true; echo "success4"; end if false ; else if false ; and true ; else if false ; or false ; else if false; echo "failure 4"; end if false ; or true | false ; echo "failure5" ; end -- cgit v1.2.3