From 1d7d194062d3c067e022ec3cb7122b214b665c4a Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 26 Jul 2023 17:14:57 +0100 Subject: [PATCH] LibGUI: Use `set_text()` for `ReplaceAllTextCommand` Previously, this was reimplementing the same thing by removing all the document text and then inserting the new text - which internally would insert each code-point individually and fire change notifications for each one. This made the "Reformat GML" button very slow, since it not only had to recalculate the visual lines of the document each time, but also rebuild the preview GUI. The reason not to use `set_text()` is that it would throw away the undo stack, since it always behaved as if the text is a new document. So, let's add a parameter to disable that behaviour. This takes the time for reformatting a ~200 line GML file from several seconds, to basically instantaneous. :^) --- Userland/Libraries/LibGUI/TextDocument.cpp | 25 +++++++++------------- Userland/Libraries/LibGUI/TextDocument.h | 10 +++++---- Userland/Libraries/LibGUI/TextEditor.cpp | 8 +------ 3 files changed, 17 insertions(+), 26 deletions(-) diff --git a/Userland/Libraries/LibGUI/TextDocument.cpp b/Userland/Libraries/LibGUI/TextDocument.cpp index 0b6e166058..2fbd5b0424 100644 --- a/Userland/Libraries/LibGUI/TextDocument.cpp +++ b/Userland/Libraries/LibGUI/TextDocument.cpp @@ -42,10 +42,11 @@ TextDocument::TextDocument(Client* client) }; } -bool TextDocument::set_text(StringView text, AllowCallback allow_callback) +bool TextDocument::set_text(StringView text, AllowCallback allow_callback, IsNewDocument is_new_document) { m_client_notifications_enabled = false; - m_undo_stack.clear(); + if (is_new_document == IsNewDocument::Yes) + m_undo_stack.clear(); m_spans.clear(); m_folding_regions.clear(); remove_all_lines(); @@ -99,7 +100,8 @@ bool TextDocument::set_text(StringView text, AllowCallback allow_callback) clear_text_guard.disarm(); // FIXME: Should the modified state be cleared on some of the earlier returns as well? - set_unmodified(); + if (is_new_document == IsNewDocument::Yes) + set_unmodified(); return true; } @@ -1057,31 +1059,24 @@ DeprecatedString InsertLineCommand::action_text() const return action_text_builder.to_deprecated_string(); } -ReplaceAllTextCommand::ReplaceAllTextCommand(GUI::TextDocument& document, DeprecatedString const& text, GUI::TextRange const& range, DeprecatedString const& action_text) +ReplaceAllTextCommand::ReplaceAllTextCommand(GUI::TextDocument& document, DeprecatedString const& text, DeprecatedString const& action_text) : TextDocumentUndoCommand(document) , m_original_text(document.text()) , m_new_text(text) - , m_range(range) , m_action_text(action_text) { } void ReplaceAllTextCommand::redo() { - m_document.remove(m_range); - m_document.set_all_cursors(m_range.start()); - auto new_cursor = m_document.insert_at(m_range.start(), m_new_text, m_client); - m_range.set_end(new_cursor); - m_document.set_all_cursors(new_cursor); + m_document.set_all_cursors({ 0, 0 }); + m_document.set_text(m_new_text, AllowCallback::Yes, TextDocument::IsNewDocument::No); } void ReplaceAllTextCommand::undo() { - m_document.remove(m_range); - m_document.set_all_cursors(m_range.start()); - auto new_cursor = m_document.insert_at(m_range.start(), m_original_text, m_client); - m_range.set_end(new_cursor); - m_document.set_all_cursors(new_cursor); + m_document.set_all_cursors({ 0, 0 }); + m_document.set_text(m_original_text, AllowCallback::Yes, TextDocument::IsNewDocument::No); } bool ReplaceAllTextCommand::merge_with(GUI::Command const&) diff --git a/Userland/Libraries/LibGUI/TextDocument.h b/Userland/Libraries/LibGUI/TextDocument.h index 8380e71bcf..74db33db0b 100644 --- a/Userland/Libraries/LibGUI/TextDocument.h +++ b/Userland/Libraries/LibGUI/TextDocument.h @@ -74,7 +74,11 @@ public: void set_spans(u32 span_collection_index, Vector spans); - bool set_text(StringView, AllowCallback = AllowCallback::Yes); + enum class IsNewDocument { + No, + Yes, + }; + bool set_text(StringView, AllowCallback = AllowCallback::Yes, IsNewDocument = IsNewDocument::Yes); Vector> const& lines() const { return m_lines; } Vector>& lines() { return m_lines; } @@ -290,19 +294,17 @@ private: class ReplaceAllTextCommand final : public GUI::TextDocumentUndoCommand { public: - ReplaceAllTextCommand(GUI::TextDocument& document, DeprecatedString const& new_text, GUI::TextRange const& range, DeprecatedString const& action_text); + ReplaceAllTextCommand(GUI::TextDocument& document, DeprecatedString const& new_text, DeprecatedString const& action_text); virtual ~ReplaceAllTextCommand() = default; void redo() override; void undo() override; bool merge_with(GUI::Command const&) override; DeprecatedString action_text() const override; DeprecatedString const& text() const { return m_new_text; } - TextRange const& range() const { return m_range; } private: DeprecatedString m_original_text; DeprecatedString m_new_text; - GUI::TextRange m_range; DeprecatedString m_action_text; }; diff --git a/Userland/Libraries/LibGUI/TextEditor.cpp b/Userland/Libraries/LibGUI/TextEditor.cpp index a326eb50a2..53313c9713 100644 --- a/Userland/Libraries/LibGUI/TextEditor.cpp +++ b/Userland/Libraries/LibGUI/TextEditor.cpp @@ -1800,14 +1800,8 @@ void TextEditor::insert_at_cursor_or_replace_selection(StringView text) void TextEditor::replace_all_text_without_resetting_undo_stack(StringView text) { - auto start = GUI::TextPosition(0, 0); - auto last_line_index = line_count() - 1; - auto end = GUI::TextPosition(last_line_index, line(last_line_index).length()); - auto range = GUI::TextRange(start, end); - auto normalized_range = range.normalized(); - execute(text, range, "GML Playground Format Text"); + execute(text, "GML Playground Format Text"); did_change(); - set_cursor(normalized_range.start()); update(); }