kicad-developers team mailing list archive

[Ian McInerney <Ian.S.McInerney@xxxxxxxx>]

I noticed some minor issues in cvpcb as I was using it this morning,
specifically:
* ESC no longer closed the window since it didn't properly generate the
close event
* Copy/Cut from a context menu did not work due to a focus-loss issue
* Invalid text could be pasted into the association after warning the user
it was invalid

This patch cleans up these issues.

-Ian

From 6caf68f4d222e6fad539a1d0b4275c8c33b11fa4 Mon Sep 17 00:00:00 2001
From: Ian McInerney <Ian.S.McInerney@xxxxxxxx>
Date: Sun, 6 Oct 2019 12:19:41 +0200
Subject: [PATCH] cvpcb: Fix some issues

* ESC no longer closed the window since it didn't properly generate
  the close event
* Copy/Cut from a context menu did not work due to a focus-loss issue
* Add better error handling for the copy/cut/paste actions to prevent
  text that isn't an FPID from being inserted
---
 cvpcb/cvpcb_mainframe.cpp              | 13 +++++++++----
 cvpcb/tools/cvpcb_association_tool.cpp | 27 +++++++++++++++++++++++---
 cvpcb/tools/cvpcb_control.cpp          |  3 +--
 3 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/cvpcb/cvpcb_mainframe.cpp b/cvpcb/cvpcb_mainframe.cpp
index 10cd5822d..70d8c546a 100644
--- a/cvpcb/cvpcb_mainframe.cpp
+++ b/cvpcb/cvpcb_mainframe.cpp
@@ -212,8 +212,12 @@ void CVPCB_MAINFRAME::setupTools()
 
     CVPCB_CONTROL* tool = m_toolManager->GetTool<CVPCB_CONTROL>();
 
+    // Even though these menus will open with the right-click, we treat them as a normal
+    // menu instead of a context menu because we don't care about their position and want
+    // to be able to tell the difference between a menu click and a hotkey activation.
+
     // Create the context menu for the component list box
-    m_componentContextMenu = new ACTION_MENU( true );
+    m_componentContextMenu = new ACTION_MENU( false );
     m_componentContextMenu->SetTool( tool );
     m_componentContextMenu->Add( CVPCB_ACTIONS::showFootprintViewer );
     m_componentContextMenu->AppendSeparator();
@@ -224,7 +228,7 @@ void CVPCB_MAINFRAME::setupTools()
     m_componentContextMenu->Add( CVPCB_ACTIONS::deleteAssoc );
 
     // Create the context menu for the footprint list box
-    m_footprintContextMenu = new ACTION_MENU( true );
+    m_footprintContextMenu = new ACTION_MENU( false );
     m_footprintContextMenu->SetTool( tool );
     m_footprintContextMenu->Add( CVPCB_ACTIONS::showFootprintViewer );
 }
@@ -248,7 +252,7 @@ void CVPCB_MAINFRAME::setupEventHandlers()
     m_saveAndContinue->Bind( wxEVT_COMMAND_BUTTON_CLICKED,
             [this]( wxCommandEvent& )
             {
-                // saveAssociations must be run immediatley
+                // saveAssociations must be run immediately
                 this->GetToolManager()->RunAction( CVPCB_ACTIONS::saveAssociations, true );
             } );
 
@@ -256,7 +260,7 @@ void CVPCB_MAINFRAME::setupEventHandlers()
     Bind( wxEVT_BUTTON,
             [this]( wxCommandEvent& )
             {
-                // saveAssociations must be run immediatley, before running Close( true )
+                // saveAssociations must be run immediately, before running Close( true )
                 this->GetToolManager()->RunAction( CVPCB_ACTIONS::saveAssociations, true );
                 Close( true );
             }, wxID_OK );
@@ -446,6 +450,7 @@ void CVPCB_MAINFRAME::AssociateFootprint( const CVPCB_ASSOCIATION& aAssociation,
         wxString msg =
                 wxString::Format( _( "\"%s\" is not a valid footprint." ), fpid.Format().wx_str() );
         DisplayErrorMessage( this, msg );
+        return;
     }
 
     // Set the new footprint
