Project settings fixes from review

- Fix file extension for new project
- Fixes for exceptions on MSW
- Fix some ASAN issues
- Allow SETTINGS_MANAGER to run headless
- Don't flag schematic as modified after schematic setup is closed
- Don't automatically unload projects when LoadProject is called
- Don't unload project if it's the same as the current one
- Make sure to properly init/de-init template field names

common/common.cpp

@@ -53,6 +53,14 @@ static boost::uuids::nil_generator nilGenerator;
 KIID niluuid( 0 );
 
 
+// For static initialization
+KIID& NilUuid()
+{
+    static KIID nil( 0 );
+    return nil;
+}
+
+
 KIID::KIID() :
         m_uuid( randomGenerator() ),
         m_cached_timestamp( 0 )

common/env_paths.cpp

@@ -177,3 +177,33 @@ wxString ResolveFile( const wxString& aFileName, const ENV_VAR_MAP* aEnvVars,
 
     return wxEmptyString;
 }
+
+
+bool PathIsInsideProject( const wxString& aFileName, const PROJECT* aProject, wxFileName* aSubPath )
+{
+    wxFileName fn( aFileName );
+    wxFileName prj( aProject->GetProjectPath() );
+
+    wxArrayString pdirs = prj.GetDirs();
+    wxArrayString fdirs = fn.GetDirs();
+
+    if( fdirs.size() < pdirs.size() )
+        return false;
+
+    for( size_t i = 0; i < pdirs.size(); i++ )
+    {
+        if( fdirs[i] != pdirs[i] )
+            return false;
+    }
+
+    // Now we know that fn is inside prj
+    if( aSubPath )
+    {
+        aSubPath->Clear();
+
+        for( size_t i = pdirs.size(); i < fdirs.size(); i++ )
+            aSubPath->AppendDir( fdirs[i] );
+    }
+
+    return true;
+}

common/marker_base.cpp

@@ -85,6 +85,7 @@ MARKER_BASE::MARKER_BASE( int aScalingFactor, RC_ITEM* aItem, TYPEMARKER aType )
 
 MARKER_BASE::~MARKER_BASE()
 {
+    delete m_rcItem;
 }
 
 

common/rc_item.h

@@ -89,14 +89,14 @@ protected:
 
 public:
 
-    RC_ITEM()
+    RC_ITEM() :
+        m_errorCode( 0 ),
+        m_parent( nullptr ),
+        m_mainItemUuid( NilUuid() ),
+        m_auxItemUuid( NilUuid() ),
+        m_auxItem2Uuid( NilUuid() ),
+        m_auxItem3Uuid( NilUuid() )
     {
-        m_errorCode     = 0;
-        m_parent        = nullptr;
-        m_mainItemUuid  = niluuid;
-        m_auxItemUuid   = niluuid;
-        m_auxItem2Uuid   = niluuid;
-        m_auxItem3Uuid   = niluuid;
     }
 
     RC_ITEM( RC_ITEM* aItem )

common/settings/nested_settings.cpp

@@ -48,19 +48,24 @@ bool NESTED_SETTINGS::LoadFromFile( const std::string& aDirectory )
 
     if( m_parent )
     {
-        try
-        {
-            update( ( *m_parent )[PointerFromString( m_path )] );
-
-            wxLogTrace( traceSettings, "Loaded NESTED_SETTINGS %s with schema %d", GetFilename(),
-                    m_schemaVersion );
+        nlohmann::json::json_pointer ptr = PointerFromString( m_path );
 
-            success = true;
-        }
-        catch( ... )
+        if( m_parent->contains( ptr ) )
         {
-            wxLogTrace( traceSettings, "NESTED_SETTINGS %s: Could not load from %s at %s",
-                    m_filename, m_parent->GetFilename(), m_path );
+            try
+            {
+                update( ( *m_parent )[ptr] );
+
+                wxLogTrace( traceSettings, "Loaded NESTED_SETTINGS %s with schema %d",
+                        GetFilename(), m_schemaVersion );
+
+                success = true;
+            }
+            catch( ... )
+            {
+                wxLogTrace( traceSettings, "NESTED_SETTINGS %s: Could not load from %s at %s",
+                        m_filename, m_parent->GetFilename(), m_path );
+            }
         }
     }
 

common/settings/settings_manager.cpp

@@ -50,7 +50,8 @@ const char* traceSettings = "SETTINGS";
 #define PROJECT_BACKUPS_DIR_SUFFIX wxT( "-backups" )
 
 
