Equip syntax highlighting with a variant that does no disk I/O. Invoke

it after expanding an abbreviation, so that the expanded abbreviation
appears with (some) syntax highlighting.

3 files changed, 86 insertions, 29 deletions

diff --git a/highlight.cpp b/highlight.cpp
index d6db4c46..c1c24dd8 100644
--- a/highlight.cpp
+++ b/highlight.cpp
@@ -911,6 +911,9 @@ class highlighter_t
 
     /* Environment variables. Again, a reference member variable! */
     const env_vars_snapshot_t &vars;
+    
+    /* Whether it's OK to do I/O */
+    const bool io_ok;
 
     /* Working directory */
     const wcstring working_directory;
@@ -943,7 +946,7 @@ class highlighter_t
 public:
 
     /* Constructor */
-    highlighter_t(const wcstring &str, size_t pos, const env_vars_snapshot_t &ev, const wcstring &wd) : buff(str), cursor_pos(pos), vars(ev), working_directory(wd), color_array(str.size())
+    highlighter_t(const wcstring &str, size_t pos, const env_vars_snapshot_t &ev, const wcstring &wd, bool can_do_io) : buff(str), cursor_pos(pos), vars(ev), io_ok(can_do_io), working_directory(wd), color_array(str.size())
     {
         /* Parse the tree */
         this->parse_tree.clear();
@@ -1010,7 +1013,7 @@ void highlighter_t::color_argument(const parse_node_t &node)
         }
 
         /* Highlight it recursively. */
-        highlighter_t cmdsub_highlighter(cmdsub_contents, cursor_subpos, this->vars, this->working_directory);
+        highlighter_t cmdsub_highlighter(cmdsub_contents, cursor_subpos, this->vars, this->working_directory, this->io_ok);
         const color_array_t &subcolors = cmdsub_highlighter.highlight();
 
         /* Copy out the subcolors back into our array */
