Move to requiring explicit action to save project settings

(cherry picked from commit 610e787ada)

Fixes https://gitlab.com/kicad/code/kicad/issues/14414

common/dialogs/dialog_page_settings.cpp

@@ -223,7 +223,7 @@ bool DIALOG_PAGES_SETTINGS::TransferDataFromWindow()
         m_screen->SetContentModified();
 
         if( LocalPrjConfigChanged() )
-            m_parent->SaveProjectSettings();
+            m_parent->OnModify();
 
         // Call the post processing (if any) after changes
         m_parent->OnPageSettingsChange();

eeschema/dialogs/dialog_annotate.cpp

@@ -132,7 +132,7 @@ DIALOG_ANNOTATE::~DIALOG_ANNOTATE()
         if( projSettings.m_AnnotateStartNum != startNum )
         {
             projSettings.m_AnnotateStartNum = startNum;
-            schFrame->SaveProjectSettings();
+            schFrame->OnModify();
         }
     }
 }

eeschema/dialogs/dialog_export_netlist.cpp

@@ -129,7 +129,7 @@ private:
     void InstallPageSpiceModel();
 
     bool TransferDataFromWindow() override;
-    void NetlistUpdateOpt();
+    bool NetlistUpdateOpt();
 
     void updateGeneratorButtons();
 
@@ -254,7 +254,7 @@ DIALOG_EXPORT_NETLIST::DIALOG_EXPORT_NETLIST( SCH_EDIT_FRAME* parent ) :
 
     SCHEMATIC_SETTINGS& settings = m_Parent->Schematic().Settings();
 
-    for( EXPORT_NETLIST_PAGE*& page : m_PanelNetType)
+    for( EXPORT_NETLIST_PAGE*& page : m_PanelNetType )
         page = nullptr;
 
     // Add notebook pages:
@@ -463,22 +463,35 @@ void DIALOG_EXPORT_NETLIST::OnNetlistTypeSelection( wxNotebookEvent& event )
 }
 
 
-void DIALOG_EXPORT_NETLIST::NetlistUpdateOpt()
+bool DIALOG_EXPORT_NETLIST::NetlistUpdateOpt()
 {
+    bool changed = false;
+
     bool saveAllVoltages = m_PanelNetType[ PANELSPICE ]->m_SaveAllVoltages->IsChecked();
     bool saveAllCurrents = m_PanelNetType[ PANELSPICE ]->m_SaveAllCurrents->IsChecked();
+
     wxString spiceCmdString = m_PanelNetType[ PANELSPICE ]->m_CommandStringCtrl->GetValue();
     bool curSheetAsRoot = m_PanelNetType[ PANELSPICE ]->m_CurSheetAsRoot->GetValue();
     bool spiceModelCurSheetAsRoot = m_PanelNetType[ PANELSPICEMODEL ]->m_CurSheetAsRoot->GetValue();
 
     SCHEMATIC_SETTINGS& settings = m_Parent->Schematic().Settings();
+    wxString netFormatName = m_PanelNetType[m_NoteBook->GetSelection()]->GetPageNetFmtName();
+
+    changed |= ( settings.m_SpiceSaveAllVoltages != saveAllVoltages );
+    changed |= ( settings.m_SpiceSaveAllCurrents != saveAllCurrents );
+    changed |= ( settings.m_SpiceCommandString != spiceCmdString );
+    changed |= ( settings.m_SpiceCurSheetAsRoot != curSheetAsRoot );
+    changed |= ( settings.m_SpiceModelCurSheetAsRoot != spiceModelCurSheetAsRoot );
+    changed |= ( settings.m_NetFormatName != netFormatName );
 
     settings.m_SpiceSaveAllVoltages     = saveAllVoltages;
     settings.m_SpiceSaveAllCurrents     = saveAllCurrents;
     settings.m_SpiceCommandString       = spiceCmdString;
     settings.m_SpiceCurSheetAsRoot      = curSheetAsRoot;
     settings.m_SpiceModelCurSheetAsRoot = spiceModelCurSheetAsRoot;
-    settings.m_NetFormatName         = m_PanelNetType[m_NoteBook->GetSelection()]->GetPageNetFmtName();
+    settings.m_NetFormatName            = netFormatName;
+
+    return changed;
 }
 
 
