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

@@ -1915,6 +1915,7 @@ void complete(const wcstring &cmd_with_subcmds, std::vector<completion_t> &comps
                     break;
 
                 case parse_statement_decoration_command:
+                case parse_statement_decoration_exec:
                     use_command = true;
                     use_function = false;
                     use_builtin = false;

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},

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;

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 */

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);

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;
     }
 }
 

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<long>(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++)

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 = <TOK_STRING> arguments_or_redirections_list optional_background
 
     argument_list = <empty> | argument argument_list

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