From 434161687ed0d426cb0649c7a4cee04cb5744548 Mon Sep 17 00:00:00 2001 From: Marek Roszko Date: Fri, 24 Mar 2023 19:45:56 -0400 Subject: [PATCH] Fix cli crash due to dialogs buried in the pcb parser... Fixes sentry KICAD-Q2 (cherry picked from commit 8a8589b9dbb87212eb29032da74fffc2a6612c5e) --- common/confirm.cpp | 25 +--- common/eda_base_frame.cpp | 2 +- common/pgm_base.cpp | 18 +++ eeschema/files-io.cpp | 2 +- include/confirm.h | 7 -- include/pgm_base.h | 7 ++ kicad/kicad_cli.cpp | 2 +- pcbnew/plugins/kicad/pcb_parser.cpp | 171 +++++++++++++++------------- pcbnew/plugins/kicad/pcb_plugin.cpp | 5 +- 9 files changed, 130 insertions(+), 109 deletions(-) diff --git a/common/confirm.cpp b/common/confirm.cpp index 8f5874cb03..6020f619a9 100644 --- a/common/confirm.cpp +++ b/common/confirm.cpp @@ -31,30 +31,13 @@ #include #include #include +#include #include "cli/cli_names.h" // Set of dialogs that have been chosen not to be shown again static std::unordered_map doNotShowAgainDlgs; -bool IsGUI() -{ - if( !wxTheApp ) - return false; - -#if wxCHECK_VERSION( 3, 1, 6 ) - return wxTheApp->IsGUI(); -#else - // wxWidgets older than version 3.1.6 do not have a way to know if the app - // has a GUI or is a console application. - // So the trick is to set the App class name when starting kicad-cli, and when - // the app class name is the kicad-cli class name the app is a console app - bool run_gui = wxTheApp->GetClassName() != KICAD_CLI_APP_NAME; - return run_gui; -#endif -} - - KIDIALOG::KIDIALOG( wxWindow* aParent, const wxString& aMessage, const wxString& aCaption, long aStyle ) : wxRichMessageDialog( aParent, aMessage, aCaption, aStyle | wxCENTRE | wxSTAY_ON_TOP ), @@ -305,7 +288,7 @@ void DisplayError( wxWindow* aParent, const wxString& aText, int aDisplayTime ) return; } - if( !IsGUI() ) + if( !Pgm().IsGUI() ) { wxFprintf( stderr, aText ); return; @@ -330,7 +313,7 @@ void DisplayErrorMessage( wxWindow* aParent, const wxString& aText, const wxStri return; } - if( !IsGUI() ) + if( !Pgm().IsGUI() ) { wxFprintf( stderr, aText ); return; @@ -357,7 +340,7 @@ void DisplayInfoMessage( wxWindow* aParent, const wxString& aMessage, const wxSt return; } - if( !IsGUI() ) + if( !Pgm().IsGUI() ) { wxFprintf( stdout, "%s %s", aMessage, aExtraInfo ); return; diff --git a/common/eda_base_frame.cpp b/common/eda_base_frame.cpp index 1fd1d4c9bf..94946c9ab2 100644 --- a/common/eda_base_frame.cpp +++ b/common/eda_base_frame.cpp @@ -1230,7 +1230,7 @@ bool EDA_BASE_FRAME::IsWritable( const wxFileName& aFileName, bool aVerbose ) void EDA_BASE_FRAME::CheckForAutoSaveFile( const wxFileName& aFileName ) { - if( !IsGUI() ) + if( !Pgm().IsGUI() ) return; wxCHECK_RET( aFileName.IsOk(), wxT( "Invalid file name!" ) ); diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp index dcde82ac5f..2aa1937504 100644 --- a/common/pgm_base.cpp +++ b/common/pgm_base.cpp @@ -857,3 +857,21 @@ ENV_VAR_MAP& PGM_BASE::GetLocalEnvVariables() const { return GetCommonSettings()->m_Env.vars; } + + +bool PGM_BASE::IsGUI() +{ + if( !wxTheApp ) + return false; + +#if wxCHECK_VERSION( 3, 1, 6 ) + return wxTheApp->IsGUI(); +#else + // wxWidgets older than version 3.1.6 do not have a way to know if the app + // has a GUI or is a console application. + // So the trick is to set the App class name when starting kicad-cli, and when + // the app class name is the kicad-cli class name the app is a console app + bool run_gui = wxTheApp->GetClassName() != KICAD_CLI_APP_NAME; + return run_gui; +#endif +} \ No newline at end of file diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index b1e34bebb1..c359cacde3 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -1394,7 +1394,7 @@ bool SCH_EDIT_FRAME::updateAutoSaveFile() void SCH_EDIT_FRAME::CheckForAutoSaveFile( const wxFileName& aFileName ) { - if( !IsGUI() ) + if( !Pgm().IsGUI() ) return; wxCHECK_RET( aFileName.IsOk(), wxT( "Invalid file name!" ) ); diff --git a/include/confirm.h b/include/confirm.h index c78d88d06c..5fe5d75c70 100644 --- a/include/confirm.h +++ b/include/confirm.h @@ -194,11 +194,4 @@ int OKOrCancelDialog( wxWindow* aParent, const wxString& aWarning, const wxStrin int SelectSingleOption( wxWindow* aParent, const wxString& aTitle, const wxString& aMessage, const wxArrayString& aOptions ); -/** - * Determine if the application is running with a GUI - * - * @return true if there is a GUI and false otherwise - */ -bool IsGUI(); - #endif /* __INCLUDE__CONFIRM_H__ */ diff --git a/include/pgm_base.h b/include/pgm_base.h index 7ef1d401af..b4715d0522 100644 --- a/include/pgm_base.h +++ b/include/pgm_base.h @@ -303,6 +303,13 @@ public: const wxString& GetSentryId(); #endif + /** + * Determine if the application is running with a GUI + * + * @return true if there is a GUI and false otherwise + */ + bool IsGUI(); + /** * wxWidgets on MSW tends to crash if you spool up more than one print job at a time. */ diff --git a/kicad/kicad_cli.cpp b/kicad/kicad_cli.cpp index ee8fd5c10a..38845f7145 100644 --- a/kicad/kicad_cli.cpp +++ b/kicad/kicad_cli.cpp @@ -486,7 +486,7 @@ struct APP_KICAD_CLI : public wxAppConsole } catch( const IO_ERROR& ioe ) { - wxLogError( ioe.What() ); + wxFprintf( stderr, ioe.What() ); } catch( ... ) { diff --git a/pcbnew/plugins/kicad/pcb_parser.cpp b/pcbnew/plugins/kicad/pcb_parser.cpp index 7f0e37df8c..d05a9acbfd 100644 --- a/pcbnew/plugins/kicad/pcb_parser.cpp +++ b/pcbnew/plugins/kicad/pcb_parser.cpp @@ -64,6 +64,7 @@ #include #include #include +#include // For some reason wxWidgets is built with wxUSE_BASE64 unset so expose the wxWidgets // base64 code. Needed for PCB_BITMAP @@ -908,92 +909,100 @@ BOARD* PCB_PARSER::parseBOARD_unchecked() if( m_undefinedLayers.size() > 0 ) { - bool deleteItems; - std::vector deleteList; - wxString msg = wxString::Format( _( "Items found on undefined layers. Do you wish to\n" - "rescue them to the User.Comments layer?" ) ); - wxString details = wxString::Format( _( "Undefined layers:" ) ); - - for( const wxString& undefinedLayer : m_undefinedLayers ) - details += wxT( "\n " ) + undefinedLayer; - - wxRichMessageDialog dlg( nullptr, msg, _( "Warning" ), - wxYES_NO | wxCANCEL | wxCENTRE | wxICON_WARNING | wxSTAY_ON_TOP ); - dlg.ShowDetailedText( details ); - dlg.SetYesNoCancelLabels( _( "Rescue" ), _( "Delete" ), _( "Cancel" ) ); - - switch( dlg.ShowModal() ) + if( Pgm().IsGUI() ) { - case wxID_YES: deleteItems = false; break; - case wxID_NO: deleteItems = true; break; - case wxID_CANCEL: - default: THROW_IO_ERROR( wxT( "CANCEL" ) ); - } + bool deleteItems; + std::vector deleteList; + wxString msg = wxString::Format( _( "Items found on undefined layers. Do you wish to\n" + "rescue them to the User.Comments layer?" ) ); + wxString details = wxString::Format( _( "Undefined layers:" ) ); - auto visitItem = [&]( BOARD_ITEM* curr_item ) - { - if( curr_item->GetLayer() == Rescue ) - { - if( deleteItems ) - deleteList.push_back( curr_item ); - else - curr_item->SetLayer( Cmts_User ); - } - }; + for( const wxString& undefinedLayer : m_undefinedLayers ) + details += wxT( "\n " ) + undefinedLayer; - for( PCB_TRACK* track : m_board->Tracks() ) - { - if( track->Type() == PCB_VIA_T ) + wxRichMessageDialog dlg( nullptr, msg, _( "Warning" ), + wxYES_NO | wxCANCEL | wxCENTRE | wxICON_WARNING + | wxSTAY_ON_TOP ); + dlg.ShowDetailedText( details ); + dlg.SetYesNoCancelLabels( _( "Rescue" ), _( "Delete" ), _( "Cancel" ) ); + + switch( dlg.ShowModal() ) { - PCB_VIA* via = static_cast( track ); - PCB_LAYER_ID top_layer, bottom_layer; + case wxID_YES: deleteItems = false; break; + case wxID_NO: deleteItems = true; break; + case wxID_CANCEL: + default: THROW_IO_ERROR( wxT( "CANCEL" ) ); + } - if( via->GetViaType() == VIATYPE::THROUGH ) - continue; - - via->LayerPair( &top_layer, &bottom_layer ); - - if( top_layer == Rescue || bottom_layer == Rescue ) + auto visitItem = [&]( BOARD_ITEM* curr_item ) + { + if( curr_item->GetLayer() == Rescue ) { if( deleteItems ) - deleteList.push_back( via ); + deleteList.push_back( curr_item ); else + curr_item->SetLayer( Cmts_User ); + } + }; + + for( PCB_TRACK* track : m_board->Tracks() ) + { + if( track->Type() == PCB_VIA_T ) + { + PCB_VIA* via = static_cast( track ); + PCB_LAYER_ID top_layer, bottom_layer; + + if( via->GetViaType() == VIATYPE::THROUGH ) + continue; + + via->LayerPair( &top_layer, &bottom_layer ); + + if( top_layer == Rescue || bottom_layer == Rescue ) { - if( top_layer == Rescue ) - top_layer = F_Cu; + if( deleteItems ) + deleteList.push_back( via ); + else + { + if( top_layer == Rescue ) + top_layer = F_Cu; - if( bottom_layer == Rescue ) - bottom_layer = B_Cu; + if( bottom_layer == Rescue ) + bottom_layer = B_Cu; - via->SetLayerPair( top_layer, bottom_layer ); + via->SetLayerPair( top_layer, bottom_layer ); + } } } + else + { + visitItem( track ); + } } - else - { - visitItem( track ); - } - } - for( BOARD_ITEM* zone : m_board->Zones() ) - visitItem( zone ); + for( BOARD_ITEM* zone : m_board->Zones() ) + visitItem( zone ); - for( BOARD_ITEM* drawing : m_board->Drawings() ) - visitItem( drawing ); - - for( FOOTPRINT* fp : m_board->Footprints() ) - { - for( BOARD_ITEM* drawing : fp->GraphicalItems() ) + for( BOARD_ITEM* drawing : m_board->Drawings() ) visitItem( drawing ); - for( BOARD_ITEM* zone : fp->Zones() ) - visitItem( zone ); + for( FOOTPRINT* fp : m_board->Footprints() ) + { + for( BOARD_ITEM* drawing : fp->GraphicalItems() ) + visitItem( drawing ); + + for( BOARD_ITEM* zone : fp->Zones() ) + visitItem( zone ); + } + + for( BOARD_ITEM* curr_item : deleteList ) + m_board->Delete( curr_item ); + + m_undefinedLayers.clear(); + } + else + { + THROW_IO_ERROR( wxT( "One or more undefined layers exists was found, open the project in the PCB Editor to resolve" ) ); } - - for( BOARD_ITEM* curr_item : deleteList ) - m_board->Delete( curr_item ); - - m_undefinedLayers.clear(); } return m_board; @@ -2166,19 +2175,27 @@ void PCB_PARSER::parseSetup() { if( m_showLegacy5ZoneWarning ) { - // Thick outline fill mode no longer supported. Make sure user is OK with - // converting fills. - KIDIALOG dlg( nullptr, _( "The legacy zone fill strategy is no longer " - "supported.\nConvert zones to smoothed polygon " - "fills?" ), - _( "Legacy Zone Warning" ), wxYES_NO | wxICON_WARNING ); + if( Pgm().IsGUI() ) + { + // Thick outline fill mode no longer supported. Make sure user is OK with + // converting fills. + KIDIALOG dlg( nullptr, + _( "The legacy zone fill strategy is no longer " + "supported.\nConvert zones to smoothed polygon " + "fills?" ), + _( "Legacy Zone Warning" ), wxYES_NO | wxICON_WARNING ); - dlg.DoNotShowCheckbox( __FILE__, __LINE__ ); + dlg.DoNotShowCheckbox( __FILE__, __LINE__ ); - if( dlg.ShowModal() == wxID_NO ) - THROW_IO_ERROR( wxT( "CANCEL" ) ); + if( dlg.ShowModal() == wxID_NO ) + THROW_IO_ERROR( wxT( "CANCEL" ) ); - m_showLegacy5ZoneWarning = false; + m_showLegacy5ZoneWarning = false; + } + else + { + THROW_IO_ERROR( wxT( "Legacy zone fill strategy found, open the project in the PCB Editor to resolve" ) ); + } } } diff --git a/pcbnew/plugins/kicad/pcb_plugin.cpp b/pcbnew/plugins/kicad/pcb_plugin.cpp index 476f0bb09b..f24b42795d 100644 --- a/pcbnew/plugins/kicad/pcb_plugin.cpp +++ b/pcbnew/plugins/kicad/pcb_plugin.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -2576,7 +2577,9 @@ void PCB_PLUGIN::FootprintSave( const wxString& aLibraryPath, const FOOTPRINT* a "Would you like to create it?"), aLibraryPath ); - if( !IsGUI() || wxMessageBox( msg, _( "Library Not Found"), wxYES_NO | wxICON_QUESTION ) != wxYES ) + if( !Pgm().IsGUI() + || wxMessageBox( msg, _( "Library Not Found" ), wxYES_NO | wxICON_QUESTION ) + != wxYES ) return; // Save throws its own IO_ERROR on failure, so no need to recreate here