From b8bbe72e48aecbfd05e18f0acd899bbbfde14e8e Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Mon, 24 Aug 2020 22:44:34 +0100
Subject: [PATCH] Another attempt to fix the select-reference-point toolstack
 thing.

This time remove the push/pop from the picker tools as that's
responsible for the issue of trying to re-activate the move action
when its popped.  Instead do the push/pop from the client code
of other actions that need it, and not at all from the move-with-
reference action.

Fixes https://gitlab.com/kicad/code/kicad/issues/5336
---
 common/tool/tools_holder.cpp        |  7 ------
 include/tool/tools_holder.h         |  2 --
 pcbnew/tools/edit_tool.cpp          | 37 ++++++++++++++++-------------
 pcbnew/tools/pcbnew_picker_tool.cpp | 10 ++++++--
 4 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/common/tool/tools_holder.cpp b/common/tool/tools_holder.cpp
index 49d31e9224..2658ab5791 100644
--- a/common/tool/tools_holder.cpp
+++ b/common/tool/tools_holder.cpp
@@ -96,13 +96,6 @@ void TOOLS_HOLDER::PopTool( const std::string& actionName )
 }
 
 
-void TOOLS_HOLDER::ClearToolStack()
-{
-    m_toolStack.clear();
-    DisplayToolMsg( ACTIONS::selectionTool.GetLabel() );
-}
-
-
 std::string TOOLS_HOLDER::CurrentToolName() const
 {
     if( m_toolStack.empty() )
diff --git a/include/tool/tools_holder.h b/include/tool/tools_holder.h
index be80414fe4..159a71c4ff 100644
--- a/include/tool/tools_holder.h
+++ b/include/tool/tools_holder.h
@@ -114,8 +114,6 @@ public:
     virtual void PushTool( const std::string& actionName );
     virtual void PopTool( const std::string& actionName );
 
-    void ClearToolStack();
-
     bool ToolStackIsEmpty() { return m_toolStack.empty(); }
 
     std::string CurrentToolName() const;
diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp
index 77eb5120f3..0530ca3256 100644
--- a/pcbnew/tools/edit_tool.cpp
+++ b/pcbnew/tools/edit_tool.cpp
@@ -369,7 +369,6 @@ int EDIT_TOOL::doMoveSelection( TOOL_EVENT aEvent, bool aPickReference )
         return 0;
 
     std::string tool = aEvent.GetCommandStr().get();
-
     editFrame->PushTool( tool );
     Activate();
     controls->ShowCursor( true );
@@ -381,8 +380,7 @@ int EDIT_TOOL::doMoveSelection( TOOL_EVENT aEvent, bool aPickReference )
         if( unselect )
             m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true );
 
-        editFrame->ClearToolStack();
-
+        editFrame->PopTool( tool );
         return 0;
     }
 
@@ -661,7 +659,6 @@ int EDIT_TOOL::doMoveSelection( TOOL_EVENT aEvent, bool aPickReference )
         m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true );
 
     editFrame->PopTool( tool );
-
     return 0;
 }
 
@@ -1570,7 +1567,6 @@ bool EDIT_TOOL::updateModificationPoint( PCBNEW_SELECTION& aSelection )
 bool EDIT_TOOL::pickReferencePoint( const wxString& aTooltip, const wxString& aSuccessMessage,
                                     const wxString& aCanceledMessage, VECTOR2I& aReferencePoint )
 {
-    std::string         tool = "pcbnew.InteractiveEdit.selectReferencePoint";
     STATUS_TEXT_POPUP   statusPopup( frame() );
     PCBNEW_PICKER_TOOL* picker = m_toolMgr->GetTool<PCBNEW_PICKER_TOOL>();
     OPT<VECTOR2I>       pickedPoint;
@@ -1625,6 +1621,7 @@ bool EDIT_TOOL::pickReferencePoint( const wxString& aTooltip, const wxString& aS
     statusPopup.Move( wxGetMousePosition() + wxPoint( 20, -50 ) );
     statusPopup.Popup();
 
+    std::string tool = "";
     m_toolMgr->RunAction( ACTIONS::pickerTool, true, &tool );
 
     while( !done )
@@ -1642,8 +1639,10 @@ bool EDIT_TOOL::pickReferencePoint( const wxString& aTooltip, const wxString& aS
 
 int EDIT_TOOL::copyToClipboard( const TOOL_EVENT& aEvent )
 {
+    std::string  tool = "pcbnew.InteractiveEdit.selectReferencePoint";
     CLIPBOARD_IO io;
 
+    frame()->PushTool( tool );
     Activate();
 
     PCBNEW_SELECTION& selection = m_selectionTool->RequestSelection(
@@ -1651,21 +1650,25 @@ int EDIT_TOOL::copyToClipboard( const TOOL_EVENT& aEvent )
                 EditToolSelectionFilter( aCollector, EXCLUDE_LOCKED_PADS | EXCLUDE_TRANSIENTS, sTool );
             } );
 
-    if( selection.Empty() )
-        return 1;
+    if( !selection.Empty() )
+    {
+        VECTOR2I refPoint;
+        bool     rv = pickReferencePoint( _( "Select reference point for the copy..." ),
+                                          _( "Selection copied." ),
+                                          _( "Copy cancelled." ),
+                                          refPoint );
+        frame()->SetMsgPanel( board() );
 
-    VECTOR2I refPoint;
-    bool     rv = pickReferencePoint( _( "Select reference point for the copy..." ),
-                                      _( "Selection copied." ), _( "Copy cancelled." ), refPoint );
-    frame()->SetMsgPanel( board() );
+        if( rv )
+        {
+            selection.SetReferencePoint( refPoint );
 
-    if( !rv )
-        return 1;
+            io.SetBoard( board() );
+            io.SaveSelection( selection, m_editModules );
+        }
+    }
 
-    selection.SetReferencePoint( refPoint );
-
-    io.SetBoard( board() );
-    io.SaveSelection( selection, m_editModules );
+    frame()->PopTool( tool );
 
     return 0;
 }
diff --git a/pcbnew/tools/pcbnew_picker_tool.cpp b/pcbnew/tools/pcbnew_picker_tool.cpp
index b59b23911f..166c412f00 100644
--- a/pcbnew/tools/pcbnew_picker_tool.cpp
+++ b/pcbnew/tools/pcbnew_picker_tool.cpp
@@ -46,7 +46,10 @@ int PCBNEW_PICKER_TOOL::Main( const TOOL_EVENT& aEvent )
     int                   finalize_state = WAIT_CANCEL;
 
     std::string tool = *aEvent.Parameter<std::string*>();
-    frame->PushTool( tool );
+
+    if( !tool.empty() )
+        frame->PushTool( tool );
+
     Activate();
     setControls();
 
@@ -152,7 +155,10 @@ int PCBNEW_PICKER_TOOL::Main( const TOOL_EVENT& aEvent )
 
     reset();
     controls->ForceCursorPosition( false );
-    frame->PopTool( tool );
+
+    if( !tool.empty() )
+        frame->PopTool( tool );
+
     return 0;
 }