Add COMMIT structure to Schematic and Symbol editors

Provides a single-point access for modifying the schematic and symbol
elements that allows chaining updates and reverting partial changes.
Standardizes the undo hierarchy between pcb and schematic editors

As this is another layer on the existing undo/redo structure, the
initial commit does not replace all undo/redo calls currently existing.
These will be handled in a series of follow-on commits

common/commit.cpp

@@ -45,7 +45,7 @@ COMMIT::~COMMIT()
 }
 
 
-COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType )
+COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen )
 {
     // CHT_MODIFY and CHT_DONE are not compatible
     wxASSERT( ( aChangeType & ( CHT_MODIFY | CHT_DONE ) ) != ( CHT_MODIFY | CHT_DONE ) );
@@ -56,11 +56,11 @@ COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType )
     {
     case CHT_ADD:
         wxASSERT( m_changedItems.find( aItem ) == m_changedItems.end() );
-        makeEntry( aItem, CHT_ADD | flag );
+        makeEntry( aItem, CHT_ADD | flag, nullptr, aScreen );
         return *this;
 
     case CHT_REMOVE:
-        makeEntry( aItem, CHT_REMOVE | flag );
+        makeEntry( aItem, CHT_REMOVE | flag, nullptr, aScreen );
         return *this;
 
     case CHT_MODIFY:
@@ -71,7 +71,7 @@ COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType )
         wxASSERT( clone );
 
         if( clone )
-            return createModified( parent, clone, flag );
+            return createModified( parent, clone, flag, aScreen );
 
         break;
     }
@@ -84,16 +84,17 @@ COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType )
 }
 
 
-COMMIT& COMMIT::Stage( std::vector<EDA_ITEM*>& container, CHANGE_TYPE aChangeType )
+COMMIT& COMMIT::Stage( std::vector<EDA_ITEM*> &container, CHANGE_TYPE aChangeType,
+        BASE_SCREEN *aScreen )
 {
     for( EDA_ITEM* item : container )
-        Stage( item, aChangeType );
+        Stage( item, aChangeType, aScreen);
 
     return *this;
 }
 
 
-COMMIT& COMMIT::Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag )
+COMMIT& COMMIT::Stage( const PICKED_ITEMS_LIST &aItems, UNDO_REDO aModFlag, BASE_SCREEN *aScreen )
 {
     for( unsigned int i = 0; i < aItems.GetCount(); i++ )
     {
@@ -108,11 +109,11 @@ COMMIT& COMMIT::Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag )
             assert( change_type == UNDO_REDO::CHANGED );
 
             // There was already a copy created, so use it
-            Modified( item, copy );
+            Modified( item, copy, aScreen );
         }
         else
         {
-            Stage( item, convert( change_type ) );
+            Stage( item, convert( change_type ), aScreen );
         }
     }
 
@@ -120,15 +121,15 @@ COMMIT& COMMIT::Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag )
 }
 
 
-int COMMIT::GetStatus( EDA_ITEM* aItem )
+int COMMIT::GetStatus( EDA_ITEM* aItem, BASE_SCREEN *aScreen )
 {
-    COMMIT_LINE* entry = findEntry( parentObject( aItem ) );
+    COMMIT_LINE* entry = findEntry( parentObject( aItem ), aScreen );
 
     return entry ? entry->m_type : 0;
 }
 
 
-COMMIT& COMMIT::createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlags )
+COMMIT& COMMIT::createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlags, BASE_SCREEN* aScreen )
 {
     EDA_ITEM* parent = parentObject( aItem );
     auto entryIt = m_changedItems.find( parent );
@@ -139,22 +140,22 @@ COMMIT& COMMIT::createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlag
         return *this; // item has been already modified once
     }
 
-    makeEntry( parent, CHT_MODIFY | aExtraFlags, aCopy );
+    makeEntry( parent, CHT_MODIFY | aExtraFlags, aCopy, aScreen );
 
     return *this;
 }
 
 
-void COMMIT::makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy )
+void COMMIT::makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy, BASE_SCREEN *aScreen )
 {
     // Expect an item copy if it is going to be modified
     wxASSERT( !!aCopy == ( ( aType & CHT_TYPE ) == CHT_MODIFY ) );
 
     if( m_changedItems.find( aItem ) != m_changedItems.end() )
     {
-        alg::delete_if( m_changes, [aItem]( const COMMIT_LINE& aEnt )
+        alg::delete_if( m_changes, [aItem, aScreen]( const COMMIT_LINE& aEnt )
                                    {
-                                       return aEnt.m_item == aItem;
+                                       return aEnt.m_item == aItem && aEnt.m_screen == aScreen;
                                    } );
     }
 
@@ -163,17 +164,18 @@ void COMMIT::makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy )
     ent.m_item = aItem;
     ent.m_type = aType;
     ent.m_copy = aCopy;
+    ent.m_screen = aScreen;
 
     m_changedItems.insert( aItem );
     m_changes.push_back( ent );
 }
 
 