-SETTINGS_MANAGER::SETTINGS_MANAGER() :
+SETTINGS_MANAGER::SETTINGS_MANAGER( bool aHeadless ) :
+        m_headless( aHeadless ),
         m_common_settings( nullptr ),
         m_migration_source()
 {
@@ -427,6 +428,12 @@ bool SETTINGS_MANAGER::MigrateIfNeeded()
         }
     }
 
+    if( m_headless )
+    {
+        wxLogTrace( traceSettings, "Manual settings migration required but running headless!" );
+        return false;
+    }
+
     // Now we have an empty path, let's figure out what to put in it
     DIALOG_MIGRATE_SETTINGS dlg( this );
 
@@ -680,9 +687,11 @@ bool SETTINGS_MANAGER::LoadProject( const wxString& aFullPath, bool aSetActive )
 
     wxString fullPath = path.GetFullPath();
 
-    // Unload if already loaded
-    if( m_projects.count( fullPath ) && !UnloadProject( m_projects.at( fullPath ).get() ) )
-        return false;
+    // If already loaded, we are all set.  This might be called more than once over a project's
+    // lifetime in case the project is first loaded by the KiCad manager and then eeschema or
+    // pcbnew try to load it again when they are launched.
+    if( m_projects.count( fullPath ) )
+        return true;
 
     // No MDI yet
     if( aSetActive && !m_projects.empty() && !UnloadProject( m_projects.begin()->second.get() ) )

eeschema/eeschema_config.cpp

@@ -279,7 +279,6 @@ void SCH_EDIT_FRAME::ShowSchematicSetupDialog( const wxString& aInitialPage )
         SaveProjectSettings();
 
         GetCanvas()->Refresh();
-        OnModify();
     }
 }
 

eeschema/files-io.cpp

@@ -27,6 +27,7 @@
 #include <fctsys.h>
 #include <sch_draw_panel.h>
 #include <confirm.h>
+#include <env_paths.h>
 #include <gestfich.h>
 #include <sch_edit_frame.h>
 #include <pgm_base.h>
@@ -282,6 +283,9 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, in
         if( pro.GetFullPath() != Prj().GetProjectFullName()
           || !Prj().GetElem( PROJECT::ELEM_SCH_PART_LIBS ) )
         {
+            GetSettingsManager()->SaveProject();
+            GetSettingsManager()->UnloadProject( &Prj() );
+
             GetSettingsManager()->LoadProject( pro.GetFullPath() );
 
             // load the libraries here, not in SCH_SCREEN::Draw() which is a context

eeschema/sch_edit_frame.cpp

@@ -45,11 +45,13 @@
 #include <pgm_base.h>
 #include <profile.h>
 #include <project.h>
+#include <project/project_file.h>
 #include <reporter.h>
 #include <sch_edit_frame.h>
 #include <sch_painter.h>
 #include <sch_sheet.h>
 #include <schematic.h>
+#include <settings/settings_manager.h>
 #include <advanced_config.h>
 #include <sim/sim_plot_frame.h>
 #include <symbol_lib_table.h>
@@ -209,9 +211,9 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ):
     m_highlightedConn( nullptr ),
     m_item_to_repeat( nullptr )
 {
-    m_schematic = new SCHEMATIC( &Prj() );
+    m_schematic = new SCHEMATIC( nullptr );
 
-    m_defaults = &m_schematic->Settings();
+    Prj().GetProjectFile().m_TemplateFieldNames = &m_templateFieldNames;
 
     m_showBorderAndTitleBlock = true;   // true to show sheet references
     m_hasAutoSave = true;
@@ -228,8 +230,13 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ):
 
     LoadSettings( eeconfig() );
 
+    // Also links the schematic to the loaded project
     CreateScreens();
 
+    // After schematic has been linked to project, SCHEMATIC_SETTINGS works
+    m_defaults = &m_schematic->Settings();
+    LoadProjectSettings();
+
     setupTools();
     ReCreateMenuBar();
     ReCreateHToolbar();
