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

@@ -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.

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.

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 && m_ctrl)
-            if (obj && m_object)
             {
-                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();

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