Browse Source

LibGUI+TextEditor: Make TextDocument modified state track undo stack

This was quite unreliable before. Changes to the undo stack's modified
state are now reflected in the document's modified state, and the
GUI::TextEditor widget has its undo/redo actions updated automatically.

UndoStack is still a bit hard to understand due to the lazy coalescing
of commands, and that's something we should improve upon (e.g with more
explicit, incremental command merging.) But for now, this is a nice
improvement and undo/redo finally behaves in a way that feels natural.
Andreas Kling 4 years ago
parent
commit
2905e10513

+ 4 - 1
Userland/Applications/TextEditor/MainWidget.cpp

@@ -59,7 +59,10 @@ MainWidget::MainWidget()
 
     m_editor->on_change = [this] {
         update_preview();
-        window()->set_modified(editor().document().is_modified());
+    };
+
+    m_editor->on_modified_change = [this](bool modified) {
+        window()->set_modified(modified);
     };
 
     m_page_view = *find_descendant_of_type_named<Web::OutOfProcessWebView>("webview");

+ 5 - 0
Userland/Libraries/LibGUI/TextEditor.cpp

@@ -1638,6 +1638,11 @@ void TextEditor::document_did_update_undo_stack()
 {
     m_undo_action->set_enabled(can_undo());
     m_redo_action->set_enabled(can_redo());
+
+    // FIXME: This is currently firing more often than it should.
+    //        Ideally we'd only send this out when the undo stack modified state actually changes.
+    if (on_modified_change)
+        on_modified_change(document().is_modified());
 }
 
 void TextEditor::document_did_set_text()

+ 1 - 0
Userland/Libraries/LibGUI/TextEditor.h

@@ -134,6 +134,7 @@ public:
     virtual void redo() { document().redo(); }
 
     Function<void()> on_change;
+    Function<void(bool modified)> on_modified_change;
     Function<void()> on_mousedown;
     Function<void()> on_return_pressed;
     Function<void()> on_escape_pressed;

+ 13 - 3
Userland/Libraries/LibGUI/UndoStack.cpp

@@ -19,7 +19,7 @@ UndoStack::~UndoStack()
 
 bool UndoStack::can_undo() const
 {
-    return m_stack_index > 0;
+    return m_stack_index > 0 || (m_stack.size() == 1 && m_stack[0].commands.size() > 0);
 }
 
 bool UndoStack::can_redo() const
@@ -67,7 +67,7 @@ void UndoStack::pop()
 void UndoStack::push(NonnullOwnPtr<Command>&& command)
 {
     if (m_stack.is_empty())
-        m_stack.append(make<Combo>());
+        finalize_current_combo();
 
     // If the stack cursor is behind the top of the stack, nuke everything from here to the top.
     if (m_stack_index != m_stack.size() - 1) {
@@ -101,6 +101,8 @@ void UndoStack::finalize_current_combo()
 
 void UndoStack::set_current_unmodified()
 {
+    finalize_current_combo();
+
     if (m_clean_index.has_value() && m_clean_index.value() == m_stack_index)
         return;
     m_clean_index = m_stack_index;
@@ -111,7 +113,15 @@ void UndoStack::set_current_unmodified()
 
 bool UndoStack::is_current_modified() const
 {
-    return !(m_clean_index.has_value() && m_clean_index.value() == m_stack_index);
+    if (!m_clean_index.has_value())
+        return true;
+    if (m_stack_index != m_clean_index.value())
+        return true;
+    if (m_stack.is_empty())
+        return false;
+    if (m_stack_index == m_stack.size() - 1 && !m_stack[m_stack_index].commands.is_empty())
+        return true;
+    return false;
 }
 
 void UndoStack::clear()