Make shell highlighting a toggle instead of a function parameter

Remove the ability to specify the "highlight function." The reader
always highlights via shell highlighting, or doesn't.

src/builtin_complete.cpp

@@ -390,7 +390,7 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
         if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) {
             std::vector<highlight_spec_t> colors;
             size_t len = repr.size();
-            highlight_shell_no_io(repr, colors, len, operation_context_t::globals());
+            highlight_shell(repr, colors, len, operation_context_t::globals());
             streams.out.append(str2wcstring(colorize(repr, colors)));
         } else {
             streams.out.append(repr);

src/builtin_functions.cpp

@@ -262,9 +262,8 @@ static int report_function_metadata(const wchar_t *funcname, bool verbose, io_st
             append_format(comment, L"# Defined in %ls @ line %d\n", path, line_number);
             if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) {
                 std::vector<highlight_spec_t> colors;
-                highlight_shell_no_io(
+                highlight_shell(comment, colors, comment.size(),
-                    comment, colors, comment.size(),
+                                operation_context_t{nullptr, env_stack_t::globals(), no_cancel});
-                    operation_context_t{nullptr, env_stack_t::globals(), no_cancel});
                 streams.out.append(str2wcstring(colorize(comment, colors)));
             } else {
                 streams.out.append(comment);
@@ -442,7 +441,7 @@ int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
 
                 if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) {
                     std::vector<highlight_spec_t> colors;
-                    highlight_shell_no_io(def, colors, def.size(), operation_context_t::globals());
+                    highlight_shell(def, colors, def.size(), operation_context_t::globals());
                     streams.out.append(str2wcstring(colorize(def, colors)));
                 } else {
                     streams.out.append(def);

src/builtin_read.cpp

@@ -209,7 +209,7 @@ static int read_interactive(parser_t &parser, wcstring &buff, int nchars, bool s
     reader_set_right_prompt(right_prompt);
     if (shell) {
         reader_set_complete_ok(true);
-        reader_set_highlight_function(&highlight_shell);
+        reader_set_highlight_ok(true);
         reader_set_test_function(&reader_shell_test);
     }
     // No autosuggestions or abbreviations in builtin_read.

src/fish_indent.cpp

@@ -662,7 +662,7 @@ static const char *highlight_role_to_string(highlight_role_t role) {
 static std::string make_pygments_csv(const wcstring &src) {
     const size_t len = src.size();
     std::vector<highlight_spec_t> colors;
-    highlight_shell_no_io(src, colors, src.size(), operation_context_t::globals());
+    highlight_shell(src, colors, src.size(), operation_context_t::globals());
     assert(colors.size() == len && "Colors and src should have same size");
 
     struct token_range_t {
@@ -964,8 +964,8 @@ int main(int argc, char *argv[]) {
         // Maybe colorize.
         std::vector<highlight_spec_t> colors;
         if (output_type != output_type_plain_text) {
-            highlight_shell_no_io(output_wtext, colors, output_wtext.size(),
+            highlight_shell(output_wtext, colors, output_wtext.size(),
-                                  operation_context_t::globals());
+                            operation_context_t::globals());
         }
 
         std::string colored_output;

src/fish_tests.cpp

@@ -4977,7 +4977,7 @@ static void test_highlighting() {
         do_test(expected_colors.size() == text.size());
 
         std::vector<highlight_spec_t> colors(text.size());
-        highlight_shell(text, colors, 20, operation_context_t{vars});
+        highlight_shell(text, colors, 20, operation_context_t{vars}, true /* io_ok */);
 
         if (expected_colors.size() != colors.size()) {
             err(L"Color vector has wrong size! Expected %lu, actual %lu", expected_colors.size(),

src/highlight.cpp

@@ -1308,16 +1308,8 @@ std::string colorize(const wcstring &text, const std::vector<highlight_spec_t> &
 }
 
 void highlight_shell(const wcstring &buff, std::vector<highlight_spec_t> &color, size_t pos,
-                     const operation_context_t &ctx) {
+                     const operation_context_t &ctx, bool io_ok) {
     const wcstring working_directory = ctx.vars.get_pwd_slash();
-    highlighter_t highlighter(buff, pos, ctx, working_directory, true /* can do IO */);
+    highlighter_t highlighter(buff, pos, ctx, working_directory, io_ok);
     color = highlighter.highlight();
 }
-
-void highlight_shell_no_io(const wcstring &buff, std::vector<highlight_spec_t> &color, size_t pos,
-                           const operation_context_t &ctx) {
-    const wcstring working_directory = ctx.vars.get_pwd_slash();
-    highlighter_t highlighter(buff, pos, ctx, working_directory, false /* no IO allowed */);
-    color = highlighter.highlight();
-}
-

src/highlight.h

@@ -82,13 +82,10 @@ std::string colorize(const wcstring &text, const std::vector<highlight_spec_t> &
 /// color, the next 8 bits for bg color.
 /// \param pos the cursor position. Used for quote matching, etc.
 /// \param ctx The variables and cancellation check for this operation.
+/// \param io_ok If set, allow IO which may block. This means that e.g. invalid commands may be
+/// detected.
 void highlight_shell(const wcstring &buffstr, std::vector<highlight_spec_t> &color, size_t pos,
-                     const operation_context_t &ctx);
+                     const operation_context_t &ctx, bool io_ok = false);
-
-/// 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, const operation_context_t &ctx);
 
 /// \return an RGB color for a given highlight spec.
 rgb_color_t highlight_get_color(const highlight_spec_t &highlight, bool is_background);

src/reader.cpp

@@ -516,8 +516,8 @@ class reader_data_t : public std::enable_shared_from_this<reader_data_t> {
     std::vector<int> indents;
     /// Whether tab completion is allowed.
     bool complete_ok{false};
-    /// Function for syntax highlighting.
+    /// Whether to perform syntax highlighting.
-    highlight_function_t highlight_func{nullptr};
+    bool highlight_ok{false};
     /// Function for testing if the string can be returned.
     test_function_t test_func{default_test};
     /// If this is true, exit reader even if there are running jobs. This happens if we press e.g.
@@ -2277,18 +2277,17 @@ void reader_data_t::highlight_complete(highlight_result_t result) {
 
 // Given text, bracket matching position, and whether IO is allowed,
 // return a function that performs highlighting. The function may be invoked on a background thread.
-static std::function<highlight_result_t(void)> get_highlight_performer(
+static std::function<highlight_result_t(void)> get_highlight_performer(parser_t &parser,
-    parser_t &parser, const wcstring &text, long match_highlight_pos,
+                                                                       const wcstring &text,
-    highlight_function_t highlight_func) {
+                                                                       long match_highlight_pos,
-    if (!highlight_func) return {};
+                                                                       bool io_ok) {
-
     auto vars = parser.vars().snapshot();
     unsigned generation_count = read_generation_count();
     return [=]() -> highlight_result_t {
         if (text.empty()) return {};
         operation_context_t ctx = get_bg_context(vars, generation_count);
         std::vector<highlight_spec_t> colors(text.size(), highlight_spec_t{});
-        highlight_func(text, colors, match_highlight_pos, ctx);
+        highlight_shell(text, colors, match_highlight_pos, ctx, io_ok);
         return {std::move(colors), text};
     };
 }
@@ -2302,6 +2301,8 @@ static std::function<highlight_result_t(void)> get_highlight_performer(
 /// \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.
 void reader_data_t::super_highlight_me_plenty(int match_highlight_pos_adjust, bool no_io) {
+    if (!highlight_ok) return;
+
     const editable_line_t *el = &command_line;
     assert(el != nullptr);
     long match_highlight_pos = static_cast<long>(el->position()) + match_highlight_pos_adjust;
@@ -2309,20 +2310,18 @@ void reader_data_t::super_highlight_me_plenty(int match_highlight_pos_adjust, bo
 
     sanity_check();
 
-    if (auto highlight_performer =
+    auto highlight_performer =
-            get_highlight_performer(parser(), el->text(), match_highlight_pos,
+        get_highlight_performer(parser(), el->text(), match_highlight_pos, !no_io);
-                                    no_io ? highlight_shell_no_io : highlight_func)) {
+    if (no_io) {
-        if (no_io) {
+        // Highlighting without IO, we just do it.
-            // Highlighting without IO, we just do it.
+        highlight_complete(highlight_performer());
-            highlight_complete(highlight_performer());
+    } else {
-        } else {
+        // Highlighting including I/O proceeds in the background.
-            // Highlighting including I/O proceeds in the background.
+        auto shared_this = this->shared_from_this();
-            auto shared_this = this->shared_from_this();
+        debounce_highlighting().perform(highlight_performer,
-            debounce_highlighting().perform(highlight_performer,
+                                        [shared_this](highlight_result_t result) {
-                                            [shared_this](highlight_result_t result) {
+                                            shared_this->highlight_complete(std::move(result));
-                                                shared_this->highlight_complete(std::move(result));
+                                        });
-                                            });
-        }
     }
     highlight_search();
 
@@ -2400,9 +2399,7 @@ void reader_set_expand_abbreviations(bool flag) { current_data()->expand_abbrevi
 
 void reader_set_complete_ok(bool flag) { current_data()->complete_ok = flag; }
 
-void reader_set_highlight_function(highlight_function_t func) {
+void reader_set_highlight_ok(bool flag) { current_data()->highlight_ok = flag; }
-    current_data()->highlight_func = func;
-}
 
 void reader_set_test_function(test_function_t f) { current_data()->test_func = f; }
 
@@ -2495,7 +2492,7 @@ uint64_t reader_run_count() { return run_count; }
 static int read_i(parser_t &parser) {
     reader_push(parser, history_session_id(parser.vars()));
     reader_set_complete_ok(true);
-    reader_set_highlight_function(&highlight_shell);
+    reader_set_highlight_ok(true);
     reader_set_test_function(&reader_shell_test);
     reader_set_allow_autosuggesting(true);
     reader_set_expand_abbreviations(true);

src/reader.h

@@ -216,21 +216,12 @@ void reader_pop();
 /// Mark whether tab completion is enabled.
 void reader_set_complete_ok(bool flag);
 
-/// The type of a highlight function.
+/// Mark whether syntax highlighting is enabled.
-using highlight_function_t = void (*)(const wcstring &, std::vector<highlight_spec_t> &, size_t,
+void reader_set_highlight_ok(bool flag);
-                                      const operation_context_t &ctx);
 
 /// Function type for testing if a string is valid for the reader to return.
 using test_function_t = parser_test_error_bits_t (*)(parser_t &, const wcstring &);
 
-/// Specify function for syntax highlighting. The function must take these arguments:
-///
-/// - The command to be highlighted as a null terminated array of wchar_t
-/// - The color code of each character as an array of ints
-/// - The cursor position
-/// - An array_list_t used for storing error messages
-void reader_set_highlight_function(highlight_function_t func);
-
 /// Specify function for testing if the command buffer contains syntax errors that must be corrected
 /// before returning.
 void reader_set_test_function(test_function_t func);