Keep selected flags on undo/redo items.

Also fixes a memory leak in Symbol Editor undo/redo.

Also moves a few more things to SCH_COMMIT.

Also fixes a couple of LIB_ITEM::Clone() methods that were
failing to return the same uuid.

eeschema/lib_field.cpp

@@ -40,8 +40,8 @@
 
 
 LIB_FIELD::LIB_FIELD( LIB_SYMBOL* aParent, int aId, const wxString& aName ) :
-    LIB_ITEM( LIB_FIELD_T, aParent ),
+        LIB_ITEM( LIB_FIELD_T, aParent ),
-    EDA_TEXT( schIUScale )
+        EDA_TEXT( schIUScale )
 {
     Init( aId );
     m_name = aName;
@@ -49,16 +49,16 @@ LIB_FIELD::LIB_FIELD( LIB_SYMBOL* aParent, int aId, const wxString& aName ) :
 
 
 LIB_FIELD::LIB_FIELD( int aId ) :
-    LIB_ITEM( LIB_FIELD_T, nullptr ),
+        LIB_ITEM( LIB_FIELD_T, nullptr ),
-    EDA_TEXT( schIUScale )
+        EDA_TEXT( schIUScale )
 {
     Init( aId );
 }
 
 
 LIB_FIELD::LIB_FIELD( int aId, const wxString& aName ) :
-    LIB_ITEM( LIB_FIELD_T, nullptr ),
+        LIB_ITEM( LIB_FIELD_T, nullptr ),
-    EDA_TEXT( schIUScale )
+        EDA_TEXT( schIUScale )
 {
     Init( aId );
     m_name = aName;
@@ -196,11 +196,7 @@ bool LIB_FIELD::HitTest( const VECTOR2I& aPosition, int aAccuracy ) const
 
 EDA_ITEM* LIB_FIELD::Clone() const
 {
-    LIB_FIELD* newfield = new LIB_FIELD( m_id );
+    return new LIB_FIELD( *this );
-
-    Copy( newfield );
-
-    return (EDA_ITEM*) newfield;
 }
 
 

eeschema/lib_text.cpp

@@ -39,8 +39,8 @@
 #include <string_utils.h>
 
 LIB_TEXT::LIB_TEXT( LIB_SYMBOL* aParent ) :
-    LIB_ITEM( LIB_TEXT_T, aParent ),
+        LIB_ITEM( LIB_TEXT_T, aParent ),
-    EDA_TEXT( schIUScale, wxEmptyString )
+        EDA_TEXT( schIUScale, wxEmptyString )
 {
     SetTextSize( VECTOR2I( schIUScale.MilsToIU( DEFAULT_TEXT_SIZE ),
                            schIUScale.MilsToIU( DEFAULT_TEXT_SIZE ) ) );
@@ -73,18 +73,7 @@ bool LIB_TEXT::HitTest( const VECTOR2I& aPosition, int aAccuracy ) const
 
 EDA_ITEM* LIB_TEXT::Clone() const
 {
-    LIB_TEXT* newitem = new LIB_TEXT( nullptr );
+    return new LIB_TEXT( *this );
-
-    newitem->m_parent    = m_parent;
-    newitem->m_unit      = m_unit;
-    newitem->m_convert   = m_convert;
-    newitem->m_private   = m_private;
-    newitem->m_flags     = m_flags;
-
-    newitem->SetText( GetText() );
-    newitem->SetAttributes( *this );
-
-    return newitem;
 }
 
 

eeschema/sch_commit.cpp

@@ -143,7 +143,7 @@ void SCH_COMMIT::pushLibEdit( const wxString& aMessage, int aCommitFlags )
         {
             if( frame && copy )
             {
-                frame->SaveCopyInUndoList( aMessage, copy );
+                frame->PushSymbolToUndoList( aMessage, copy );
                 copy = nullptr;   // we've transferred ownership to the undo stack
             }
         }
@@ -381,7 +381,24 @@ EDA_ITEM* SCH_COMMIT::makeImage( EDA_ITEM* aItem ) const
     if( m_isLibEditor )
     {
         SYMBOL_EDIT_FRAME* frame = static_cast<SYMBOL_EDIT_FRAME*>( m_toolMgr->GetToolHolder() );
-        return new LIB_SYMBOL( *frame->GetCurSymbol() );
+        LIB_SYMBOL*        symbol = frame->GetCurSymbol();
+        std::vector<KIID>  selected;
+
+        for( const LIB_ITEM& item : symbol->GetDrawItems() )
+        {
+            if( item.IsSelected() )
+                selected.push_back( item.m_Uuid );
+        }
+
+        symbol = new LIB_SYMBOL( *symbol );
+
+        for( LIB_ITEM& item : symbol->GetDrawItems() )
+        {
+            if( alg::contains( selected, item.m_Uuid ) )
+                item.SetSelected();
+        }
+
+        return symbol;
     }
 
     return aItem->Clone();

eeschema/symbol_editor/symbol_edit_frame.h

@@ -281,9 +281,12 @@ public:
      * Because a symbol in library editor does not have a lot of primitives, the full data is
      * duplicated. It is not worth to try to optimize this save function.
      */
-    void SaveCopyInUndoList( const wxString& aDescription, EDA_ITEM* aItem,
+    void SaveCopyInUndoList( const wxString& aDescription, LIB_SYMBOL* aSymbol,
                              UNDO_REDO aUndoType = UNDO_REDO::LIBEDIT );
 
+    void PushSymbolToUndoList( const wxString& aDescription, LIB_SYMBOL* aSymbolCopy,
+                               UNDO_REDO aUndoType = UNDO_REDO::LIBEDIT );
+
     void GetSymbolFromUndoList();
     void GetSymbolFromRedoList();
 

eeschema/symbol_editor/symbol_editor_undo_redo.cpp

@@ -31,23 +31,20 @@
 #include <tools/ee_selection_tool.h>
 
 
-void SYMBOL_EDIT_FRAME::SaveCopyInUndoList( const wxString& aDescription, EDA_ITEM* aItem,
+void SYMBOL_EDIT_FRAME::PushSymbolToUndoList( const wxString& aDescription, LIB_SYMBOL* aSymbolCopy,
-                                            UNDO_REDO aUndoType )
+                                              UNDO_REDO aUndoType )
 {
-    if( !aItem )
+    if( !aSymbolCopy )
         return;
 
-    LIB_SYMBOL*        copyItem;
     PICKED_ITEMS_LIST* lastcmd = new PICKED_ITEMS_LIST();
 
-    copyItem = new LIB_SYMBOL( * (LIB_SYMBOL*) aItem );
-
     // Clear current flags (which can be temporary set by a current edit command).
-    copyItem->ClearTempFlags();
+    aSymbolCopy->ClearTempFlags();
-    copyItem->ClearEditFlags();
+    aSymbolCopy->ClearEditFlags();
-    copyItem->SetFlags( UR_TRANSIENT );
+    aSymbolCopy->SetFlags( UR_TRANSIENT );
 
-    ITEM_PICKER wrapper( GetScreen(), copyItem, aUndoType );
+    ITEM_PICKER wrapper( GetScreen(), aSymbolCopy, aUndoType );
     lastcmd->PushItem( wrapper );
     lastcmd->SetDescription( aDescription );
     PushCommandToUndoList( lastcmd );
@@ -57,13 +54,19 @@ void SYMBOL_EDIT_FRAME::SaveCopyInUndoList( const wxString& aDescription, EDA_IT
 }
 
 
+void SYMBOL_EDIT_FRAME::SaveCopyInUndoList( const wxString& aDescription, LIB_SYMBOL* aSymbol,
+                                            UNDO_REDO aUndoType )
+{
+    if( aSymbol )
+        PushSymbolToUndoList( aDescription, new LIB_SYMBOL( *aSymbol ), aUndoType );
+}
+
+
 void SYMBOL_EDIT_FRAME::GetSymbolFromRedoList()
 {
     if( GetRedoCommandCount() <= 0 )
         return;
 
-    m_toolManager->RunAction( EE_ACTIONS::clearSelection );
-
     // Load the last redo entry
     PICKED_ITEMS_LIST* redoCommand = PopCommandFromRedoList();
     ITEM_PICKER        redoWrapper = redoCommand->PopItem();
@@ -115,8 +118,6 @@ void SYMBOL_EDIT_FRAME::GetSymbolFromUndoList()
     if( GetUndoCommandCount() <= 0 )
         return;
 
-    m_toolManager->RunAction( EE_ACTIONS::clearSelection );
-
     // Load the last undo entry
     PICKED_ITEMS_LIST* undoCommand = PopCommandFromUndoList();
     wxString           description = undoCommand->GetDescription();

eeschema/tools/ee_tool_base.h

@@ -155,7 +155,7 @@ protected:
             SYMBOL_EDIT_FRAME* editFrame = dynamic_cast<SYMBOL_EDIT_FRAME*>( m_frame );
             wxCHECK_RET( editFrame, wxT( "editFrame is null" ) );
 
-            editFrame->SaveCopyInUndoList( wxEmptyString, static_cast<LIB_ITEM*>( aItem ) );
+            editFrame->SaveCopyInUndoList( wxEmptyString, dynamic_cast<LIB_SYMBOL*>( aItem ) );
         }
         else
         {

eeschema/tools/symbol_editor_edit_tool.cpp

@@ -458,12 +458,9 @@ int SYMBOL_EDITOR_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent )
         switch( item->Type() )
         {
         case LIB_PIN_T:
-        {
+            if( SYMBOL_EDITOR_PIN_TOOL* pinTool = m_toolMgr->GetTool<SYMBOL_EDITOR_PIN_TOOL>() )
-            SYMBOL_EDITOR_PIN_TOOL* pinTool = m_toolMgr->GetTool<SYMBOL_EDITOR_PIN_TOOL>();
-
-            if( pinTool )
                 pinTool->EditPinProperties( (LIB_PIN*) item );
-        }
+
             break;
 
         case LIB_SHAPE_T:
@@ -572,13 +569,14 @@ void SYMBOL_EDITOR_EDIT_TOOL::editFieldProperties( LIB_FIELD* aField )
     wxString newFieldValue = EscapeString( dlg.GetText(), CTX_LIBID );
     wxString oldFieldValue = aField->GetFullText( m_frame->GetUnit() );
 
-    saveCopyInUndoList( parent, UNDO_REDO::LIBEDIT );
+    SCH_COMMIT commit( m_toolMgr );
+    commit.Modify( aField, m_frame->GetScreen() );
 
     dlg.UpdateField( aField );
 
-    updateItem( aField, true );
+    commit.Push( caption );
+
     m_frame->GetCanvas()->Refresh();
-    m_frame->OnModify();
     m_frame->UpdateSymbolMsgPanelInfo();
 }
 
@@ -632,22 +630,25 @@ void SYMBOL_EDITOR_EDIT_TOOL::handlePinDuplication( LIB_PIN* aOldPin, LIB_PIN* a
 
 int SYMBOL_EDITOR_EDIT_TOOL::PinTable( const TOOL_EVENT& aEvent )
 {
+    SCH_COMMIT  commit( m_frame );
     LIB_SYMBOL* symbol = m_frame->GetCurSymbol();
 
     if( !symbol )
         return 0;
 
-    m_toolMgr->RunAction( EE_ACTIONS::clearSelection );
+    commit.Modify( symbol );
 
-    saveCopyInUndoList( symbol, UNDO_REDO::LIBEDIT );
+    m_toolMgr->RunAction( EE_ACTIONS::clearSelection );
 
     DIALOG_LIB_EDIT_PIN_TABLE dlg( m_frame, symbol );
 
     if( dlg.ShowModal() == wxID_CANCEL )
         return -1;
 
+    // TODO: 9.0: this would be better as "Edit Pins", but we're past string freeze, so this
+    // (existing) string will have to do.
+    commit.Push( _( "Edit Pin Properties" ) );
     m_frame->RebuildView();
-    m_frame->OnModify();
 
     return 0;
 }
@@ -719,9 +720,14 @@ int SYMBOL_EDITOR_EDIT_TOOL::SetUnitDisplayName( const TOOL_EVENT& aEvent )
 
 int SYMBOL_EDITOR_EDIT_TOOL::Undo( const TOOL_EVENT& aEvent )
 {
-    m_frame->GetSymbolFromUndoList();
-
     EE_SELECTION_TOOL* selTool = m_toolMgr->GetTool<EE_SELECTION_TOOL>();
+
+    // Nuke the selection for later rebuilding.  This does *not* clear the flags on any items;
+    // it just clears the SELECTION's reference to them.
+    selTool->GetSelection().Clear();
+    {
+        m_frame->GetSymbolFromUndoList();
+    }
     selTool->RebuildSelection();
 
     return 0;
@@ -730,9 +736,14 @@ int SYMBOL_EDITOR_EDIT_TOOL::Undo( const TOOL_EVENT& aEvent )
 
 int SYMBOL_EDITOR_EDIT_TOOL::Redo( const TOOL_EVENT& aEvent )
 {
-    m_frame->GetSymbolFromRedoList();
-
     EE_SELECTION_TOOL* selTool = m_toolMgr->GetTool<EE_SELECTION_TOOL>();
+
+    // Nuke the selection for later rebuilding.  This does *not* clear the flags on any items;
+    // it just clears the SELECTION's reference to them.
+    selTool->GetSelection().Clear();
+    {
+        m_frame->GetSymbolFromRedoList();
+    }
     selTool->RebuildSelection();
 
     return 0;