Don't toggle selection twice when trying to select.

Also fixes issues with trying to use a CONTEXT_MENU as a wxMenu.

eeschema/tools/sch_selection_tool.cpp

@@ -38,6 +38,7 @@
 #include <sch_collectors.h>
 #include <painter.h>
 #include <eeschema_id.h>
+#include <menus_helpers.h>
 
 // Selection tool actions
 TOOL_ACTION SCH_ACTIONS::selectionActivate( "eeschema.InteractiveSelection",
@@ -312,6 +313,20 @@ void SCH_SELECTION_TOOL::guessSelectionCandidates( SCH_COLLECTOR& collector,
 }
 
 
+SELECTION& SCH_SELECTION_TOOL::RequestSelection()
+{
+    if( m_selection.Empty() )
+    {
+        VECTOR2D cursorPos = getViewControls()->GetCursorPosition( false );
+
+        clearSelection();
+        SelectPoint( cursorPos );
+    }
+
+    return m_selection;
+}
+
+
 bool SCH_SELECTION_TOOL::selectCursor( const KICAD_T aFilterList[], bool aForceSelect  )
 {
     if( aForceSelect || m_selection.Empty() )
@@ -405,6 +420,7 @@ int SCH_SELECTION_TOOL::ClearSelection( const TOOL_EVENT& aEvent )
 int SCH_SELECTION_TOOL::SelectionMenu( const TOOL_EVENT& aEvent )
 {
     SCH_COLLECTOR* collector = aEvent.Parameter<SCH_COLLECTOR*>();
+
     doSelectionMenu( collector, wxEmptyString );
 
     return 0;
@@ -414,6 +430,43 @@ int SCH_SELECTION_TOOL::SelectionMenu( const TOOL_EVENT& aEvent )
 bool SCH_SELECTION_TOOL::doSelectionMenu( SCH_COLLECTOR* aCollector, const wxString& aTitle )
 {
     SCH_ITEM* current = nullptr;
+#if 1
+// ====================================================================================
+// JEY TODO: use wxWidgets event loop for showing menu until we move to modern toolset event loop
+    wxMenu selectMenu;
+
+    AddMenuItem( &selectMenu, wxID_NONE, _( "Clarify Selection" ), KiBitmap( info_xpm ) );
+    selectMenu.AppendSeparator();
+
+    for( int i = 0;  i < aCollector->GetCount() && i < MAX_SELECT_ITEM_IDS;  i++ )
+    {
+        SCH_ITEM* item = ( *aCollector )[i];
+        wxString text = item->GetSelectMenuText( m_frame->GetUserUnits() );
+        BITMAP_DEF xpm = item->GetMenuImage();
+        AddMenuItem( &selectMenu, i, text, KiBitmap( xpm ) );
+    }
+
+    // Set to NULL in case the user aborts the clarification context menu.
+    m_frame->GetScreen()->SetCurItem( nullptr );
+    int idx = m_frame->GetPopupMenuSelectionFromUser( selectMenu );
+
+    if( idx == wxNOT_FOUND )
+    {
+        m_frame->GetScreen()->SetCurItem( nullptr );
+        return false;
+    }
+
+    m_frame->GetCanvas()->MoveCursorToCrossHair();
+
+    current = ( *aCollector )[ idx ];
+    m_frame->GetScreen()->SetCurItem( current );
+
+    aCollector->Empty();
+    aCollector->Append( current );
+    return true;
+
+// ====================================================================================
+#endif
     CONTEXT_MENU menu;
 
     int limit = std::min( MAX_SELECT_ITEM_IDS, aCollector->GetCount() );
@@ -433,28 +486,6 @@ bool SCH_SELECTION_TOOL::doSelectionMenu( SCH_COLLECTOR* aCollector, const wxStr
 
     menu.SetIcon( info_xpm );
     menu.DisplayTitle( true );
-
-#if 1
-    // JEY TODO: use wxWidgets event loop for showing menu until we move over to modern toolset event loop
-    m_frame->GetCanvas()->SetAbortRequest( true );   // Changed to false if an item is selected
-    m_frame->PopupMenu( &menu );
-
-    if( m_frame->GetCanvas()->GetAbortRequest() )
-    {
-        m_frame->GetScreen()->SetCurItem( nullptr );
-        return false;
-    }
-
-    m_frame->GetCanvas()->MoveCursorToCrossHair();
-    current = m_frame->GetScreen()->GetCurItem();
-
-    toggleSelection( current );
-
-    aCollector->Empty();
-    aCollector->Append( current );
-    return true;
-
-#endif
     SetContextMenu( &menu, CMENU_NOW );
 
     while( OPT_TOOL_EVENT evt = Wait() )
@@ -498,8 +529,6 @@ bool SCH_SELECTION_TOOL::doSelectionMenu( SCH_COLLECTOR* aCollector, const wxStr
     {
         unhighlight( current, BRIGHTENED );
 
-        toggleSelection( current );
-
         aCollector->Empty();
         aCollector->Append( current );
         return true;

eeschema/tools/sch_selection_tool.h

@@ -66,6 +66,13 @@ public:
      */
     SELECTION& GetSelection();
 
+    /**
+     * Function RequestSelection()
+     *
+     * Similar to GetSelection(), but will run SelectCursor() first if the selection is empty.
+     */
+    SELECTION& RequestSelection();
+
     /**
      * Function selectPoint()
      * Selects an item pointed by the parameter aWhere. If there is more than one item at that
@@ -97,8 +104,10 @@ public:
 
     /**
      * Function SelectionMenu()
-     * Allows the selection of a single item from a list of items via a popup menu.  The
+     * Shows a popup menu to trim the COLLECTOR passed as aEvent's parameter down to a single
-     * list is passed as aEvent's parameter.
+     * item.
+     *
+     * NOTE: this routine DOES NOT modify the selection.
      */
     int SelectionMenu( const TOOL_EVENT& aEvent );
 

pcbnew/tools/selection_tool.cpp

@@ -1427,6 +1427,7 @@ void SELECTION_TOOL::clearSelection()
 int SELECTION_TOOL::SelectionMenu( const TOOL_EVENT& aEvent )
 {
     GENERAL_COLLECTOR* collector = aEvent.Parameter<GENERAL_COLLECTOR*>();
+
     doSelectionMenu( collector, wxEmptyString );
 
     return 0;
@@ -1501,7 +1502,6 @@ bool SELECTION_TOOL::doSelectionMenu( GENERAL_COLLECTOR* aCollector, const wxStr
 
     if( current )
     {
-        toggleSelection( current );
         aCollector->Empty();
         aCollector->Append( current );
         return true;

pcbnew/tools/selection_tool.h

@@ -127,8 +127,10 @@ public:
 
     /**
      * Function SelectionMenu()
-     * Allows the selection of a single item from a list of items via a popup menu.  The
+     * Shows a popup menu to trim the COLLECTOR passed as aEvent's parameter down to a single
-     * list is passed as aEvent's parameter.
+     * item.
+     *
+     * NOTE: this routine DOES NOT modify the selection.
      */
     int SelectionMenu( const TOOL_EVENT& aEvent );