@@ -489,7 +502,8 @@ bool DIALOG_EXPORT_NETLIST::TransferDataFromWindow()
     wxString    fileExt;
     wxString    title = _( "Save Netlist File" );
 
-    NetlistUpdateOpt();
+    if( NetlistUpdateOpt() )
+        m_Parent->OnModify();
 
     EXPORT_NETLIST_PAGE* currPage;
     currPage = (EXPORT_NETLIST_PAGE*) m_NoteBook->GetCurrentPage();
@@ -621,7 +635,8 @@ bool DIALOG_EXPORT_NETLIST::FilenamePrms( NETLIST_TYPE_ID aType, wxString * aExt
 
 void DIALOG_EXPORT_NETLIST::WriteCurrentNetlistSetup()
 {
-    NetlistUpdateOpt();
+    if( NetlistUpdateOpt() )
+        m_Parent->OnModify();
 
     EESCHEMA_SETTINGS* cfg = dynamic_cast<EESCHEMA_SETTINGS*>( Kiface().KifaceSettings() );
     wxASSERT( cfg );
@@ -798,7 +813,7 @@ int InvokeDialogNetList( SCH_EDIT_FRAME* aCaller )
     DIALOG_EXPORT_NETLIST dlg( aCaller );
 
     int ret = dlg.ShowModal();
-    aCaller->SaveProjectSettings();
+    aCaller->SaveProjectLocalSettings();
 
     return ret;
 }

eeschema/eeschema_config.cpp

@@ -39,6 +39,7 @@
 #include <wildcards_and_files_ext.h>
 #include <drawing_sheet/ds_data_model.h>
 #include <zoom_defines.h>
+#include <sim/spice_settings.h>
 
 
 /// Helper for all the old plotting/printing code while it still exists
@@ -88,7 +89,8 @@ void SCH_EDIT_FRAME::ShowSchematicSetupDialog( const wxString& aInitialPage )
 
     if( dlg.ShowQuasiModal() == wxID_OK )
     {
-        SaveProjectSettings();
+        // Mark document as modified so that project settings can be saved as part of doc save
+        OnModify();
 
         Kiway().CommonSettingsChanged( false, true );
 
@@ -119,7 +121,7 @@ int SCH_EDIT_FRAME::GetSchematicJunctionSize()
 }
 
 
-void SCH_EDIT_FRAME::SaveProjectSettings()
+void SCH_EDIT_FRAME::saveProjectSettings()
 {
     wxFileName fn = Schematic().RootScreen()->GetFileName();  //ConfigFileName
 
@@ -130,6 +132,8 @@ void SCH_EDIT_FRAME::SaveProjectSettings()
 
     RecordERCExclusions();
 
+    if( Kiway().Player( FRAME_SIMULATOR, false ) )
+        Prj().GetProjectFile().m_SchematicSettings->m_NgspiceSimulatorSettings->SaveToFile();
 
     // Save the page layout file if doesn't exist yet (e.g. if we opened a non-kicad schematic)
 
@@ -158,6 +162,12 @@ void SCH_EDIT_FRAME::SaveProjectSettings()
 }
 
 
+void SCH_EDIT_FRAME::SaveProjectLocalSettings()
+{
+    // No schematic local settings yet
+}
+
+
 void SCH_EDIT_FRAME::LoadSettings( APP_SETTINGS_BASE* aCfg )
 {
     // For now, axes are forced off in Eeschema even if turned on in config

eeschema/sch_edit_frame.cpp

@@ -822,8 +822,6 @@ void SCH_EDIT_FRAME::doCloseWindow()
     if( m_toolManager )
         m_toolManager->ShutdownAllTools();
 
-    RecordERCExclusions();
-
     // Close the find dialog and preserve its setting if it is displayed.
     if( m_findReplaceDialog )
     {
@@ -842,9 +840,6 @@ void SCH_EDIT_FRAME::doCloseWindow()
         m_auimgr.Update();
     }
 
-    if( Kiway().Player( FRAME_SIMULATOR, false ) )
-        Prj().GetProjectFile().m_SchematicSettings->m_NgspiceSimulatorSettings->SaveToFile();
-
     SCH_SCREENS screens( Schematic().Root() );
     wxFileName fn;
 
@@ -873,7 +868,7 @@ void SCH_EDIT_FRAME::doCloseWindow()
         UpdateFileHistory( fileName );
 
     // Make sure local settings are persisted
-    SaveProjectSettings();
+    SaveProjectLocalSettings();
 
     Schematic().RootScreen()->Clear();
 

eeschema/sch_edit_frame.h

@@ -114,7 +114,7 @@ public:
     /**
      * Save changes to the project settings to the project (.pro) file.
      */
-    void SaveProjectSettings() override;
+    void SaveProjectLocalSettings() override;
 
     /**
      * Load the KiCad project file (*.pro) settings specific to Eeschema.
@@ -836,6 +836,8 @@ protected:
 
     void onSize( wxSizeEvent& aEvent );
 
+    void saveProjectSettings() override;
+
 private:
     // Called when resizing the Hierarchy Navigator panel
     void OnResizeHierarchyNavigator( wxSizeEvent& aEvent );

eeschema/sim/sim_plot_frame.cpp

@@ -291,7 +291,7 @@ void SIM_PLOT_FRAME::SaveSettings( APP_SETTINGS_BASE* aCfg )
             project.m_SchematicSettings->m_NgspiceSimulatorSettings->SaveToFile();
 
         if( m_schematicFrame )
-            m_schematicFrame->SaveProjectSettings();
+            m_schematicFrame->OnModify();
     }
 }
 

eeschema/tools/sch_editor_control.cpp

@@ -277,12 +277,6 @@ int SCH_EDITOR_CONTROL::RemapSymbols( const TOOL_EVENT& aEvent )
 int SCH_EDITOR_CONTROL::Print( const TOOL_EVENT& aEvent )
 {
     InvokeDialogPrintUsingPrinter( m_frame );
-
-    wxFileName fn = m_frame->Prj().AbsolutePath( m_frame->Schematic().RootScreen()->GetFileName() );
-
-    if( fn.GetName() != NAMELESS_PROJECT )
-        m_frame->SaveProjectSettings();
-
     return 0;
 }
 
@@ -295,7 +289,7 @@ int SCH_EDITOR_CONTROL::Plot( const TOOL_EVENT& aEvent )
 
     // save project config if the prj config has changed:
     if( dlg.PrjConfigChanged() )
-        m_frame->SaveProjectSettings();
+        m_frame->OnModify();
 
     return 0;
 }

include/eda_base_frame.h

@@ -354,14 +354,16 @@ public:
     }
 
     /**
-     * Save changes to the project settings to the project (.pro) file.
+     * Save changes to the project local settings.  These settings are used to save/restore the
+     * view state for a specific project, and should never contain design data.  This method is
+     * normally called automatically at various points in the workflow so that the user's most
+     * recent display settings are automatically persisted.
      *
      * The method is virtual so you can override it to call the suitable save method.
      * The base method does nothing.
      *
-     * @param aAskForSave true to open a dialog before saving the settings.
      */
-    virtual void SaveProjectSettings() {};
+    virtual void SaveProjectLocalSettings() {};
 
     /**
      * Prompt the user for a hotkey file to read, and read it.
@@ -663,6 +665,13 @@ protected:
 
     void ensureWindowIsOnScreen();
 
+    /**
+     * Saves any design-related project settings associated with this frame.
+     * This method should only be called as the result of direct user action, for example from an
+     * explicit "Save Project" command or as a consequence of saving a design document.
+     */
+    virtual void saveProjectSettings() {}
+
     /**
      * Handles event fired when a file is dropped to the window.
      * In this base class, stores the path of files accepted.

pcbnew/files.cpp

@@ -398,13 +398,11 @@ bool PCB_EDIT_FRAME::Files_io_from_id( int id )
                 return false;
         }
 
-        SaveProjectSettings();
+        SaveProjectLocalSettings();
 
         GetBoard()->ClearProject();
 
         SETTINGS_MANAGER* mgr = GetSettingsManager();
-
-        mgr->SaveProject( mgr->Prj().GetProjectFullName() );
         mgr->UnloadProject( &mgr->Prj() );
 
         if( !Clear_Pcb( false ) )
@@ -670,7 +668,7 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, in
     if( pro.GetFullPath() != mgr->Prj().GetProjectFullName() )
     {
         // calls SaveProject
-        SaveProjectSettings();
+        SaveProjectLocalSettings();
 
         GetBoard()->ClearProject();
         mgr->UnloadProject( &mgr->Prj() );
@@ -1071,7 +1069,7 @@ bool PCB_EDIT_FRAME::SavePcbFile( const wxString& aFileName, bool addToHistory,
     {
         // Save various DRC parameters, such as violation severities (which may have been
         // edited via the DRC dialog as well as the Board Setup dialog), DRC exclusions, etc.
-        SaveProjectSettings();
+        saveProjectSettings();
 
         GetBoard()->SynchronizeProperties();
         GetBoard()->SynchronizeNetsAndNetClasses( false );
@@ -1177,7 +1175,7 @@ bool PCB_EDIT_FRAME::SavePcbCopy( const wxString& aFileName, bool aCreateProject
 
     // Save various DRC parameters, such as violation severities (which may have been
     // edited via the DRC dialog as well as the Board Setup dialog), DRC exclusions, etc.
-    SaveProjectSettings();
+    SaveProjectLocalSettings();
 
     GetBoard()->SynchronizeNetsAndNetClasses( false );
 

pcbnew/netlist_reader/board_netlist_updater.cpp

@@ -1054,7 +1054,10 @@ bool BOARD_NETLIST_UPDATER::UpdateNetlist( NETLIST& aNetlist )
         m_commit.Push( _( "Update netlist" ), m_newFootprintsCount ? ZONE_FILL_OP  : 0 );
 
         m_board->SynchronizeNetsAndNetClasses( true );
-        m_frame->SaveProjectSettings();
+
+        // Although m_commit will probably also set this, it's not guaranteed, and we need to make
+        // sure any modification to netclasses gets persisted to project settings through a save.
+        m_frame->OnModify();
     }
 
     if( m_isDryRun )

pcbnew/pcb_edit_frame.cpp

@@ -1102,7 +1102,7 @@ void PCB_EDIT_FRAME::doCloseWindow()
     }
 
     // Make sure local settings are persisted
-    SaveProjectSettings();
+    SaveProjectLocalSettings();
 
     // Do not show the layer manager during closing to avoid flicker
     // on some platforms (Windows) that generate useless redraw of items in
@@ -1149,7 +1149,8 @@ void PCB_EDIT_FRAME::ShowBoardSetupDialog( const wxString& aInitialPage )
     if( dlg.ShowQuasiModal() == wxID_OK )
     {
         GetBoard()->SynchronizeNetsAndNetClasses( true );
-        SaveProjectSettings();
+        // We don't know if anything was modified, so err on the side of requiring a save
+        OnModify();
 
         Kiway().CommonSettingsChanged( false, true );
 
@@ -1507,7 +1508,7 @@ void PCB_EDIT_FRAME::SetLastPath( LAST_PATH_TYPE aType, const wxString& aLastPat
     if( relativeFileName.GetFullPath() != project.m_PcbLastPath[ aType ] )
     {
         project.m_PcbLastPath[ aType ] = relativeFileName.GetFullPath();
-        SaveProjectSettings();
+        OnModify();
     }
 }
 

pcbnew/pcb_edit_frame.h

@@ -196,10 +196,7 @@ public:
      */
     static std::vector<ACTION_PLUGIN*> GetOrderedActionPlugins();
 
-    /**
+    void SaveProjectLocalSettings() override;
-     * Save changes to the project settings to the project (.pro) file.
-     */
-    void SaveProjectSettings() override;
 
     /**
      * Load the current project's file configuration settings which are pertinent
@@ -807,6 +804,8 @@ protected:
 
     void redrawNetnames( wxTimerEvent& aEvent );
 
+    void saveProjectSettings() override;
+
 public:
     PCB_LAYER_BOX_SELECTOR* m_SelLayerBox;  // a combo box to display and select active layer
 

pcbnew/pcbnew_config.cpp

@@ -128,7 +128,7 @@ bool PCB_EDIT_FRAME::LoadProjectSettings()
 }
 
 
-void PCB_EDIT_FRAME::SaveProjectSettings()
+void PCB_EDIT_FRAME::SaveProjectLocalSettings()
 {
     wxFileName fn = Prj().GetProjectFullName();
 
@@ -142,7 +142,6 @@ void PCB_EDIT_FRAME::SaveProjectSettings()
         return;
 
     PROJECT_FILE& project = Prj().GetProjectFile();
-    PROJECT_LOCAL_SETTINGS& localSettings = Prj().GetLocalSettings();
 
     // TODO: Can this be pulled out of BASE_SCREEN?
     project.m_BoardDrawingSheetFile = BASE_SCREEN::m_DrawingSheetFileName;
@@ -152,40 +151,12 @@ void PCB_EDIT_FRAME::SaveProjectSettings()
 
     RecordDRCExclusions();
 
-    // Save appearance control settings
-
-    localSettings.m_ActiveLayer       = GetActiveLayer();
-    localSettings.m_ActiveLayerPreset = m_appearancePanel->GetActiveLayerPreset();
-
-    const PCB_DISPLAY_OPTIONS& displayOpts = GetDisplayOptions();
-
-    localSettings.m_ContrastModeDisplay = displayOpts.m_ContrastModeDisplay;
-    localSettings.m_NetColorMode        = displayOpts.m_NetColorMode;
-    localSettings.m_TrackOpacity        = displayOpts.m_TrackOpacity;
-    localSettings.m_ViaOpacity          = displayOpts.m_ViaOpacity;
-    localSettings.m_PadOpacity          = displayOpts.m_PadOpacity;
-    localSettings.m_ZoneOpacity         = displayOpts.m_ZoneOpacity;
-    localSettings.m_ZoneDisplayMode     = displayOpts.m_ZoneDisplayMode;
-    localSettings.m_ImageOpacity        = displayOpts.m_ImageOpacity;
-
-    // Save Design settings
-    const BOARD_DESIGN_SETTINGS& bds = GetDesignSettings();
-    localSettings.m_AutoTrackWidth   = bds.m_UseConnectedTrackWidth;
-
     // Save render settings that aren't stored in PCB_DISPLAY_OPTIONS
 
     std::shared_ptr<NET_SETTINGS>& netSettings = project.NetSettings();
+    NETINFO_LIST&                  nets = GetBoard()->GetNetInfo();
     KIGFX::RENDER_SETTINGS*     rs = GetCanvas()->GetView()->GetPainter()->GetSettings();
     KIGFX::PCB_RENDER_SETTINGS* renderSettings = static_cast<KIGFX::PCB_RENDER_SETTINGS*>( rs );
-    NETINFO_LIST&                  nets = GetBoard()->GetNetInfo();
-
-    localSettings.m_HiddenNets.clear();
-
-    for( int netcode : renderSettings->GetHiddenNets() )
-    {
-        if( NETINFO_ITEM* net = nets.GetNetItem( netcode ) )
-            localSettings.m_HiddenNets.emplace_back( net->GetNetname() );
-    }
 
     netSettings->m_NetColorAssignments.clear();
 
@@ -204,11 +175,6 @@ void PCB_EDIT_FRAME::SaveProjectSettings()
             netclass->SetPcbColor( netclassColors.at( name ) );
     }
 
-    PCB_SELECTION_TOOL*       selTool = GetToolManager()->GetTool<PCB_SELECTION_TOOL>();
-    SELECTION_FILTER_OPTIONS& filterOpts = selTool->GetFilter();
-
-    localSettings.m_SelectionFilter = filterOpts;
-
     /**
      * The below automatically saves the project on exit, which is what we want to do if the project
      * already exists.  If the project doesn't already exist, we don't want to create it through
@@ -221,3 +187,57 @@ void PCB_EDIT_FRAME::SaveProjectSettings()
     if( !Prj().IsNullProject() && fn.Exists() )
         GetSettingsManager()->SaveProject();
 }
+
+
+void PCB_EDIT_FRAME::saveProjectSettings()
+{
+    wxFileName fn = Prj().GetProjectFullName();
+
+    // Check for the filename before checking IsWritable as this
+    // will throw errors on bad names.  Here, we just want to not
+    // save the Settings if we don't have a name
+    if( !fn.IsOk() )
+        return;
+
+    if( !fn.IsDirWritable() )
+        return;
+
+    PROJECT_LOCAL_SETTINGS& localSettings = Prj().GetLocalSettings();
+
+    // Save appearance control settings
+    localSettings.m_ActiveLayer       = GetActiveLayer();
+    localSettings.m_ActiveLayerPreset = m_appearancePanel->GetActiveLayerPreset();
+
+    const PCB_DISPLAY_OPTIONS& displayOpts = GetDisplayOptions();
+
+    localSettings.m_ContrastModeDisplay = displayOpts.m_ContrastModeDisplay;
+    localSettings.m_NetColorMode        = displayOpts.m_NetColorMode;
+    localSettings.m_TrackOpacity        = displayOpts.m_TrackOpacity;
+    localSettings.m_ViaOpacity          = displayOpts.m_ViaOpacity;
+    localSettings.m_PadOpacity          = displayOpts.m_PadOpacity;
+    localSettings.m_ZoneOpacity         = displayOpts.m_ZoneOpacity;
+    localSettings.m_ZoneDisplayMode     = displayOpts.m_ZoneDisplayMode;
+    localSettings.m_ImageOpacity        = displayOpts.m_ImageOpacity;
+
+    // Save Design settings
+    const BOARD_DESIGN_SETTINGS& bds = GetDesignSettings();
+    localSettings.m_AutoTrackWidth   = bds.m_UseConnectedTrackWidth;
+
+    // Net display settings
+    NETINFO_LIST&               nets = GetBoard()->GetNetInfo();
+    KIGFX::RENDER_SETTINGS*     rs = GetCanvas()->GetView()->GetPainter()->GetSettings();
+    KIGFX::PCB_RENDER_SETTINGS* renderSettings = static_cast<KIGFX::PCB_RENDER_SETTINGS*>( rs );
+
+    localSettings.m_HiddenNets.clear();
+
+    for( int netcode : renderSettings->GetHiddenNets() )
+    {
+        if( NETINFO_ITEM* net = nets.GetNetItem( netcode ) )
+            localSettings.m_HiddenNets.emplace_back( net->GetNetname() );
+    }
+
+    PCB_SELECTION_TOOL*       selTool = GetToolManager()->GetTool<PCB_SELECTION_TOOL>();
+    SELECTION_FILTER_OPTIONS& filterOpts = selTool->GetFilter();
+
+    localSettings.m_SelectionFilter = filterOpts;
+}