-COMMIT::COMMIT_LINE* COMMIT::findEntry( EDA_ITEM* aItem )
+COMMIT::COMMIT_LINE* COMMIT::findEntry( EDA_ITEM* aItem, BASE_SCREEN *aScreen )
 {
     for( COMMIT_LINE& change : m_changes )
     {
-        if( change.m_item == aItem )
+        if( change.m_item == aItem && change.m_screen == aScreen )
             return &change;
     }
 

eeschema/CMakeLists.txt

@@ -317,6 +317,7 @@ set( EESCHEMA_SRCS
     sch_validators.cpp
     sch_view.cpp
     schematic.cpp
+    schematic_commit.cpp
     schematic_settings.cpp
     schematic_undo_redo.cpp
     sheet.cpp

eeschema/annotate.cpp

@@ -27,6 +27,7 @@
 #include <reporter.h>
 #include <sch_edit_frame.h>
 #include <schematic.h>
+#include <schematic_commit.h>
 #include <erc_settings.h>
 #include <sch_reference_list.h>
 #include <symbol_library.h>
@@ -57,16 +58,20 @@ void SCH_EDIT_FRAME::mapExistingAnnotation( std::map<wxString, wxString>& aMap )
 }
 
 
-void SCH_EDIT_FRAME::DeleteAnnotation( ANNOTATE_SCOPE_T aAnnotateScope, bool aRecursive,
+void SCH_EDIT_FRAME::DeleteAnnotation( ANNOTATE_SCOPE_T aAnnotateScope, bool aRecursive )
-                                       bool* aAppendUndo )
 {
+
+    SCH_SHEET_LIST sheets = Schematic().GetSheets();
+    SCH_SCREEN*    screen = GetScreen();
+    SCH_SHEET_PATH currentSheet = GetCurrentSheet();
+    SCHEMATIC_COMMIT commit( this );
+
     auto clearSymbolAnnotation =
         [&]( EDA_ITEM* aItem, SCH_SCREEN* aScreen, SCH_SHEET_PATH* aSheet, bool aResetPrefixes )
         {
             SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( aItem );
+            commit.Modify( aItem, aScreen );
 
-            SaveCopyInUndoList( aScreen, symbol, UNDO_REDO::CHANGED, *aAppendUndo );
-            *aAppendUndo = true;
             symbol->ClearAnnotation( aSheet, aResetPrefixes );
         };
 
@@ -77,10 +82,6 @@ void SCH_EDIT_FRAME::DeleteAnnotation( ANNOTATE_SCOPE_T aAnnotateScope, bool aRe
                     clearSymbolAnnotation( item, aScreen, aSheet, aResetPrefixes );
             };
 
-    SCH_SHEET_LIST sheets = Schematic().GetSheets();
-    SCH_SCREEN*    screen = GetScreen();
-    SCH_SHEET_PATH currentSheet = GetCurrentSheet();
-
     switch( aAnnotateScope )
     {
     case ANNOTATE_ALL:
@@ -151,9 +152,9 @@ void SCH_EDIT_FRAME::DeleteAnnotation( ANNOTATE_SCOPE_T aAnnotateScope, bool aRe
     if( erc_dlg )
         static_cast<DIALOG_ERC*>( erc_dlg )->UpdateAnnotationWarning();
 
-    SyncView();
+    commit.Push( _( "Delete Annotation" ) );
+
     GetCanvas()->Refresh();
-    OnModify();
 
     // Must go after OnModify() so the connectivity graph has been updated
     UpdateNetHighlightStatus();

eeschema/dialogs/dialog_annotate.cpp

@@ -245,9 +245,7 @@ void DIALOG_ANNOTATE::OnApplyClick( wxCommandEvent& event )
 
 void DIALOG_ANNOTATE::OnClearAnnotationClick( wxCommandEvent& event )
 {
-    bool appendUndo = false;
+    m_Parent->DeleteAnnotation( GetScope(), GetRecursive() );
-
-    m_Parent->DeleteAnnotation( GetScope(), GetRecursive(), &appendUndo );
     m_btnClear->Enable( false );
 }
 

eeschema/lib_field.h

@@ -73,6 +73,11 @@ public:
         return wxT( "LIB_FIELD" );
     }
 
+    static inline bool ClassOf( const EDA_ITEM* aItem )
+    {
+        return aItem && aItem->Type() == LIB_FIELD_T;
+    }
+
     wxString GetTypeName() const override
     {
         return _( "Field" );

eeschema/lib_item.h

@@ -92,6 +92,26 @@ public:
      */
     virtual wxString GetTypeName() const = 0;
 
+    static inline bool ClassOf( const EDA_ITEM* aItem )
+    {
+        if( !aItem )
+            return false;
+
+        switch ( aItem->Type() )
+        {
+        case LIB_SHAPE_T:
+        case LIB_TEXT_T:
+        case LIB_TEXTBOX_T:
+        case LIB_PIN_T:
+        case LIB_FIELD_T:
+            return true;
+        default:
+            break;
+        }
+
+        return false;
+    }
+
     /**
      * Begin drawing a symbol library draw item at \a aPosition.
      *

eeschema/lib_pin.h

@@ -66,6 +66,11 @@ public:
         return wxT( "LIB_PIN" );
     }
 
+    static inline bool ClassOf( const EDA_ITEM* aItem )
+    {
+        return aItem && aItem->Type() == LIB_PIN_T;
+    }
+
     wxString GetTypeName() const override
     {
         return _( "Pin" );

eeschema/lib_shape.h

@@ -44,6 +44,11 @@ public:
         return wxT( "LIB_SHAPE" );
     }
 
+    static inline bool ClassOf( const EDA_ITEM* aItem )
+    {
+        return aItem && aItem->Type() == LIB_SHAPE_T;
+    }
+
     wxString GetTypeName() const override
     {
         return ShowShape();

eeschema/lib_symbol.cpp

@@ -1336,6 +1336,13 @@ wxString LIB_SYMBOL::GetPrefix()
 }
 
 
+void LIB_SYMBOL::RunOnChildren( const std::function<void( LIB_ITEM* )>& aFunction )
+{
+    for( LIB_ITEM& item : m_drawings )
+        aFunction( &item );
+}
+
+
 int LIB_SYMBOL::UpdateFieldOrdinals()
 {
     int retv = 0;

eeschema/lib_symbol.h

@@ -134,6 +134,11 @@ public:
         return wxT( "LIB_SYMBOL" );
     }
 
+    static inline bool ClassOf( const EDA_ITEM* aItem )
+    {
+        return aItem && aItem->Type() == LIB_SYMBOL_T;
+    }
+
     virtual void SetName( const wxString& aName );
     wxString GetName() const override { return m_name; }
 
@@ -309,6 +314,8 @@ public:
 
     wxString GetPrefix();
 
+    void RunOnChildren( const std::function<void( LIB_ITEM* )>& aFunction );
+
     /**
      * Order optional field indices.
      *

eeschema/lib_text.h

@@ -50,6 +50,11 @@ public:
         return wxT( "LIB_TEXT" );
     }
 
+    static inline bool ClassOf( const EDA_ITEM* aItem )
+    {
+        return aItem && aItem->Type() == LIB_TEXT_T;
+    }
+
     wxString GetTypeName() const override
     {
         return _( "Text" );

eeschema/sch_base_frame.h

@@ -216,7 +216,7 @@ public:
      * Add an item to the screen (and view)
      * aScreen is the screen the item is located on, if not the current screen
      */
-    void AddToScreen( EDA_ITEM* aItem, SCH_SCREEN* aScreen );
+    void AddToScreen( EDA_ITEM* aItem, SCH_SCREEN* aScreen = nullptr );
 
     /**
      * Remove an item from the screen (and view)

eeschema/sch_edit_frame.h

@@ -344,9 +344,8 @@ public:
      * Clear the current symbol annotation.
      *
      * @param aCurrentSheetOnly Where to clear the annotation. See #ANNOTATE_SCOPE_T
-     * @param appendUndo true to add the action to the previous undo list
      */
-    void DeleteAnnotation( ANNOTATE_SCOPE_T aAnnotateScope, bool aRecursive, bool* appendUndo );
+    void DeleteAnnotation( ANNOTATE_SCOPE_T aAnnotateScope, bool aRecursive );
 
     /**
      * Annotate the symbols in the schematic that are not currently annotated. Multi-unit symbols

eeschema/schematic_commit.cpp

@@ -0,0 +1,484 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#include <macros.h>
+#include <tool/tool_manager.h>
+#include <tools/ee_tool_base.h>
+
+#include <lib_item.h>
+#include <lib_pin.h>
+#include <lib_shape.h>
+#include <lib_symbol.h>
+#include <lib_text.h>
+
+#include <sch_screen.h>
+#include <sch_sheet_path.h>
+#include <schematic.h>
+
+#include <view/view.h>
+#include <schematic_commit.h>
+
+#include <functional>
+
+
+SCHEMATIC_COMMIT::SCHEMATIC_COMMIT( TOOL_MANAGER* aToolMgr ) :
+        m_toolMgr( aToolMgr ),
+        m_isLibEditor( false )
+{
+}
+
+
+SCHEMATIC_COMMIT::SCHEMATIC_COMMIT( EE_TOOL_BASE<SCH_BASE_FRAME>* aTool )
+{
+    m_toolMgr = aTool->GetManager();
+    m_isLibEditor = aTool->IsSymbolEditor();
+}
+
+
+SCHEMATIC_COMMIT::SCHEMATIC_COMMIT( EDA_DRAW_FRAME* aFrame )
+{
+    m_toolMgr = aFrame->GetToolManager();
+    m_isLibEditor = aFrame->IsType( FRAME_SCH_SYMBOL_EDITOR );
+}
+
+
+SCHEMATIC_COMMIT::~SCHEMATIC_COMMIT()
+{
+}
+
+
+COMMIT& SCHEMATIC_COMMIT::Stage( EDA_ITEM *aItem, CHANGE_TYPE aChangeType, BASE_SCREEN *aScreen )
+{
+    wxCHECK( aItem, *this );
+
+    aItem->ClearFlags( IS_MODIFIED_CHILD );
+
+    // If aItem belongs a symbol, the full symbol will be saved because undo/redo does
+    // not handle "sub items" modifications.
+    if( aItem->GetParent() && aItem->GetParent()->IsType( { SCH_SYMBOL_T, LIB_SYMBOL_T } ) )
+    {
+        aItem->SetFlags( IS_MODIFIED_CHILD );
+        aItem = aItem->GetParent();
+    }
+
+
+    return COMMIT::Stage( aItem, aChangeType, aScreen );
+}
+
+
+COMMIT& SCHEMATIC_COMMIT::Stage( std::vector<EDA_ITEM*> &container, CHANGE_TYPE aChangeType,
+        BASE_SCREEN *aScreen )
+{
+    for( EDA_ITEM* item : container )
+        Stage( item, aChangeType, aScreen );
+
+    return *this;
+}
+
+
+COMMIT& SCHEMATIC_COMMIT::Stage( const PICKED_ITEMS_LIST &aItems, UNDO_REDO aModFlag,
+        BASE_SCREEN *aScreen )
+{
+    return COMMIT::Stage( aItems, aModFlag, aScreen );
+}
+
+
+void SCHEMATIC_COMMIT::pushLibEdit( const wxString& aMessage, int aCommitFlags )
+{
+    // Objects potentially interested in changes:
+    PICKED_ITEMS_LIST   undoList;
+    KIGFX::VIEW*        view = m_toolMgr->GetView();
+
+    SYMBOL_EDIT_FRAME*  sym_frame = static_cast<SYMBOL_EDIT_FRAME*>( m_toolMgr->GetToolHolder() );
+    LIB_SYMBOL*         symbol = sym_frame->GetCurSymbol();
+    std::set<EDA_ITEM*> savedModules;
+    EE_SELECTION_TOOL*  selTool = m_toolMgr->GetTool<EE_SELECTION_TOOL>();
+    bool                itemsDeselected = false;
+    bool                selectedModified = false;
+
+    if( Empty() )
+        return;
+
+    for( COMMIT_LINE& ent : m_changes )
+    {
+        int changeType = ent.m_type & CHT_TYPE;
+        int changeFlags = ent.m_type & CHT_FLAGS;
+        LIB_ITEM* libItem = static_cast<LIB_ITEM*>( ent.m_item );
+
+        wxASSERT( ent.m_item );
+
+        // Module items need to be saved in the undo buffer before modification
+        if( ent.m_item->Type() != LIB_SYMBOL_T )
+        {
+            ent.m_item = ent.m_item->GetParent();
+            wxASSERT( ent.m_item );
+        }
+
+        // We have not saved the symbol yet, so let's create an entry
+        if( savedModules.count( ent.m_item ) == 0 )
+        {
+            if( !( aCommitFlags & SKIP_UNDO ) && sym_frame )
+                sym_frame->SaveCopyInUndoList( ent.m_item, UNDO_REDO::CHANGED, aCommitFlags & APPEND_UNDO );
+
+            savedModules.insert( ent.m_item );
+        }
+
+        if( ent.m_item->IsSelected() )
+            selectedModified = true;
+
+        symbol->RunOnChildren(
+            [&selectedModified]( LIB_ITEM* aItem )
+            {
+                if( aItem->HasFlag( IS_MODIFIED_CHILD ) )
+                    selectedModified = true;
+            } );
+
+        switch( changeType )
+        {
+        case CHT_ADD:
+        {
+            wxASSERT( libItem->Type() != LIB_SYMBOL_T );
+
+            libItem->SetParent( symbol );
+
+            if( !( changeFlags & CHT_DONE ) )
+                symbol->AddDrawItem( libItem );
+
+            if( view )
+                view->Add( libItem );
+
+            break;
+        }
+
+        case CHT_REMOVE:
+        {
+            if( libItem->IsSelected() )
+            {
+                if( selTool )
+                    selTool->RemoveItemFromSel( libItem, true /* quiet mode */ );
+
+                itemsDeselected = true;
+            }
+
+            // Avoid removing mandatory fields
+            if( libItem->Type() == LIB_FIELD_T && static_cast<LIB_FIELD*>( libItem )->IsMandatory() )
+                break;
+
+            if( view )
+                view->Remove( libItem );
+
+            if( !( changeFlags & CHT_DONE ) )
+                symbol->RemoveDrawItem( libItem );
+
+            break;
+
+        }
+
+        case CHT_MODIFY:
+        {
+            if( view )
+            {
+                view->Update( libItem );
+
+                symbol->RunOnChildren(
+                        [&]( LIB_ITEM* aChild )
+                        {
+                            view->Update( aChild );
+                        });
+            }
+
+            // if no undo entry is needed, the copy would create a memory leak
+            if( aCommitFlags & SKIP_UNDO )
+                delete ent.m_copy;
+
+            break;
+        }
+
+        default:
+            wxASSERT( false );
+            break;
+        }
+    }
+
+    m_toolMgr->PostEvent( { TC_MESSAGE, TA_MODEL_CHANGE, AS_GLOBAL } );
+
+    if( itemsDeselected )
+        m_toolMgr->PostEvent( EVENTS::UnselectedEvent );
+
+    if( selectedModified )
+        m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified );
+
+    if( sym_frame && !( aCommitFlags & SKIP_SET_DIRTY ) )
+        sym_frame->OnModify();
+
+    clear();
+}
+
+
+void SCHEMATIC_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags )
+{
+    // Objects potentially interested in changes:
+    PICKED_ITEMS_LIST   undoList;
+    KIGFX::VIEW*        view = m_toolMgr->GetView();
+
+    SCH_EDIT_FRAME*     frame = static_cast<SCH_EDIT_FRAME*>( m_toolMgr->GetToolHolder() );
+    EE_SELECTION_TOOL*  selTool = m_toolMgr->GetTool<EE_SELECTION_TOOL>();
+    bool                itemsDeselected = false;
+    bool                selectedModified = false;
+
+    if( Empty() )
+        return;
+
+    for( COMMIT_LINE& ent : m_changes )
+    {
+        int changeType = ent.m_type & CHT_TYPE;
+        int changeFlags = ent.m_type & CHT_FLAGS;
+        SCH_ITEM* schItem = static_cast<SCH_ITEM*>( ent.m_item );
+        SCH_SCREEN* screen = static_cast<SCH_SCREEN*>( ent.m_screen );
+
+        wxASSERT( ent.m_item );
+
+        if( ent.m_item->IsSelected() )
+            selectedModified = true;
+
+        switch( changeType )
+        {
+        case CHT_ADD:
+        {
+            if( !schItem->GetParent() )
+            {
+                if( !( aCommitFlags & SKIP_UNDO ) )
+                    undoList.PushItem( ITEM_PICKER( screen, schItem, UNDO_REDO::NEWITEM ) );
+
+                if( !( changeFlags & CHT_DONE ) )
+                    frame->GetScreen()->Append( schItem );
+            }
+
+            if( view )
+                view->Add( schItem );
+
+            break;
+        }
+
+        case CHT_REMOVE:
+        {
+            if( !( aCommitFlags & SKIP_UNDO ) )
+                undoList.PushItem( ITEM_PICKER( screen, schItem, UNDO_REDO::DELETED ) );
+
+            if( schItem->IsSelected() )
+            {
+                if( selTool )
+                    selTool->RemoveItemFromSel( schItem, true /* quiet mode */ );
+
+                itemsDeselected = true;
+            }
+
+            if( schItem->Type() == SCH_FIELD_T )
+            {
+                static_cast<SCH_FIELD*>( schItem )->SetVisible( false );
+                break;
+            }
+
+            if( view )
+                view->Remove( schItem );
+
+            if( !( changeFlags & CHT_DONE ) )
+                frame->GetScreen()->Remove( schItem );
+
+            break;
+        }
+
+        case CHT_MODIFY:
+        {
+            if( !( aCommitFlags & SKIP_UNDO ) )
+            {
+                ITEM_PICKER itemWrapper( screen, schItem, UNDO_REDO::CHANGED );
+                wxASSERT( ent.m_copy );
+                itemWrapper.SetLink( ent.m_copy );
+                undoList.PushItem( itemWrapper );
+            }
+
+            if( view )
+                view->Update( schItem );
+
+            // if no undo entry is needed, the copy would create a memory leak
+            if( aCommitFlags & SKIP_UNDO )
+                delete ent.m_copy;
+
+            break;
+        }
+
+        default:
+            wxASSERT( false );
+            break;
+        }
+    }
+
+    if( !( aCommitFlags & SKIP_UNDO ) && frame )
+        frame->SaveCopyInUndoList(undoList, UNDO_REDO::UNSPECIFIED, aCommitFlags & APPEND_UNDO );
+
+    m_toolMgr->PostEvent( { TC_MESSAGE, TA_MODEL_CHANGE, AS_GLOBAL } );
+
+    if( itemsDeselected )
+        m_toolMgr->PostEvent( EVENTS::UnselectedEvent );
+
+    if( selectedModified )
+        m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified );
+
+    if( frame && !( aCommitFlags & SKIP_SET_DIRTY ) )
+        frame->OnModify();
+
+    clear();
+}
+
+
+void SCHEMATIC_COMMIT::Push( const wxString& aMessage, int aCommitFlags )
+{
+    if( m_isLibEditor )
+        pushLibEdit( aMessage, aCommitFlags );
+    else
+        pushSchEdit( aMessage, aCommitFlags );
+}
+
+
+EDA_ITEM* SCHEMATIC_COMMIT::parentObject( EDA_ITEM* aItem ) const
+{
+    if( SCH_SYMBOL* parentSymbol = dyn_cast<SCH_SYMBOL*>( aItem->GetParent() ) )
+        return parentSymbol;
+
+    if( LIB_SYMBOL* parentSymbol = dyn_cast<LIB_SYMBOL*>( aItem->GetParent() ) )
+        return parentSymbol;
+
+    if( m_isLibEditor )
+        return static_cast<SYMBOL_EDIT_FRAME*>( m_toolMgr->GetToolHolder() )->GetCurSymbol();
+
+    return aItem;
+}
+
+
+EDA_ITEM* SCHEMATIC_COMMIT::makeImage( EDA_ITEM* aItem ) const
+{
+    if( m_isLibEditor )
+        return new LIB_SYMBOL(
+                *static_cast<SYMBOL_EDIT_FRAME*>( m_toolMgr->GetToolHolder() )->GetCurSymbol() );
+
+    return aItem->Clone();
+}
+
+
+void SCHEMATIC_COMMIT::revertLibEdit()
+{
+    // The first element in the commit is the original, and libedit
+    // just saves copies of the whole symbol, so grab the original and discard the rest
+    SYMBOL_EDIT_FRAME*  sym_frame = static_cast<SYMBOL_EDIT_FRAME*>( m_toolMgr->GetToolHolder() );
+    LIB_SYMBOL*         sym = static_cast<LIB_SYMBOL*>( m_changes.front().m_item );
+
+    sym_frame->SetCurSymbol( sym,  false );
+
+    for( size_t ii = 1; ii < m_changes.size(); ++ii )
+        delete m_changes[ii].m_item;
+
+    clear();
+}
+
+
+void SCHEMATIC_COMMIT::Revert()
+{
+    PICKED_ITEMS_LIST                  undoList;
+    KIGFX::VIEW*                       view = m_toolMgr->GetView();
+
+    if( m_changes.empty() )
+        return;
+
+    if( m_isLibEditor )
+    {
+        revertLibEdit();
+        return;
+    }
+
+    SCHEMATIC& schematic = static_cast<SCH_EDIT_FRAME*>( m_toolMgr->GetToolHolder() )->Schematic();
+
+    for( auto it = m_changes.rbegin(); it != m_changes.rend(); ++it )
+    {
+        COMMIT_LINE& ent = *it;
+        SCH_ITEM* item = static_cast<SCH_ITEM*>( ent.m_item );
+        SCH_ITEM* copy = static_cast<SCH_ITEM*>( ent.m_copy );
+        SCH_SCREEN* screen = static_cast<SCH_SCREEN*>( ent.m_screen );
+        int changeType = ent.m_type & CHT_TYPE;
+        int changeFlags = ent.m_type & CHT_FLAGS;
+
+        switch( changeType )
+        {
+        case CHT_ADD:
+            if( !( changeFlags & CHT_DONE ) )
+                break;
+
+            view->Remove( item );
+            screen->Remove( item );
+            break;
+
+        case CHT_REMOVE:
+            if( !( changeFlags & CHT_DONE ) )
+                break;
+
+            view->Add( item );
+            screen->Append( item );
+            break;
+
+        case CHT_MODIFY:
+        {
+            view->Remove( item );
+            item->SwapData( copy );
+
+            // Special cases for items which have instance data
+            if( item->GetParent() && item->GetParent()->Type() == SCH_SYMBOL_T
+                    && item->Type() == SCH_FIELD_T )
+            {
+                SCH_FIELD*  field = static_cast<SCH_FIELD*>( item );
+                SCH_SYMBOL* symbol = static_cast<SCH_SYMBOL*>( item->GetParent() );
+
+                if( field->GetId() == REFERENCE_FIELD )
+                {
+                    symbol->SetRef( schematic.GetSheets().FindSheetForScreen( screen ),
+                                    field->GetText() );
+                }
+            }
+
+            view->Add( item );
+
+            delete copy;
+            break;
+        }
+
+        default:
+            wxASSERT( false );
+            break;
+        }
+    }
+
+    EE_SELECTION_TOOL* selTool = m_toolMgr->GetTool<EE_SELECTION_TOOL>();
+    selTool->RebuildSelection();
+
+    clear();
+}
+