@@ -1046,13 +1049,16 @@ void highlighter_t::color_arguments(const parse_node_t &list_node)
 {
     /* Hack: determine whether the parent is the cd command, so we can show errors for non-directories */
     bool cmd_is_cd = false;
-    const parse_node_t *parent = this->parse_tree.get_parent(list_node, symbol_plain_statement);
-    if (parent != NULL)
+    if (this->io_ok)
     {
-        wcstring cmd_str;
-        if (plain_statement_get_expanded_command(this->buff, this->parse_tree, *parent, &cmd_str))
+        const parse_node_t *parent = this->parse_tree.get_parent(list_node, symbol_plain_statement);
+        if (parent != NULL)
         {
-            cmd_is_cd = (cmd_str == L"cd");
+            wcstring cmd_str;
+            if (plain_statement_get_expanded_command(this->buff, this->parse_tree, *parent, &cmd_str))
+            {
+                cmd_is_cd = (cmd_str == L"cd");
+            }
         }
     }
 
@@ -1072,7 +1078,7 @@ void highlighter_t::color_arguments(const parse_node_t &list_node)
             if (expand_one(param, EXPAND_SKIP_CMDSUBST))
             {
                 bool is_help = string_prefixes_string(param, L"--help") || string_prefixes_string(param, L"-h");
-                if (!is_help && ! is_potential_cd_path(param, working_directory, PATH_EXPAND_TILDE, NULL))
+                if (! is_help && this->io_ok && ! is_potential_cd_path(param, working_directory, PATH_EXPAND_TILDE, NULL))
                 {
                     this->color_node(*child, highlight_spec_error);
                 }
@@ -1102,14 +1108,21 @@ void highlighter_t::color_redirection(const parse_node_t &redirection_node)
         if (parse_util_locate_cmdsubst(target.c_str(), NULL, NULL, true) != 0)
         {
             if (redirection_target != NULL)
+            {
                 this->color_argument(*redirection_target);
+            }
         }
         else
         {
             /* No command substitution, so we can highlight the target file or fd. For example, disallow redirections into a non-existent directory */
             bool target_is_valid = true;
 
-            if (! expand_one(target, EXPAND_SKIP_CMDSUBST))
+            if (! this->io_ok)
+            {
+                /* I/O is disallowed, so we don't have much hope of catching anything but gross errors. Assume it's valid. */
+                target_is_valid = true;
+            }
+            else if (! expand_one(target, EXPAND_SKIP_CMDSUBST))
             {
                 /* Could not be expanded */
                 target_is_valid = false;
@@ -1280,7 +1293,11 @@ static bool command_is_valid(const wcstring &cmd, enum parse_statement_decoratio
 
 const highlighter_t::color_array_t & highlighter_t::highlight()
 {
-    ASSERT_IS_BACKGROUND_THREAD();
+    // If we are doing I/O, we must be in a background thread
+    if (io_ok)
+    {
+        ASSERT_IS_BACKGROUND_THREAD();
+    }
 
     const size_t length = buff.size();
     assert(this->buff.size() == this->color_array.size());
@@ -1307,7 +1324,7 @@ const highlighter_t::color_array_t & highlighter_t::highlight()
 
         switch (node.type)
         {
-                // Color direct string descendants, e.g. 'for' and 'in'.
+            // Color direct string descendants, e.g. 'for' and 'in'.
             case symbol_while_header:
             case symbol_begin_header:
             case symbol_function_header:
@@ -1346,7 +1363,7 @@ const highlighter_t::color_array_t & highlighter_t::highlight()
 
             case symbol_plain_statement:
             {
-                // Get the decoration from the parent
+                /* Get the decoration from the parent */
                 enum parse_statement_decoration_t decoration = parse_tree.decoration_for_plain_statement(node);
 
                 /* Color the command */
@@ -1354,13 +1371,22 @@ const highlighter_t::color_array_t & highlighter_t::highlight()
                 if (cmd_node != NULL && cmd_node->has_source())
                 {
                     bool is_valid_cmd = false;
-                    wcstring cmd(buff, cmd_node->source_start, cmd_node->source_length);
-
-                    /* Try expanding it. If we cannot, it's an error. */
-                    bool expanded = expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS);
-                    if (expanded && ! has_expand_reserved(cmd))
+                    if (! this->io_ok)
+                    {
+                        /* We cannot check if the command is invalid, so just assume it's valid */
+                        is_valid_cmd = true;
+                    }
+                    else
                     {
-                        is_valid_cmd = command_is_valid(cmd, decoration, working_directory, vars);
+                        /* Check to see if the command is valid */
+                        wcstring cmd(buff, cmd_node->source_start, cmd_node->source_length);
+
+                        /* Try expanding it. If we cannot, it's an error. */
+                        bool expanded = expand_one(cmd, EXPAND_SKIP_CMDSUBST | EXPAND_SKIP_VARIABLES | EXPAND_SKIP_JOBS);
+                        if (expanded && ! has_expand_reserved(cmd))
+                        {
+                            is_valid_cmd = command_is_valid(cmd, decoration, working_directory, vars);
+                        }
                     }
                     this->color_node(*cmd_node, is_valid_cmd ? highlight_spec_command : highlight_spec_error);
                 }
@@ -1398,7 +1424,7 @@ const highlighter_t::color_array_t & highlighter_t::highlight()
         }
     }
 
-    if (this->cursor_pos <= this->buff.size())
+    if (this->io_ok && this->cursor_pos <= this->buff.size())
     {
         /* If the cursor is over an argument, and that argument is a valid path, underline it */
         for (parse_node_tree_t::const_iterator iter = parse_tree.begin(); iter != parse_tree.end(); ++iter)
@@ -1438,7 +1464,17 @@ void highlight_shell(const wcstring &buff, std::vector<highlight_spec_t> &color,
     const wcstring working_directory = env_get_pwd_slash();
 
     /* Highlight it! */
-    highlighter_t highlighter(buff, pos, vars, working_directory);
+    highlighter_t highlighter(buff, pos, vars, working_directory, true /* can do IO */);
+    color = highlighter.highlight();
+}
+
+void highlight_shell_no_io(const wcstring &buff, std::vector<highlight_spec_t> &color, size_t pos, wcstring_list_t *error, const env_vars_snapshot_t &vars)
+{
+    /* Do something sucky and get the current working directory on this background thread. This should really be passed in. */
+    const wcstring working_directory = env_get_pwd_slash();
+
+    /* Highlight it! */
+    highlighter_t highlighter(buff, pos, vars, working_directory, false /* no IO allowed */);
     color = highlighter.highlight();
 }
 
diff --git a/highlight.h b/highlight.h
index d411ae36..b8211a81 100644
--- a/highlight.h
+++ b/highlight.h
@@ -77,6 +77,11 @@ struct file_detection_context_t;
 void highlight_shell(const wcstring &buffstr, std::vector<highlight_spec_t> &color, size_t pos, wcstring_list_t *error, const env_vars_snapshot_t &vars);
 
 /**
+   Perform a non-blocking shell highlighting. The function will not do any I/O that may block. As a result, invalid commands may not be detected, etc.
+*/
+void highlight_shell_no_io(const wcstring &buffstr, std::vector<highlight_spec_t> &color, size_t pos, wcstring_list_t *error, const env_vars_snapshot_t &vars);
+
+/**
    Perform syntax highlighting for the text in buff. Matching quotes and paranthesis are highlighted. The result is
    stored in the color array as a color_code from the HIGHLIGHT_ enum
    for each character in buff.
diff --git a/reader.cpp b/reader.cpp
index 7bbac43a..3bf97080 100644
--- a/reader.cpp
+++ b/reader.cpp
@@ -429,7 +429,7 @@ static struct termios terminal_mode_on_startup;
 static struct termios terminal_mode_for_executing_programs;
 
 
-static void reader_super_highlight_me_plenty(int highlight_pos_adjust = 0);
+static void reader_super_highlight_me_plenty(int highlight_pos_adjust = 0, bool no_io = false);
 
 /**
    Variable to keep track of forced exits - see \c reader_exit_forced();
@@ -781,7 +781,7 @@ bool reader_expand_abbreviation_in_command(const wcstring &cmdline, size_t curso
     return result;
 }
 
-/* Expand abbreviations at the current cursor position, minus the given  cursor backtrack. This may change the command line but does NOT repaint it. This is to allow the caller to coalesce repaints. */
+/* Expand abbreviations at the current cursor position, minus the given cursor backtrack. This may change the command line but does NOT repaint it. This is to allow the caller to coalesce repaints. */
 bool reader_data_t::expand_abbreviation_as_necessary(size_t cursor_backtrack)
 {
     bool result = false;
@@ -2654,7 +2654,7 @@ public:
     {
     }
 
-    int threaded_highlight()
+    int perform_highlight()
     {
         if (generation_count != s_generation_count)
         {
@@ -2710,7 +2710,7 @@ static void highlight_complete(background_highlight_context_t *ctx, int result)
 
 static int threaded_highlight(background_highlight_context_t *ctx)
 {
-    return ctx->threaded_highlight();
+    return ctx->perform_highlight();
 }
 
 
@@ -2722,17 +2722,30 @@ static int threaded_highlight(background_highlight_context_t *ctx)
 
    \param match_highlight_pos_adjust the adjustment to the position to use for bracket matching. This is added to the current cursor position and may be negative.
    \param error if non-null, any possible errors in the buffer are further descibed by the strings inserted into the specified arraylist
+    \param no_io if true, do a highlight that does not perform I/O, synchronously. If false, perform an asynchronous highlight in the background, which may perform disk I/O.
 */
-static void reader_super_highlight_me_plenty(int match_highlight_pos_adjust)
+static void reader_super_highlight_me_plenty(int match_highlight_pos_adjust, bool no_io)
 {
     const editable_line_t *el = &data->command_line;
     long match_highlight_pos = (long)el->position + match_highlight_pos_adjust;
     assert(match_highlight_pos >= 0);
     
     reader_sanity_check();
-
-    background_highlight_context_t *ctx = new background_highlight_context_t(el->text, match_highlight_pos, data->highlight_function);
-    iothread_perform(threaded_highlight, highlight_complete, ctx);
+    
+    highlight_function_t highlight_func = no_io ? highlight_shell_no_io : data->highlight_function;
+    background_highlight_context_t *ctx = new background_highlight_context_t(el->text, match_highlight_pos, highlight_func);
+    if (no_io)
+    {
+        // Highlighting without IO, we just do it
+        // Note that highlight_complete deletes ctx.
+        int result = ctx->perform_highlight();
+        highlight_complete(ctx, result);
+    }
+    else
+    {
+        // Highlighting including I/O proceeds in the background
+        iothread_perform(threaded_highlight, highlight_complete, ctx);
+    }
     highlight_search();
 
     /* Here's a hack. Check to see if our autosuggestion still applies; if so, don't recompute it. Since the autosuggestion computation is asynchronous, this avoids "flashing" as you type into the autosuggestion. */
@@ -3452,8 +3465,11 @@ const wchar_t *reader_readline(void)
                     if (abbreviation_expanded)
                     {
                         /* It's our reponsibility to rehighlight and repaint. But everything we do below triggers a repaint. */
-                        reader_super_highlight_me_plenty();
                         command_test_result = data->test_func(el->text.c_str());
+                        
+                        /* If the command is OK, then we're going to execute it. We still want to do syntax highlighting, but a synchronous variant that performs no I/O, so as not to block the user */
+                        bool skip_io = (command_test_result == 0);
+                        reader_super_highlight_me_plenty(0, skip_io);
                     }
                 }