From 31be840031191df671d5f5ce3397ab87a1f960ee Mon Sep 17 00:00:00 2001 From: Julian Smart Date: Mon, 6 May 2013 13:39:01 +0000 Subject: [PATCH] Applied patches for #15184 (wxRichTextAction fix for when the command identifier is wxRICHTEXT_CHANGE_OBJECT) and #15185 (Make adding/deleting wxRichTextTable rows and columns undoable) git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@73941 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- include/wx/richtext/richtextbuffer.h | 6 ++ interface/wx/richtext/richtextbuffer.h | 8 ++ src/richtext/richtextbuffer.cpp | 95 +++++++++++++++++- tests/controls/richtextctrltest.cpp | 128 +++++++++++++++++++++++++ 4 files changed, 234 insertions(+), 3 deletions(-) diff --git a/include/wx/richtext/richtextbuffer.h b/include/wx/richtext/richtextbuffer.h index a64fc8aa20..ce85bc5e69 100644 --- a/include/wx/richtext/richtextbuffer.h +++ b/include/wx/richtext/richtextbuffer.h @@ -5961,6 +5961,12 @@ public: */ wxRichTextObject* GetObject() const { return m_object; } + /** + Stores the object to replace the one at the position defined by the container address + without making an address for it (cf SetObject() and MakeObject()). + */ + void StoreObject(wxRichTextObject* obj) { m_object = obj; } + /** Sets the object to replace the one at the position defined by the container address and the action's range start position. diff --git a/interface/wx/richtext/richtextbuffer.h b/interface/wx/richtext/richtextbuffer.h index d930d33381..7a12ac6392 100644 --- a/interface/wx/richtext/richtextbuffer.h +++ b/interface/wx/richtext/richtextbuffer.h @@ -5783,6 +5783,14 @@ public: */ wxRichTextObject* GetObject() const { return m_object; } + /** + Stores the object to replace the one at the position defined by the container address + without making an address for it + + @see SetObject(), MakeObject()). + */ + void StoreObject(wxRichTextObject* obj) { m_object = obj; } + /** Sets the object to replace the one at the position defined by the container address and the action's range start position. diff --git a/src/richtext/richtextbuffer.cpp b/src/richtext/richtextbuffer.cpp index 22a123116f..d272acb553 100644 --- a/src/richtext/richtextbuffer.cpp +++ b/src/richtext/richtextbuffer.cpp @@ -10245,6 +10245,19 @@ bool wxRichTextTable::DeleteRows(int startRow, int noRows) if ((startRow + noRows) > m_rowCount) return false; + wxRichTextBuffer* buffer = GetBuffer(); + wxRichTextAction* action = NULL; + wxRichTextTable* clone = NULL; + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + // Create a clone containing the current state of the table. It will be used to Undo the action + clone = wxStaticCast(this->Clone(), wxRichTextTable); + clone->SetParent(GetParent()); + action = new wxRichTextAction(NULL, _("Delete row"), wxRICHTEXT_CHANGE_OBJECT, buffer, this, buffer->GetRichTextCtrl()); + action->SetObject(this); + action->SetPosition(GetRange().GetStart()); + } + int i, j; for (i = startRow; i < (startRow+noRows); i++) { @@ -10262,6 +10275,13 @@ bool wxRichTextTable::DeleteRows(int startRow, int noRows) m_rowCount = m_rowCount - noRows; + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + buffer->SubmitAction(action); + // Finally store the original-state clone; doing so earlier would cause various failures + action->StoreObject(clone); + } + return true; } @@ -10271,6 +10291,19 @@ bool wxRichTextTable::DeleteColumns(int startCol, int noCols) if ((startCol + noCols) > m_colCount) return false; + wxRichTextBuffer* buffer = GetBuffer(); + wxRichTextAction* action = NULL; + wxRichTextTable* clone = NULL; + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + // Create a clone containing the current state of the table. It will be used to Undo the action + clone = wxStaticCast(this->Clone(), wxRichTextTable); + clone->SetParent(GetParent()); + action = new wxRichTextAction(NULL, _("Delete column"), wxRICHTEXT_CHANGE_OBJECT, buffer, this, buffer->GetRichTextCtrl()); + action->SetObject(this); + action->SetPosition(GetRange().GetStart()); + } + bool deleteRows = (noCols == m_colCount); int i, j; @@ -10292,6 +10325,13 @@ bool wxRichTextTable::DeleteColumns(int startCol, int noCols) m_rowCount = 0; m_colCount = m_colCount - noCols; + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + buffer->SubmitAction(action); + // Finally store the original-state clone; doing so earlier would cause various failures + action->StoreObject(clone); + } + return true; } @@ -10301,6 +10341,19 @@ bool wxRichTextTable::AddRows(int startRow, int noRows, const wxRichTextAttr& at if (startRow > m_rowCount) return false; + wxRichTextBuffer* buffer = GetBuffer(); + wxRichTextAction* action = NULL; + wxRichTextTable* clone = NULL; + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + // Create a clone containing the current state of the table. It will be used to Undo the action + clone = wxStaticCast(this->Clone(), wxRichTextTable); + clone->SetParent(GetParent()); + action = new wxRichTextAction(NULL, _("Add row"), wxRICHTEXT_CHANGE_OBJECT, buffer, this, buffer->GetRichTextCtrl()); + action->SetObject(this); + action->SetPosition(GetRange().GetStart()); + } + int i, j; for (i = 0; i < noRows; i++) { @@ -10329,6 +10382,14 @@ bool wxRichTextTable::AddRows(int startRow, int noRows, const wxRichTextAttr& at } m_rowCount = m_rowCount + noRows; + + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + buffer->SubmitAction(action); + // Finally store the original-state clone; doing so earlier would cause various failures + action->StoreObject(clone); + } + return true; } @@ -10338,6 +10399,19 @@ bool wxRichTextTable::AddColumns(int startCol, int noCols, const wxRichTextAttr& if (startCol > m_colCount) return false; + wxRichTextBuffer* buffer = GetBuffer(); + wxRichTextAction* action = NULL; + wxRichTextTable* clone = NULL; + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + // Create a clone containing the current state of the table. It will be used to Undo the action + clone = wxStaticCast(this->Clone(), wxRichTextTable); + clone->SetParent(GetParent()); + action = new wxRichTextAction(NULL, _("Add column"), wxRICHTEXT_CHANGE_OBJECT, buffer, this, buffer->GetRichTextCtrl()); + action->SetObject(this); + action->SetPosition(GetRange().GetStart()); + } + int i, j; for (i = 0; i < m_rowCount; i++) { @@ -10359,6 +10433,13 @@ bool wxRichTextTable::AddColumns(int startCol, int noCols, const wxRichTextAttr& m_colCount = m_colCount + noCols; + if (!buffer->GetRichTextCtrl()->SuppressingUndo()) + { + buffer->SubmitAction(action); + // Finally store the original-state clone; doing so earlier would cause various failures + action->StoreObject(clone); + } + return true; } @@ -10736,10 +10817,18 @@ bool wxRichTextAction::Do() case wxRICHTEXT_CHANGE_OBJECT: { wxRichTextObject* obj = m_objectAddress.GetObject(m_buffer); - // wxRichTextObject* obj = container->GetChildAtPosition(GetRange().GetStart()); - if (obj && m_object) + if (obj && m_object && m_ctrl) { - wxRichTextObjectList::compatibility_iterator node = container->GetChildren().Find(obj); + // If the cloned object is unparented it will cause layout asserts later + // An alternative (would it always be valid?) could be to do: m_object->SetParent(obj->GetParent()) + wxCHECK_MSG(m_object->GetParent(), false, "The stored object must have a valid parent"); + + // The plan is to swap the current object with the stored, previous-state, clone + // We can't get 'node' from the containing buffer (as it doesn't directly store objects) + // so use the parent paragraph + wxRichTextParagraph* para = wxDynamicCast(obj->GetParent(), wxRichTextParagraph); + wxCHECK_MSG(para, false, "Invalid parent paragraph"); + wxRichTextObjectList::compatibility_iterator node = para->GetChildren().Find(obj); if (node) { wxRichTextObject* obj = node->GetData(); diff --git a/tests/controls/richtextctrltest.cpp b/tests/controls/richtextctrltest.cpp index 0d311e54b9..9b9e9ac442 100644 --- a/tests/controls/richtextctrltest.cpp +++ b/tests/controls/richtextctrltest.cpp @@ -62,6 +62,7 @@ private: CPPUNIT_TEST( Font ); CPPUNIT_TEST( Delete ); CPPUNIT_TEST( Url ); + CPPUNIT_TEST( Table ); CPPUNIT_TEST_SUITE_END(); void CharacterEvent(); @@ -91,6 +92,7 @@ private: void Font(); void Delete(); void Url(); + void Table(); wxRichTextCtrl* m_rich; @@ -755,4 +757,130 @@ void RichTextCtrlTestCase::Url() CPPUNIT_ASSERT_EQUAL("http://www.wxwidgets.org", url.GetURL()); } + // Helper function for ::Table() +wxRichTextTable* GetCurrentTableInstance(wxRichTextParagraph* para) +{ + wxRichTextTable* table = wxDynamicCast(para->FindObjectAtPosition(0), wxRichTextTable); + CPPUNIT_ASSERT(table); + return table; +} + +void RichTextCtrlTestCase::Table() +{ + m_rich->BeginSuppressUndo(); + wxRichTextTable* table = m_rich->WriteTable(1, 1); + m_rich->EndSuppressUndo(); + CPPUNIT_ASSERT(table); + CPPUNIT_ASSERT(m_rich->CanUndo() == false); + + // Run the tests twice: first for the original table, then for a contained one + for (int t = 0; t < 2; ++t) + { + // Undo() and Redo() switch table instances, so invalidating 'table' + // The containing paragraph isn't altered, and so can be used to find the current object + wxRichTextParagraph* para = wxDynamicCast(table->GetParent(), wxRichTextParagraph); + CPPUNIT_ASSERT(para); + + CPPUNIT_ASSERT(table->GetColumnCount() == 1); + CPPUNIT_ASSERT(table->GetRowCount() == 1); + + // Test adding columns and rows + for (size_t n = 0; n < 3; ++n) + { + m_rich->BeginBatchUndo("Add col and row"); + + table->AddColumns(0, 1); + table->AddRows(0, 1); + + m_rich->EndBatchUndo(); + } + CPPUNIT_ASSERT(table->GetColumnCount() == 4); + CPPUNIT_ASSERT(table->GetRowCount() == 4); + + // Test deleting columns and rows + for (size_t n = 0; n < 3; ++n) + { + m_rich->BeginBatchUndo("Delete col and row"); + + table->DeleteColumns(table->GetColumnCount() - 1, 1); + table->DeleteRows(table->GetRowCount() - 1, 1); + + m_rich->EndBatchUndo(); + } + CPPUNIT_ASSERT(table->GetColumnCount() == 1); + CPPUNIT_ASSERT(table->GetRowCount() == 1); + + // Test undo, first of the deletions... + CPPUNIT_ASSERT(m_rich->CanUndo()); + for (size_t n = 0; n < 3; ++n) + { + m_rich->Undo(); + } + table = GetCurrentTableInstance(para); + CPPUNIT_ASSERT(table->GetColumnCount() == 4); + CPPUNIT_ASSERT(table->GetRowCount() == 4); + + // ...then the additions + for (size_t n = 0; n < 3; ++n) + { + m_rich->Undo(); + } + table = GetCurrentTableInstance(para); + CPPUNIT_ASSERT(table->GetColumnCount() == 1); + CPPUNIT_ASSERT(table->GetRowCount() == 1); + CPPUNIT_ASSERT(m_rich->CanUndo() == false); + + // Similarly test redo. Additions: + CPPUNIT_ASSERT(m_rich->CanRedo()); + for (size_t n = 0; n < 3; ++n) + { + m_rich->Redo(); + } + table = GetCurrentTableInstance(para); + CPPUNIT_ASSERT(table->GetColumnCount() == 4); + CPPUNIT_ASSERT(table->GetRowCount() == 4); + + // Deletions: + for (size_t n = 0; n < 3; ++n) + { + m_rich->Redo(); + } + table = GetCurrentTableInstance(para); + CPPUNIT_ASSERT(table->GetColumnCount() == 1); + CPPUNIT_ASSERT(table->GetRowCount() == 1); + CPPUNIT_ASSERT(m_rich->CanRedo() == false); + + // Now test multiple addition and deletion, and also suppression + m_rich->BeginSuppressUndo(); + table->AddColumns(0, 3); + table->AddRows(0, 3); + CPPUNIT_ASSERT(table->GetColumnCount() == 4); + CPPUNIT_ASSERT(table->GetRowCount() == 4); + + // Only delete 2 of these. This makes it easy to be sure we're dealing with the child table when we loop + table->DeleteColumns(0, 2); + table->DeleteRows(0, 2); + CPPUNIT_ASSERT(table->GetColumnCount() == 2); + CPPUNIT_ASSERT(table->GetRowCount() == 2); + m_rich->EndSuppressUndo(); + + m_rich->GetCommandProcessor()->ClearCommands(); // otherwise the command-history from this loop will cause CPPUNIT_ASSERT failures in the next one + + if (t == 0) + { + // For round 2, re-run the tests on another table inside the last cell of the first one + wxRichTextCell* cell = table->GetCell(table->GetRowCount() - 1, table->GetColumnCount() - 1); + CPPUNIT_ASSERT(cell); + m_rich->SetFocusObject(cell); + m_rich->BeginSuppressUndo(); + table = m_rich->WriteTable(1, 1); + m_rich->EndSuppressUndo(); + CPPUNIT_ASSERT(table); + } + } + + m_rich->Clear(); + m_rich->SetFocusObject(NULL); +} + #endif //wxUSE_RICHTEXT -- 2.45.2