eeschema/schematic_commit.h

@@ -0,0 +1,80 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2016 CERN
+ * Copyright (C) 2016-2023 KiCad Developers, see AUTHORS.txt for contributors.
+ * @author Tomasz Wlostowski <tomasz.wlostowski@cern.ch>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+#ifndef SCHEMATIC_COMMIT_H
+#define SCHEMATIC_COMMIT_H
+
+#include <commit.h>
+
+class PICKED_ITEMS_LIST;
+class TOOL_MANAGER;
+class SCH_EDIT_FRAME;
+class EDA_DRAW_FRAME;
+class TOOL_BASE;
+
+template<class T>
+class EE_TOOL_BASE;
+
+#define SKIP_UNDO          0x0001
+#define APPEND_UNDO        0x0002
+#define SKIP_SET_DIRTY     0x0004
+#define SKIP_CONNECTIVITY  0x0008
+
+class SCHEMATIC_COMMIT : public COMMIT
+{
+public:
+    SCHEMATIC_COMMIT( TOOL_MANAGER* aToolMgr );
+    SCHEMATIC_COMMIT( EDA_DRAW_FRAME* aFrame );
+    SCHEMATIC_COMMIT( EE_TOOL_BASE<SCH_BASE_FRAME>* aFrame );
+
+    virtual ~SCHEMATIC_COMMIT();
+
+    virtual void Push( const wxString& aMessage = wxT( "A commit" ),
+                       int aCommitFlags = 0 ) override;
+
+    virtual void Revert() override;
+    COMMIT&      Stage( EDA_ITEM *aItem, CHANGE_TYPE aChangeType, BASE_SCREEN *aScreen = nullptr )
+                     override;
+    COMMIT&      Stage( std::vector<EDA_ITEM*> &container, CHANGE_TYPE aChangeType,
+                    BASE_SCREEN *aScreen = nullptr ) override;
+    COMMIT&      Stage( const PICKED_ITEMS_LIST &aItems, UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED,
+                    BASE_SCREEN *aScreen = nullptr ) override;
+
+private:
+    EDA_ITEM* parentObject( EDA_ITEM* aItem ) const override;
+
+    EDA_ITEM* makeImage( EDA_ITEM* aItem ) const override;
+
+    void pushLibEdit(  const wxString& aMessage, int aCommitFlags );
+    void pushSchEdit(  const wxString& aMessage, int aCommitFlags );
+
+    void revertLibEdit();
+
+private:
+    TOOL_MANAGER*  m_toolMgr;
+    bool           m_isLibEditor;
+};
+
+#endif

