Fix selection when pasting/duplicating items

The item flags weren't getting reset after placing
the new items, so the next operation on them was
having weird behavior.

Fixes https://gitlab.com/kicad/code/kicad/issues/7528

pcbnew/tools/edit_tool.cpp

@@ -628,7 +628,7 @@ int EDIT_TOOL::doMoveSelection( TOOL_EVENT aEvent, bool aPickReference )
         return 0;
 
     LSET     item_layers = selection.GetSelectionLayers();
-    bool     unselect    = selection.IsHover(); // N.B. This must be saved before the re-selection
+    bool     is_hover    = selection.IsHover(); // N.B. This must be saved before the re-selection
                                                 // below
     VECTOR2I pickedReferencePoint;
 
@@ -710,14 +710,15 @@ int EDIT_TOOL::doMoveSelection( TOOL_EVENT aEvent, bool aPickReference )
     if( aPickReference && !pickReferencePoint( _( "Select reference point for move..." ), "", "",
                                                pickedReferencePoint ) )
     {
-        if( unselect )
+        if( is_hover )
             m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true );
 
         editFrame->PopTool( tool );
         return 0;
     }
 
-    std::vector<BOARD_ITEM*> sel_items;
+    std::vector<BOARD_ITEM*> sel_items;     // All the items operated on by the move below
+    std::vector<BOARD_ITEM*> orig_items;    // All the original items in the selection
 
     for( EDA_ITEM* item : selection )
     {
@@ -725,7 +726,10 @@ int EDIT_TOOL::doMoveSelection( TOOL_EVENT aEvent, bool aPickReference )
         FOOTPRINT*  footprint = dynamic_cast<FOOTPRINT*>( item );
 
         if( boardItem )
+        {
+            orig_items.push_back( boardItem );
             sel_items.push_back( boardItem );
+        }
 
         if( footprint )
         {
@@ -963,9 +967,13 @@ int EDIT_TOOL::doMoveSelection( TOOL_EVENT aEvent, bool aPickReference )
     // Remove the dynamic ratsnest from the screen
     m_toolMgr->RunAction( PCB_ACTIONS::hideDynamicRatsnest, true );
 
-    if( unselect )
+    // Unselect all items to update flags
     m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true );
 
+    // Reselect items if they were already selected and we completed the move
+    if( !is_hover && !restore_state )
+        m_toolMgr->RunAction( PCB_ACTIONS::selectItems, true, &orig_items );
+
     editFrame->PopTool( tool );
 
     return restore_state ? -1 : 0;
@@ -1980,7 +1988,7 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent )
     bool increment = aEvent.IsAction( &PCB_ACTIONS::duplicateIncrement );
 
     // Be sure that there is at least one item that we can modify
-    const auto& selection = m_selectionTool->RequestSelection(
+    const PCB_SELECTION& selection = m_selectionTool->RequestSelection(
                 []( const VECTOR2I&, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool )
                 {
                     std::set<BOARD_ITEM*> added_items;
@@ -2119,19 +2127,19 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent )
         editFrame->DisplayToolMsg( wxString::Format( _( "Duplicated %d item(s)" ),
                                                      (int) new_items.size() ) );
 
+        // TODO(ISM): This line can't be used to activate the tool until we allow multiple activations
+        // m_toolMgr->RunAction( PCB_ACTIONS::move, true );
+        // Instead we have to create the event and call the tool's function
+        // directly
+
         // If items were duplicated, pick them up
         // this works well for "dropping" copies around and pushes the commit
         TOOL_EVENT evt = PCB_ACTIONS::move.MakeEvent();
-        bool       move_cancelled = Move( evt ) == -1;
+        Move( evt );
 
-        // After moving the new items, we need to refresh the group and view flags
+        // Deslect the duplicated item if we originally started as a hover selection
+        if( is_hover )
             m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true );
-
-        if( !is_hover && !move_cancelled )
-        {
-            // Do not select new items if they've been deleted again by cancelling Move()
-            m_toolMgr->RunAction( PCB_ACTIONS::selectItems, true, &new_items );
-        }
     }
 
     return 0;

pcbnew/tools/pcb_control.cpp

@@ -850,7 +850,9 @@ int PCB_CONTROL::placeBoardItems( std::vector<BOARD_ITEM*>& aItems, bool aIsNew,
 
     PCB_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool<PCB_SELECTION_TOOL>();
     EDIT_TOOL*          editTool = m_toolMgr->GetTool<EDIT_TOOL>();
-    PCB_SELECTION&      selection = selectionTool->GetSelection();
+
+    std::vector<BOARD_ITEM*> itemsToSel;
+    itemsToSel.reserve( aItems.size() );
 
     for( BOARD_ITEM* item : aItems )
     {
@@ -897,20 +899,18 @@ int PCB_CONTROL::placeBoardItems( std::vector<BOARD_ITEM*>& aItems, bool aIsNew,
         else
             editTool->GetCurrentCommit()->Added( item );
 
-        // Matching the logic of PCB_SELECTION_TOOL::select for PCB_GROUP_T, there
+        // We only need to add the items that aren't inside a group currently selected
-        // is a distinction between which items are SetSelected and which are in
+        // to the selection. If an item is inside a group and that group is selected,
-        // the selection object.  Top-level groups or items not in groups are
+        // then the selection tool will select it for us.
-        // added to the selection object (via selection.Add(), below), but all
-        // items have SetSelected called.  This is because much of the selection
-        // management logic (e.g. move) recursively acts on groups in the
-        // selection, so descendents of groups should not be in the selection
-        // object.
-        item->SetSelected();
-
         if( !item->GetParentGroup() || !alg::contains( aItems, item->GetParentGroup() ) )
-            selection.Add( item );
+            itemsToSel.push_back( item );
     }
 
+    // Select the items that should be selected
+    m_toolMgr->RunAction( PCB_ACTIONS::selectItems, true, &itemsToSel );
+
+    PCB_SELECTION& selection = selectionTool->GetSelection();
+
     if( selection.Size() > 0 )
     {
         if( aAnchorAtOrigin )