diff --git a/cvpcb/tools/cvpcb_association_tool.cpp b/cvpcb/tools/cvpcb_association_tool.cpp
index d9fb66d18..b8319d19f 100644
--- a/cvpcb/tools/cvpcb_association_tool.cpp
+++ b/cvpcb/tools/cvpcb_association_tool.cpp
@@ -50,7 +50,15 @@ int CVPCB_ASSOCIATION_TOOL::CopyAssoc( const TOOL_EVENT& aEvent )
 {
     COMPONENT* comp;
     LIB_ID     fpid;
-    switch( m_frame->GetFocusedControl() )
+
+    // Default to using the component control as the source of the copy
+    CVPCB_MAINFRAME::CONTROL_TYPE copyControl = CVPCB_MAINFRAME::CONTROL_COMPONENT;
+
+    // If using the keyboard to copy, find out what control we are in and use that.
+    if( aEvent.HasPosition() )
+        copyControl = m_frame->GetFocusedControl();
+
+    switch( copyControl )
     {
     case CVPCB_MAINFRAME::CONTROL_FOOTPRINT:
         fpid.Parse( m_frame->GetSelectedFootprint(), LIB_ID::ID_PCB );
@@ -84,6 +92,8 @@ int CVPCB_ASSOCIATION_TOOL::CopyAssoc( const TOOL_EVENT& aEvent )
         wxTheClipboard->Flush();
         wxTheClipboard->Close();
     }
+    else
+        wxLogDebug( "Failed to open the clipboard" );
 
     return 0;
 }
@@ -91,8 +101,9 @@ int CVPCB_ASSOCIATION_TOOL::CopyAssoc( const TOOL_EVENT& aEvent )
 
 int CVPCB_ASSOCIATION_TOOL::CutAssoc( const TOOL_EVENT& aEvent )
 {
-    // Only cut when in the component frame
-    if( m_frame->GetFocusedControl() != CVPCB_MAINFRAME::CONTROL_COMPONENT )
+    // If using the keyboard, only cut in the component frame
+    if( aEvent.HasPosition()
+            && ( m_frame->GetFocusedControl() != CVPCB_MAINFRAME::CONTROL_COMPONENT ) )
         return 0;
 
     // Get the selection, but only use the first one
@@ -123,6 +134,11 @@ int CVPCB_ASSOCIATION_TOOL::CutAssoc( const TOOL_EVENT& aEvent )
         wxTheClipboard->Flush();
         wxTheClipboard->Close();
     }
+    else
+    {
+        wxLogDebug( "Failed to open the clipboard" );
+        return 0;
+    }
 
     // Remove the association
     m_frame->AssociateFootprint( CVPCB_ASSOCIATION( idx.front(), "" ) );
@@ -148,6 +164,11 @@ int CVPCB_ASSOCIATION_TOOL::PasteAssoc( const TOOL_EVENT& aEvent )
         wxTheClipboard->GetData( data );
         wxTheClipboard->Close();
     }
+    else
+    {
+        wxLogDebug( "Failed to open the clipboard" );
+        return 0;
+    }
 
     if( fpid.Parse( data.GetText(), LIB_ID::ID_PCB ) >= 0 )
         return 0;
diff --git a/cvpcb/tools/cvpcb_control.cpp b/cvpcb/tools/cvpcb_control.cpp
index 1040b3b88..e63e4f7ec 100644
--- a/cvpcb/tools/cvpcb_control.cpp
+++ b/cvpcb/tools/cvpcb_control.cpp
@@ -60,8 +60,7 @@ int CVPCB_CONTROL::Main( const TOOL_EVENT& aEvent )
         // The escape key maps to the cancel event, which is used to close the window
         if( evt->IsCancel() )
         {
-            wxCloseEvent dummy;
-            m_frame->OnCloseWindow( dummy );
+            m_frame->Close( false );
             handled = true;
         }
         else if( evt->IsKeyPressed() )
-- 
2.21.0