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

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 = <empty> |
-                job job_list
+                job job_list |
                 <TOK_END> 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 = <IF> job <TOK_END> job_list
+    if_clause = <IF> job <TOK_END> andor_job_list job_list
     else_clause = <empty> |
                  <ELSE> 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 <TOK_END>
-    while_header = WHILE job <TOK_END>
+    while_header = WHILE job <TOK_END> 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 <TOK_END>
 
 # 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 = <empty> |
+                     job andor_job_list |
+                     <TOK_END> 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