eeschema/symbol_editor/symbol_editor_undo_redo.cpp

@@ -30,6 +30,7 @@
 #include <tools/ee_actions.h>
 #include <tools/ee_selection_tool.h>
 
+
 void SYMBOL_EDIT_FRAME::SaveCopyInUndoList( EDA_ITEM* aItem, UNDO_REDO aUndoType, bool aAppend )
 {
     wxASSERT_MSG( !aAppend, "Append not needed/supported for symbol editor" );

eeschema/tools/ee_tool_base.h

@@ -32,12 +32,12 @@
 #include <tool/tool_menu.h>
 #include <tool/actions.h>
 #include <tools/ee_selection_tool.h>
-#include <sch_view.h>
 #include <sch_edit_frame.h>
+#include <sch_view.h>
+#include <schematic_commit.h>
 #include <symbol_edit_frame.h>
 #include <undo_redo_container.h>
 
-
 class EE_SELECTION;
 
 /**
@@ -92,9 +92,20 @@ public:
             m_isSymbolEditor = dynamic_cast<SYMBOL_EDIT_FRAME*>( m_frame ) != nullptr;
         }
 
+        if( aReason != RUN )
+            m_commit = std::make_unique<SCHEMATIC_COMMIT>( m_toolMgr );
+
         m_view = static_cast<KIGFX::SCH_VIEW*>( getView() );
     }
 
+    /**
+     * Returns true if the tool is running in the symbol editor
+     */
+    bool IsSymbolEditor() const
+    {
+        return m_isSymbolEditor;
+    }
+
 protected:
     /**
      * Similar to getView()->Update(), but handles items that are redrawn by their parents
@@ -190,6 +201,8 @@ protected:
     KIGFX::SCH_VIEW*   m_view;
     EE_SELECTION_TOOL* m_selectionTool;
     bool               m_isSymbolEditor;
+
+    std::unique_ptr<SCHEMATIC_COMMIT> m_commit;
 };
 
 

include/commit.h

@@ -34,6 +34,7 @@
 #include <undo_redo_container.h>
 
 class EDA_ITEM;
+class BASE_SCREEN;
 
 ///< Types of changes
 enum CHANGE_TYPE {
@@ -75,39 +76,39 @@ public:
     virtual ~COMMIT();
 
     ///< Add a new item to the model
-    COMMIT& Add( EDA_ITEM* aItem )
+    COMMIT& Add( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr )
     {
-        return Stage( aItem, CHT_ADD );
+        return Stage( aItem, CHT_ADD, aScreen );
     }
 
     ///< Notify observers that aItem has been added
-    COMMIT& Added( EDA_ITEM* aItem )
+    COMMIT& Added( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr )
     {
-        return Stage( aItem, CHT_ADD | CHT_DONE );
+        return Stage( aItem, CHT_ADD | CHT_DONE, aScreen );
     }
 
     ///< Remove a new item from the model
-    COMMIT& Remove( EDA_ITEM* aItem )
+    COMMIT& Remove( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr )
     {
-        return Stage( aItem, CHT_REMOVE );
+        return Stage( aItem, CHT_REMOVE, aScreen );
     }
 
     ///< Notify observers that aItem has been removed
-    COMMIT& Removed( EDA_ITEM* aItem )
+    COMMIT& Removed( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr )
     {
-        return Stage( aItem, CHT_REMOVE | CHT_DONE );
+        return Stage( aItem, CHT_REMOVE | CHT_DONE, aScreen );
     }
 
     ///< Modify a given item in the model.
     ///< Must be called before modification is performed.
-    COMMIT& Modify( EDA_ITEM* aItem )
+    COMMIT& Modify( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr )
     {
-        return Stage( aItem, CHT_MODIFY );
+        return Stage( aItem, CHT_MODIFY, aScreen );
     }
 
     ///< Create an undo entry for an item that has been already modified. Requires a copy done
     ///< before the modification.
-    COMMIT& Modified( EDA_ITEM* aItem, EDA_ITEM* aCopy )
+    COMMIT& Modified( EDA_ITEM* aItem, EDA_ITEM* aCopy, BASE_SCREEN *aScreen = nullptr )
     {
         return createModified( aItem, aCopy );
     }
@@ -123,12 +124,15 @@ public:
     }
 
     ///< Add a change of the item aItem of type aChangeType to the change list.
-    virtual COMMIT& Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType );
+    virtual COMMIT& Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType,
+                           BASE_SCREEN *aScreen = nullptr );
 
-    virtual COMMIT& Stage( std::vector<EDA_ITEM*>& container, CHANGE_TYPE aChangeType );
+    virtual COMMIT& Stage( std::vector<EDA_ITEM*>& container, CHANGE_TYPE aChangeType,
+                           BASE_SCREEN *aScreen = nullptr );
 
     virtual COMMIT& Stage( const PICKED_ITEMS_LIST& aItems,
-                           UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED );
+                           UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED,
+                           BASE_SCREEN *aScreen = nullptr );
 
     ///< Execute the changes.
     virtual void Push( const wxString& aMessage = wxT( "A commit" ), int aFlags = 0 ) = 0;
@@ -142,7 +146,7 @@ public:
     }
 
     ///< Returns status of an item.
-    int GetStatus( EDA_ITEM* aItem );
+    int GetStatus( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr );
 
 protected:
     struct COMMIT_LINE
@@ -150,6 +154,7 @@ protected:
         EDA_ITEM*   m_item;       ///< Main item that is added/deleted/modified
         EDA_ITEM*   m_copy;       ///< Optional copy of the item
         CHANGE_TYPE m_type;       ///< Modification type
+        BASE_SCREEN* m_screen;
     };
 
     // Should be called in Push() & Revert() methods
@@ -159,16 +164,18 @@ protected:
         m_changes.clear();
     }
 
-    COMMIT& createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlags = 0 );
+    COMMIT& createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlags = 0,
+                            BASE_SCREEN *aScreen = nullptr );
 
-    virtual void makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy = nullptr );
+    virtual void makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy = nullptr,
+                            BASE_SCREEN *aScreen = nullptr );
 
     /**
      * Search for an entry describing change for a particular item.
      *
      * @return null if there is no related entry.
      */
-    COMMIT_LINE* findEntry( EDA_ITEM* aItem );
+    COMMIT_LINE* findEntry( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr );
 
     virtual EDA_ITEM* parentObject( EDA_ITEM* aItem ) const = 0;
 

pcbnew/board_commit.cpp

@@ -75,7 +75,7 @@ BOARD* BOARD_COMMIT::GetBoard() const
 }
 
 
