From 6a0bb7d6deea3c92293f5ac6c360cec93732e0c8 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Wed, 14 Sep 2022 21:26:33 +0200 Subject: [PATCH] reader: when updating commandline, also update rendered highlighting Whenever the command line changes, we redraw it with the previously computed syntax highlighting. At the same time we start recomputing highlighting in a background thread. On some systems, the highlighting computation is slow, so the stale syntax highlighting is visible. The stale highlighting was computed for an old commandline. When the user had inserted or deleted some characters in the middle, then the highlighting is wrong for the characters to the right. This is because the characters to the right have shifted but the highlighting hasn't. Fix this by also shifting highlighting. This means that text that was alrady highlighted will use the same highlighting until a new one is computed. Newly inserted text uses the color left of the cursor. This is implemented by giving editable_line_t ownership of the highlighting. It is able to perfectly sync text and highlighting; they will invariably have the same length. Fixes #9180 --- src/fish_tests.cpp | 3 ++- src/reader.cpp | 54 ++++++++++++++++++++++------------------------ src/reader.h | 13 +++++++++-- 3 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index 6085a67d8..fa6cc3934 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2415,7 +2415,8 @@ static void test_abbreviations() { const environment_t &vars) -> maybe_t { if (auto edit = reader_expand_abbreviation_in_command(cmdline, cursor_pos, vars)) { wcstring cmdline_expanded = cmdline; - apply_edit(&cmdline_expanded, *edit); + std::vector colors{cmdline_expanded.size()}; + apply_edit(&cmdline_expanded, &colors, *edit); return cmdline_expanded; } return none_t(); diff --git a/src/reader.cpp b/src/reader.cpp index 5af211a4b..fbc0e4bda 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -203,8 +203,15 @@ void undo_history_t::clear() { may_coalesce = false; } -void apply_edit(wcstring *target, const edit_t &edit) { - target->replace(edit.offset, edit.length, edit.replacement); +void apply_edit(wcstring *target, std::vector *colors, const edit_t &edit) { + size_t offset = edit.offset; + target->replace(offset, edit.length, edit.replacement); + + // Now do the same to highlighting. + auto it = colors->begin() + offset; + colors->erase(it, it + edit.length); + highlight_spec_t last_color = offset < 1 ? highlight_spec_t{} : colors->at(offset - 1); + colors->insert(it, edit.replacement.size(), last_color); } /// Returns the number of characters left of the cursor that are removed by the @@ -237,7 +244,7 @@ bool editable_line_t::undo() { edit_t inverse = edit_t(edit.offset, edit.replacement.size(), L""); inverse.replacement = edit.old; size_t old_position = edit.cursor_position_before_edit; - apply_edit(&text_, inverse); + apply_edit(&text_, &colors_, inverse); set_position(old_position); did_undo = true; } @@ -250,7 +257,7 @@ bool editable_line_t::undo() { void editable_line_t::clear() { undo_history_.clear(); if (empty()) return; - text_ = L""; + apply_edit(&text_, &colors_, edit_t(0, text_.length(), L"")); set_position(0); } @@ -261,7 +268,7 @@ void editable_line_t::push_edit(edit_t edit, bool allow_coalesce) { assert(edit.offset == position()); edit_t &last_edit = undo_history_.edits.back(); last_edit.replacement.append(edit.replacement); - apply_edit(&text_, edit); + apply_edit(&text_, &colors_, edit); set_position(position() + edit.replacement.size()); assert(undo_history_.may_coalesce); @@ -285,7 +292,7 @@ void editable_line_t::push_edit(edit_t edit, bool allow_coalesce) { } edit.cursor_position_before_edit = position(); edit.old = text_.substr(edit.offset, edit.length); - apply_edit(&text_, edit); + apply_edit(&text_, &colors_, edit); set_position(cursor_position_after_edit(edit)); assert(undo_history_.edits_applied == undo_history_.edits.size()); undo_history_.may_coalesce = @@ -306,7 +313,7 @@ bool editable_line_t::redo() { } last_group_id = edit.group_id; undo_history_.edits_applied++; - apply_edit(&text_, edit); + apply_edit(&text_, &colors_, edit); set_position(cursor_position_after_edit(edit)); did_redo = true; } @@ -790,12 +797,12 @@ class reader_data_t : public std::enable_shared_from_this { /// Generate a new layout data from the current state of the world. /// If \p mcolors has a value, then apply it; otherwise extend existing colors. - layout_data_t make_layout_data(maybe_t mcolors = none()) const; + layout_data_t make_layout_data() const; /// Generate a new layout data from the current state of the world, and paint with it. /// If \p mcolors has a value, then apply it; otherwise extend existing colors. - void layout_and_repaint(const wchar_t *reason, maybe_t mcolors = none()) { - this->rendered_layout = make_layout_data(std::move(mcolors)); + void layout_and_repaint(const wchar_t *reason) { + this->rendered_layout = make_layout_data(); paint_layout(reason); } @@ -1138,17 +1145,12 @@ bool reader_data_t::is_repaint_needed(const std::vector *mcolo check(pager.rendering_needs_update(current_page_rendering), L"pager"); } -layout_data_t reader_data_t::make_layout_data(maybe_t mcolors) const { +layout_data_t reader_data_t::make_layout_data() const { layout_data_t result{}; bool focused_on_pager = active_edit_line() == &pager.search_field_line; result.text = command_line.text(); - - if (mcolors.has_value()) { - result.colors = mcolors.acquire(); - } else { - result.colors = rendered_layout.colors; - } - + result.colors = command_line.colors(); + assert(result.text.size() == result.colors.size()); result.position = focused_on_pager ? pager.cursor_position() : command_line.position(); result.selection = selection; result.focused_on_pager = (active_edit_line() == &pager.search_field_line); @@ -1157,12 +1159,6 @@ layout_data_t reader_data_t::make_layout_data(maybe_t mcolors) result.left_prompt_buff = left_prompt_buff; result.mode_prompt_buff = mode_prompt_buff; result.right_prompt_buff = right_prompt_buff; - - // Ensure our color list has the same length as the command line, by extending it with the last - // color. This typically reduces redraws; e.g. if the user continues types into an argument, we - // guess it's still an argument, while the highlighting proceeds in the background. - highlight_spec_t last_color = result.colors.empty() ? highlight_spec_t{} : result.colors.back(); - result.colors.resize(result.text.size(), last_color); return result; } @@ -1704,7 +1700,8 @@ void reader_data_t::delete_char(bool backward) { /// Returns true if the string changed. void reader_data_t::insert_string(editable_line_t *el, const wcstring &str) { if (!str.empty()) { - el->push_edit(edit_t(el->position(), 0, str), !history_search.active()); + el->push_edit(edit_t(el->position(), 0, str), + !history_search.active() /* allow_coalesce */); } if (el == &command_line) { @@ -1714,8 +1711,8 @@ void reader_data_t::insert_string(editable_line_t *el, const wcstring &str) { maybe_refilter_pager(el); } -void reader_data_t::push_edit(editable_line_t *el, edit_t edit) { - el->push_edit(std::move(edit), false); +void reader_data_t::push_edit(editable_line_t &el, edit_t edit) { + el->push_edit(std::move(edit), false /* allow_coalesce */); maybe_refilter_pager(el); } @@ -2689,7 +2686,8 @@ void reader_data_t::highlight_complete(highlight_result_t result) { if (result.text == command_line.text()) { assert(result.colors.size() == command_line.size()); if (this->is_repaint_needed(&result.colors)) { - this->layout_and_repaint(L"highlight", std::move(result.colors)); + command_line.set_colors(std::move(result.colors)); + this->layout_and_repaint(L"highlight"); } } } diff --git a/src/reader.h b/src/reader.h index ee4059abb..37e1ccaec 100644 --- a/src/reader.h +++ b/src/reader.h @@ -14,6 +14,7 @@ #include "common.h" #include "complete.h" +#include "highlight.h" #include "maybe.h" #include "parse_constants.h" @@ -45,9 +46,9 @@ struct edit_t { bool operator==(const edit_t &other) const; }; -/// Modify a string according to the given edit. +/// Modify a string and its syntax highlighting according to the given edit. /// Currently exposed for testing only. -void apply_edit(wcstring *target, const edit_t &edit); +void apply_edit(wcstring *target, std::vector *colors, const edit_t &edit); /// The history of all edits to some command line. struct undo_history_t { @@ -82,6 +83,12 @@ class editable_line_t { public: const wcstring &text() const { return text_; } + const std::vector &colors() const { return colors_; } + void set_colors(std::vector colors) { + assert(colors.size() == size()); + colors_ = std::move(colors); + } + size_t position() const { return position_; } void set_position(size_t position) { position_ = position; } @@ -115,6 +122,8 @@ class editable_line_t { /// The command line. wcstring text_; + /// Syntax highlighting. + std::vector colors_; /// The current position of the cursor in the command line. size_t position_ = 0;