@@ -295,6 +302,9 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ):
 
 SCH_EDIT_FRAME::~SCH_EDIT_FRAME()
 {
+    GetSettingsManager()->SaveProject();
+    Prj().GetProjectFile().m_TemplateFieldNames = nullptr;
+
     // Shutdown all running tools
     if( m_toolManager )
         m_toolManager->ShutdownAllTools();

eeschema/schematic_settings.h

@@ -33,8 +33,9 @@
  * These are loaded from eeschema settings but then overwritten by the project settings.
  * All of the values are stored in IU, but the backing file stores in mils.
  */
-struct SCHEMATIC_SETTINGS : public NESTED_SETTINGS
+class SCHEMATIC_SETTINGS : public NESTED_SETTINGS
 {
+public:
     SCHEMATIC_SETTINGS( JSON_SETTINGS* aParent, const std::string& aPath );
 
     virtual ~SCHEMATIC_SETTINGS();

include/common.h

@@ -114,6 +114,8 @@ private:
 
 extern KIID niluuid;
 
+KIID& NilUuid();
+
 // declare KIID_VECT_LIST as std::vector<KIID> both for c++ and swig:
 DECL_VEC_FOR_SWIG( KIID_VECT_LIST, KIID )
 

include/env_paths.h

@@ -63,4 +63,14 @@ wxString NormalizePath(
 wxString ResolveFile(
         const wxString& aFileName, const ENV_VAR_MAP* aEnvVars, const PROJECT* aProject );
 
+/**
+ * Checks if a given filename is within a given project directory (not whether it exists!)
+ * @param aFileName is the absolute path to check
+ * @param aProject is the project to test agains
+ * @param aSubPath will be filled with the relative path to the file inside the project (if any)
+ * @return true if aFileName's path is inside aProject's path
+ */
+bool PathIsInsideProject( const wxString& aFileName, const PROJECT* aProject,
+                          wxFileName* aSubPath = nullptr );
+
 #endif /* ENV_PATHS_H */

include/settings/settings_manager.h

@@ -33,7 +33,7 @@ class PROJECT_FILE;
 class SETTINGS_MANAGER
 {
 public:
-    SETTINGS_MANAGER();
+    SETTINGS_MANAGER( bool aHeadless = false );
 
     ~SETTINGS_MANAGER();
 
@@ -313,6 +313,9 @@ private:
 
 private:
 
+    /// True if running outside a UI context
+    bool m_headless;
+
     std::vector<std::unique_ptr<JSON_SETTINGS>> m_settings;
 
     std::unordered_map<wxString, COLOR_SETTINGS*> m_color_settings;

kicad/kicad_manager_frame.cpp

@@ -334,7 +334,10 @@ void KICAD_MANAGER_FRAME::LoadProject( const wxFileName& aProjectFileName )
 
     // Save the project file for the currently loaded project.
     if( m_active_project )
+    {
         Pgm().GetSettingsManager().SaveProject();
+        Pgm().GetSettingsManager().UnloadProject( &Prj() );
+    }
 
     m_active_project = true;
     ClearMsg();

kicad/tools/kicad_manager_control.cpp

@@ -95,10 +95,10 @@ int KICAD_MANAGER_CONTROL::NewProject( const TOOL_EVENT& aEvent )
     // wxFileName automatically extracts an extension.  But if it isn't
     // a .pro extension, we should keep it as part of the filename
     if( !pro.GetExt().IsEmpty()
-        && pro.GetExt().ToStdString() != LegacyProjectFileExtension )
+        && pro.GetExt().ToStdString() != ProjectFileExtension )
         pro.SetName( pro.GetName() + wxT( "." ) + pro.GetExt() );
 
-    pro.SetExt( LegacyProjectFileExtension );     // enforce extension
+    pro.SetExt( ProjectFileExtension );     // enforce extension
 
     if( !pro.IsAbsolute() )
         pro.MakeAbsolute();

pcbnew/files.cpp

@@ -298,13 +298,17 @@ bool PCB_EDIT_FRAME::Files_io_from_id( int id )
                 return false;
         }
 
-        GetSettingsManager()->SaveProject( GetSettingsManager()->Prj().GetProjectFullName() );
+        SETTINGS_MANAGER* mgr = GetSettingsManager();
+
+        mgr->SaveProject( mgr->Prj().GetProjectFullName() );
+        mgr->UnloadProject( &mgr->Prj() );
+
         GetBoard()->ClearProject();
 
         wxFileName fn( wxStandardPaths::Get().GetDocumentsDir(), wxT( "noname" ),
                        ProjectFileExtension );
 
-        GetSettingsManager()->LoadProject( fn.GetFullPath() );
+        mgr->LoadProject( fn.GetFullPath() );
 
         LoadProjectSettings();
 
@@ -509,7 +513,15 @@ bool PCB_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, in
         // The calling code should know not to ask me here to change projects unless
         // it knows what consequences that will have on other KIFACEs running and using
         // this same PROJECT.  It can be very harmful if that calling code is stupid.
-        GetSettingsManager()->LoadProject( pro.GetFullPath() );
+        SETTINGS_MANAGER* mgr = GetSettingsManager();
+
+        if( pro.GetFullPath() != mgr->Prj().GetProjectFullName() )
+        {
+            mgr->SaveProject( mgr->Prj().GetProjectFullName() );
+            mgr->UnloadProject( &mgr->Prj() );
+
+            mgr->LoadProject( pro.GetFullPath() );
+        }
 
         // load project settings before BOARD
         LoadProjectSettings();
@@ -691,7 +703,13 @@ bool PCB_EDIT_FRAME::SavePcbFile( const wxString& aFileName, bool addToHistory )
         if( projectFile.GetFullPath() != Prj().GetProjectFullName() )
         {
             GetBoard()->ClearProject();
-            GetSettingsManager()->LoadProject( projectFile.GetFullPath() );
+
+            SETTINGS_MANAGER* mgr = GetSettingsManager();
+
+            mgr->SaveProject( Prj().GetProjectFullName() );
+            mgr->UnloadProject( &Prj() );
+
+            mgr->LoadProject( projectFile.GetFullPath() );
             GetBoard()->SetProject( &Prj() );
         }
     }

pcbnew/swig/pcbnew_scripting_helpers.cpp

@@ -47,12 +47,6 @@
 
 static PCB_EDIT_FRAME* s_PcbEditFrame = NULL;
 
-/**
- * We need to track the loaded PROJECTs for each loaded BOARD here, since in Python you can
- * easily load more than one board if desired.
- */
-static std::map<wxString, PROJECT*> s_Projects;
-
 static SETTINGS_MANAGER* s_SettingsManager = nullptr;
 
 BOARD* GetBoard()
@@ -86,7 +80,7 @@ BOARD* LoadBoard( wxString& aFileName )
 SETTINGS_MANAGER* GetSettingsManager()
 {
     if( !s_SettingsManager )
-        s_SettingsManager = new SETTINGS_MANAGER;
+        s_SettingsManager = new SETTINGS_MANAGER( true );
 
     return s_SettingsManager;
 }
@@ -94,15 +88,12 @@ SETTINGS_MANAGER* GetSettingsManager()
 
 PROJECT* GetDefaultProject()
 {
-    PROJECT* project = nullptr;
+    PROJECT* project = GetSettingsManager()->GetProject( "" );
 
-    if( s_Projects.count( "" ) )
-        project = s_Projects.at( "" );
-    else
+    if( !project )
     {
         GetSettingsManager()->LoadProject( "" );
         project = GetSettingsManager()->GetProject( "" );
-        s_Projects[""] = project;
     }
 
     return project;
@@ -116,14 +107,12 @@ BOARD* LoadBoard( wxString& aFileName, IO_MGR::PCB_FILE_T aFormat )
     pro.MakeAbsolute();
     wxString projectPath = pro.GetFullPath();
 
-    PROJECT* project = nullptr;
+    PROJECT* project = GetSettingsManager()->GetProject( projectPath );
 
-    if( s_Projects.count( projectPath ) )
-        project = s_Projects.at( projectPath );
-    else if( GetSettingsManager()->LoadProject( projectPath ) )
+    if( !project )
     {
-        project = GetSettingsManager()->GetProject( projectPath );
-        s_Projects[projectPath] = project;
+        GetSettingsManager()->LoadProject( projectPath );
+        GetSettingsManager()->GetProject( projectPath );
     }
 
     // Board cannot be loaded without a project, so create the default project

qa/eeschema/sim/test_netlist_exporter_pspice_sim.cpp

@@ -42,6 +42,7 @@ class TEST_NETLIST_EXPORTER_PSPICE_SIM
 {
 public:
     TEST_NETLIST_EXPORTER_PSPICE_SIM() :
+            m_manager( true ),
             m_schematic( nullptr ),
             m_exporter( &m_schematic )
     {

qa/eeschema/test_netlists.cpp

@@ -36,7 +36,8 @@ class TEST_NETLISTS_FIXTURE
 {
 public:
     TEST_NETLISTS_FIXTURE() :
-            m_schematic( nullptr )
+            m_schematic( nullptr ),
+            m_manager( true )
     {
         m_pi = SCH_IO_MGR::FindPlugin( SCH_IO_MGR::SCH_KICAD );
     }