-COMMIT& BOARD_COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType )
+COMMIT& BOARD_COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen )
 {
     wxCHECK( aItem, *this );
 
@@ -127,15 +127,15 @@ COMMIT& BOARD_COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType )
 }
 
 
-COMMIT& BOARD_COMMIT::Stage( std::vector<EDA_ITEM*>& container, CHANGE_TYPE aChangeType )
+COMMIT& BOARD_COMMIT::Stage( std::vector<EDA_ITEM*>& container, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen )
 {
-    return COMMIT::Stage( container, aChangeType );
+    return COMMIT::Stage( container, aChangeType, aScreen );
 }
 
 
-COMMIT& BOARD_COMMIT::Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag )
+COMMIT& BOARD_COMMIT::Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag, BASE_SCREEN* aScreen )
 {
-    return COMMIT::Stage( aItems, aModFlag );
+    return COMMIT::Stage( aItems, aModFlag, aScreen );
 }
 
 

pcbnew/board_commit.h

@@ -57,10 +57,13 @@ public:
                        int aCommitFlags = 0 ) override;
 
     virtual void Revert() override;
-    COMMIT&      Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType ) override;
+    COMMIT&      Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType,
-    COMMIT&      Stage( std::vector<EDA_ITEM*>& container, CHANGE_TYPE aChangeType ) override;
+                        BASE_SCREEN* aScreen = nullptr ) override;
+    COMMIT&      Stage( std::vector<EDA_ITEM*>& container, CHANGE_TYPE aChangeType,
+                        BASE_SCREEN* aScreen = nullptr ) override;
     COMMIT&      Stage( const PICKED_ITEMS_LIST& aItems,
-                        UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED ) override;
+                        UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED,
+                        BASE_SCREEN* aScreen = nullptr ) override;
 
 private:
     EDA_ITEM* parentObject( EDA_ITEM* aItem ) const override;