Fix cli crash due to dialogs buried in the pcb parser...

Fixes sentry KICAD-Q2

common/confirm.cpp

@@ -31,30 +31,13 @@
 #include <dialogs/html_message_box.h>
 #include <functional>
 #include <unordered_map>
+#include <pgm_base.h>
 #include "cli/cli_names.h"
 
 // Set of dialogs that have been chosen not to be shown again
 static std::unordered_map<unsigned long, int> 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;

common/eda_base_frame.cpp

@@ -1231,7 +1231,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!" ) );

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
+}

eeschema/files-io.cpp

@@ -1400,7 +1400,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!" ) );

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__ */

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.
      */

kicad/kicad_cli.cpp

@@ -499,7 +499,7 @@ struct APP_KICAD_CLI : public wxAppConsole
         }
         catch( const IO_ERROR& ioe )
         {
-            wxLogError( ioe.What() );
+            wxFprintf( stderr, ioe.What() );
         }
         catch( ... )
         {

pcbnew/plugins/kicad/pcb_parser.cpp

@@ -64,6 +64,7 @@
 #include <wx/log.h>
 #include <progress_reporter.h>
 #include <board_stackup_manager/stackup_predefined_prms.h>
+#include <pgm_base.h>
 
 // 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;
+        if( Pgm().IsGUI() )
-        std::vector<BOARD_ITEM*> 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() )
         {
-        case wxID_YES:    deleteItems = false; break;
+            bool                     deleteItems;
-        case wxID_NO:     deleteItems = true;  break;
+            std::vector<BOARD_ITEM*> deleteList;
-        case wxID_CANCEL:
+            wxString msg = wxString::Format( _( "Items found on undefined layers.  Do you wish to\n"
-        default:          THROW_IO_ERROR( wxT( "CANCEL" ) );
+                                                "rescue them to the User.Comments layer?" ) );
-        }
+            wxString details = wxString::Format( _( "Undefined layers:" ) );
 
-        auto visitItem = [&]( BOARD_ITEM* curr_item )
+            for( const wxString& undefinedLayer : m_undefinedLayers )
-                            {
+                details += wxT( "\n   " ) + undefinedLayer;
-                                if( curr_item->GetLayer() == Rescue )
-                                {
-                                    if( deleteItems )
-                                        deleteList.push_back( curr_item );
-                                    else
-                                        curr_item->SetLayer( Cmts_User );
-                                }
-                            };
 
-        for( PCB_TRACK* track : m_board->Tracks() )
+            wxRichMessageDialog dlg( nullptr, msg, _( "Warning" ),
-        {
+                                     wxYES_NO | wxCANCEL | wxCENTRE | wxICON_WARNING
-            if( track->Type() == PCB_VIA_T )
+                                             | wxSTAY_ON_TOP );
+            dlg.ShowDetailedText( details );
+            dlg.SetYesNoCancelLabels( _( "Rescue" ), _( "Delete" ), _( "Cancel" ) );
+
+            switch( dlg.ShowModal() )
             {
-                PCB_VIA*     via = static_cast<PCB_VIA*>( track );
+            case wxID_YES: deleteItems = false; break;
-                PCB_LAYER_ID top_layer, bottom_layer;
+            case wxID_NO: deleteItems = true; break;
+            case wxID_CANCEL:
+            default: THROW_IO_ERROR( wxT( "CANCEL" ) );
+            }
 
-                if( via->GetViaType() == VIATYPE::THROUGH )
+            auto visitItem = [&]( BOARD_ITEM* curr_item )
-                    continue;
+            {
-
+                if( curr_item->GetLayer() == Rescue )
-                via->LayerPair( &top_layer, &bottom_layer );
-
-                if( top_layer == Rescue || bottom_layer == 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<PCB_VIA*>( 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 )
+                        if( deleteItems )
-                            top_layer = F_Cu;
+                            deleteList.push_back( via );
+                        else
+                        {
+                            if( top_layer == Rescue )
+                                top_layer = F_Cu;
 
-                        if( bottom_layer == Rescue )
+                            if( bottom_layer == Rescue )
-                            bottom_layer = B_Cu;
+                                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() )
+            for( BOARD_ITEM* zone : m_board->Zones() )
-            visitItem( zone );
+                visitItem( zone );
 
-        for( BOARD_ITEM* drawing : m_board->Drawings() )
+            for( BOARD_ITEM* drawing : m_board->Drawings() )
-            visitItem( drawing );
-
-        for( FOOTPRINT* fp : m_board->Footprints() )
-        {
-            for( BOARD_ITEM* drawing : fp->GraphicalItems() )
                 visitItem( drawing );
 
-            for( BOARD_ITEM* zone : fp->Zones() )
+            for( FOOTPRINT* fp : m_board->Footprints() )
-                visitItem( zone );
+            {
+                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
+                    if( Pgm().IsGUI() )
-                    // converting fills.
+                    {
-                    KIDIALOG dlg( nullptr, _( "The legacy zone fill strategy is no longer "
+                        // Thick outline fill mode no longer supported.  Make sure user is OK with
-                                              "supported.\nConvert zones to smoothed polygon "
+                        // converting fills.
-                                              "fills?" ),
+                        KIDIALOG dlg( nullptr,
-                                  _( "Legacy Zone Warning" ), wxYES_NO | wxICON_WARNING );
+                                      _( "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 )
+                        if( dlg.ShowModal() == wxID_NO )
-                        THROW_IO_ERROR( wxT( "CANCEL" ) );
+                            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" ) );
+                    }
                 }
             }
 

pcbnew/plugins/kicad/pcb_plugin.cpp

@@ -45,6 +45,7 @@
 #include <pcb_text.h>
 #include <pcb_textbox.h>
 #include <pcbnew_settings.h>
+#include <pgm_base.h>
 #include <plugins/kicad/pcb_plugin.h>
 #include <plugins/kicad/pcb_parser.h>
 #include <trace_helpers.h>
@@ -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