Fix several bugs and refactor schematic saving

Having the logic split between three different paths was
a bit much, this should hopefully be a bit easier to
maintain.  All save logic now goes through SaveProject;
individual file writing is now a private function.

Also change Save As to Save Copy As in project manager
mode, as we don't want to change projects from inside
the editor in project manager mode.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/9015

eeschema/files-io.cpp

@@ -73,7 +73,7 @@ public:
     {
         m_cbCreateProject = new wxCheckBox( this, wxID_ANY,
                                             _( "Create a new project for this schematic" ) );
-        m_cbCreateProject->SetValue( false );
+        m_cbCreateProject->SetValue( true );
         m_cbCreateProject->SetToolTip( _( "Creating a project will enable features such as "
                                           "text variables, net classes, and ERC exclusions" ) );
 
@@ -98,202 +98,6 @@ protected:
 };
 
 
-bool SCH_EDIT_FRAME::SaveEEFile( SCH_SHEET* aSheet, bool aSaveUnderNewName, bool* aCreateProject )
-{
-    wxString msg;
-    wxFileName schematicFileName;
-    wxFileName oldFileName;
-    bool success;
-
-    if( aSheet == nullptr )
-        aSheet = GetCurrentSheet().Last();
-
-    SCH_SCREEN* screen = aSheet->GetScreen();
-
-    wxCHECK( screen, false );
-
-    // If no name exists in the window yet - save as new.
-    if( screen->GetFileName().IsEmpty() )
-        aSaveUnderNewName = true;
-
-    // Construct the name of the file to be saved
-    schematicFileName = Prj().AbsolutePath( screen->GetFileName() );
-    oldFileName = schematicFileName;
-
-    if( aSaveUnderNewName )
-    {
-        wxFileName savePath( Prj().GetProjectFullName() );
-
-        if( !savePath.IsOk() || !savePath.IsDirWritable() )
-        {
-            savePath = GetMruPath();
-
-            if( !savePath.IsOk() || !savePath.IsDirWritable() )
-                savePath = PATHS::GetDefaultUserProjectsPath();
-        }
-
-        wxFileDialog dlg( this, _( "Schematic Files" ), savePath.GetPath(),
-                          schematicFileName.GetFullName(), KiCadSchematicFileWildcard(),
-                          wxFD_SAVE | wxFD_OVERWRITE_PROMPT );
-
-        // Add a "Create a project" checkbox in standalone mode and one isn't loaded
-        dlg.SetExtraControlCreator( &CREATE_PROJECT_CHECKBOX::Create );
-
-        if( dlg.ShowModal() == wxID_CANCEL )
-            return false;
-
-        schematicFileName = dlg.GetPath();
-
-        if( schematicFileName.GetExt().IsEmpty() )
-            schematicFileName.SetExt( KiCadSchematicFileExtension );
-
-        wxWindow* extraControl = dlg.GetExtraControl();
-
-        if( aCreateProject )
-        {
-            wxASSERT( extraControl );
-            *aCreateProject = static_cast<CREATE_PROJECT_CHECKBOX*>( extraControl )->GetValue();
-        }
-    }
-
-    // Write through symlinks, don't replace them
-    WX_FILENAME::ResolvePossibleSymlinks( schematicFileName );
-
-    if( !IsWritable( schematicFileName ) )
-        return false;
-
-    // This is a new schematic file so make sure it has a unique ID.
-    if( aSaveUnderNewName && schematicFileName != oldFileName )
-        screen->AssignNewUuid();
-
-    wxFileName tempFile( schematicFileName );
-    tempFile.SetName( wxT( "." ) + tempFile.GetName() );
-    tempFile.SetExt( tempFile.GetExt() + wxT( "$" ) );
-
-    // Save
-    wxLogTrace( traceAutoSave, "Saving file " + schematicFileName.GetFullPath() );
-
-    SCH_IO_MGR::SCH_FILE_T pluginType = SCH_IO_MGR::GuessPluginTypeFromSchPath(
-            schematicFileName.GetFullPath() );
-    SCH_PLUGIN::SCH_PLUGIN_RELEASER pi( SCH_IO_MGR::FindPlugin( pluginType ) );
-
-    try
-    {
-        pi->Save( tempFile.GetFullPath(), aSheet, &Schematic() );
-        success = true;
-    }
-    catch( const IO_ERROR& ioe )
-    {
-        msg.Printf( _( "Error saving schematic file '%s'.\n%s" ),
-                    schematicFileName.GetFullPath(),
-                    ioe.What() );
-        DisplayError( this, msg );
-
-        msg.Printf( _( "Failed to create temporary file '%s'." ),
-                    tempFile.GetFullPath() );
-        SetMsgPanel( wxEmptyString, msg );
-
-        // In case we started a file but didn't fully write it, clean up
-        wxRemoveFile( tempFile.GetFullPath() );
-
-        success = false;
-    }
-
-    if( success )
-    {
-        // Replace the original with the temporary file we just wrote
-        success = wxRenameFile( tempFile.GetFullPath(), schematicFileName.GetFullPath() );
-
-        if( !success )
-        {
-            msg.Printf( _( "Error saving schematic file '%s'.\n"
-                           "Failed to rename temporary file '%s'." ),
-                        schematicFileName.GetFullPath(),
-                        tempFile.GetFullPath() );
-            DisplayError( this, msg );
-
-            msg.Printf( _( "Failed to rename temporary file '%s'." ),
-                        tempFile.GetFullPath() );
-            SetMsgPanel( wxEmptyString, msg );
-        }
-    }
-
-    if( success )
-    {
-        // Delete auto save file.
-        wxFileName autoSaveFileName = schematicFileName;
-        autoSaveFileName.SetName( GetAutoSaveFilePrefix() + schematicFileName.GetName() );
-
-        if( autoSaveFileName.FileExists() )
-        {
-            wxLogTrace( traceAutoSave,
-                        wxT( "Removing auto save file <" ) + autoSaveFileName.GetFullPath() +
-                        wxT( ">" ) );
-
-            wxRemoveFile( autoSaveFileName.GetFullPath() );
-        }
-
-        // Update the screen and frame info and reset the lock file.
-        if( aSaveUnderNewName )
-        {
-            screen->SetFileName( schematicFileName.GetFullPath() );
-            aSheet->SetFileName( schematicFileName.GetFullPath() );
-            LockFile( schematicFileName.GetFullPath() );
-
-            UpdateFileHistory( schematicFileName.GetFullPath() );
-        }
-
-        screen->SetContentModified( false );
-        UpdateTitle();
-
-        msg.Printf( _( "File '%s' saved." ),  screen->GetFileName() );
-        SetStatusText( msg, 0 );
-    }
-    else
-    {
-        DisplayError( this, _( "File write operation failed." ) );
-    }
-
-    return success;
-}
-
-
-void SCH_EDIT_FRAME::Save_File( bool doSaveAs )
-{
-    if( doSaveAs )
-    {
-        bool createProject = false;
-
-        if( SaveEEFile( nullptr, true, &createProject ) )
-        {
-            SCH_SCREEN* screen = GetScreen();
-
-            wxCHECK( screen, /* void */ );
-
-            wxFileName fn = screen->GetFileName();
-
-            if( fn.GetExt() == LegacySchematicFileExtension )
-                CreateArchiveLibraryCacheFile( true );
-
-            // If we are saving under a new name, and don't have a real project yet, create one
-            fn.SetExt( ProjectFileExtension );
-
-            if( fn.IsDirWritable() && !fn.FileExists() )
-            {
-                Prj().SetReadOnly( !createProject );
-                GetSettingsManager()->SaveProjectAs( fn.GetFullPath() );
-            }
-        }
-    }
-    else
-    {
-        SaveEEFile( nullptr );
-    }
-
-    UpdateTitle();
-}
-
-
 bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, int aCtl )
 {
     // implement the pseudo code from KIWAY_PLAYER.h:
@@ -826,11 +630,114 @@ void SCH_EDIT_FRAME::OnImportProject( wxCommandEvent& aEvent )
 }
 
 
-bool SCH_EDIT_FRAME::SaveProject()
+bool SCH_EDIT_FRAME::saveSchematicFile( SCH_SHEET* aSheet, const wxString& aSavePath )
+{
+    wxString msg;
+    wxFileName schematicFileName;
+    wxFileName oldFileName;
+    bool success;
+
+    SCH_SCREEN* screen = aSheet->GetScreen();
+
+    wxCHECK( screen, false );
+
+    // Construct the name of the file to be saved
+    schematicFileName = Prj().AbsolutePath( aSavePath );
+    oldFileName = schematicFileName;
+
+    // Write through symlinks, don't replace them
+    WX_FILENAME::ResolvePossibleSymlinks( schematicFileName );
+
+    if( !IsWritable( schematicFileName ) )
+        return false;
+
+    wxFileName tempFile( schematicFileName );
+    tempFile.SetName( wxT( "." ) + tempFile.GetName() );
+    tempFile.SetExt( tempFile.GetExt() + wxT( "$" ) );
+
+    // Save
+    wxLogTrace( traceAutoSave, "Saving file " + schematicFileName.GetFullPath() );
+
+    SCH_IO_MGR::SCH_FILE_T pluginType = SCH_IO_MGR::GuessPluginTypeFromSchPath(
+            schematicFileName.GetFullPath() );
+    SCH_PLUGIN::SCH_PLUGIN_RELEASER pi( SCH_IO_MGR::FindPlugin( pluginType ) );
+
+    try
+    {
+        pi->Save( tempFile.GetFullPath(), aSheet, &Schematic() );
+        success = true;
+    }
+    catch( const IO_ERROR& ioe )
+    {
+        msg.Printf( _( "Error saving schematic file '%s'.\n%s" ),
+                    schematicFileName.GetFullPath(),
+                    ioe.What() );
+        DisplayError( this, msg );
+
+        msg.Printf( _( "Failed to create temporary file '%s'." ),
+                    tempFile.GetFullPath() );
+        SetMsgPanel( wxEmptyString, msg );
+
+        // In case we started a file but didn't fully write it, clean up
+        wxRemoveFile( tempFile.GetFullPath() );
+
+        success = false;
+    }
+
+    if( success )
+    {
+        // Replace the original with the temporary file we just wrote
+        success = wxRenameFile( tempFile.GetFullPath(), schematicFileName.GetFullPath() );
+
+        if( !success )
+        {
+            msg.Printf( _( "Error saving schematic file '%s'.\n"
+                           "Failed to rename temporary file '%s'." ),
+                        schematicFileName.GetFullPath(),
+                        tempFile.GetFullPath() );
+            DisplayError( this, msg );
+
+            msg.Printf( _( "Failed to rename temporary file '%s'." ),
+                        tempFile.GetFullPath() );
+            SetMsgPanel( wxEmptyString, msg );
+        }
+    }
+
+    if( success )
+    {
+        // Delete auto save file.
+        wxFileName autoSaveFileName = schematicFileName;
+        autoSaveFileName.SetName( GetAutoSaveFilePrefix() + schematicFileName.GetName() );
+
+        if( autoSaveFileName.FileExists() )
+        {
+            wxLogTrace( traceAutoSave,
+                        wxT( "Removing auto save file <" ) + autoSaveFileName.GetFullPath() +
+                        wxT( ">" ) );
+
+            wxRemoveFile( autoSaveFileName.GetFullPath() );
+        }
+
+        screen->SetContentModified( false );
+
+        msg.Printf( _( "File '%s' saved." ),  screen->GetFileName() );
+        SetStatusText( msg, 0 );
+    }
+    else
+    {
+        DisplayError( this, _( "File write operation failed." ) );
+    }
+
+    return success;
+}
+
+
+bool SCH_EDIT_FRAME::SaveProject( bool aSaveAs )
 {
     wxString msg;
     SCH_SCREEN* screen;
     SCH_SCREENS screens( Schematic().Root() );
+    bool        saveCopyAs       = aSaveAs && !Kiface().IsSingle();
     bool        success          = true;
     bool        updateFileType   = false;
     bool        createNewProject = false;
@@ -839,29 +746,50 @@ bool SCH_EDIT_FRAME::SaveProject()
     wxString    fileName = Prj().AbsolutePath( Schematic().Root().GetFileName() );
     wxFileName  fn = fileName;
 
-    // If this a new schematic without a project and we are in the stand alone mode.  All new
+    // Path to save each screen to: will be the stored filename by default, but is overwritten by
-    // sheets that are not loaded from an existing file will have to be saved to a new path
+    // a Save As Copy operation.
-    // along with the root sheet.
+    std::unordered_map<SCH_SCREEN*, wxString> filenameMap;
-    if( Prj().IsNullProject() )
+
+    // Handle "Save As" and saving a new project/schematic for the first time in standalone
+    if( Prj().IsNullProject() || aSaveAs )
     {
-        // This should only be possible in stand alone mode.
+        // Null project should only be possible in standalone mode.
-        wxCHECK( Kiface().IsSingle(), false );
+        wxCHECK( Kiface().IsSingle() || aSaveAs, false );
 
-        wxFileDialog dlg( this, _( "Schematic Files" ), fn.GetPath(), fn.GetFullName(),
+        wxFileName newFileName;
-                          KiCadSchematicFileWildcard(), wxFD_SAVE | wxFD_OVERWRITE_PROMPT );
+        wxFileName savePath( Prj().GetProjectFullName() );
 
+        if( !savePath.IsOk() || !savePath.IsDirWritable() )
+        {
+            savePath = GetMruPath();
+
+            if( !savePath.IsOk() || !savePath.IsDirWritable() )
+                savePath = PATHS::GetDefaultUserProjectsPath();
+        }
+
+        if( savePath.HasExt() )
+            savePath.SetExt( KiCadSchematicFileExtension );
+        else
+            savePath.SetName( wxEmptyString );
+
+        wxFileDialog dlg( this, _( "Schematic Files" ), savePath.GetPath(),
+                          savePath.GetFullName(), KiCadSchematicFileWildcard(),
+                          wxFD_SAVE | wxFD_OVERWRITE_PROMPT );
+
+        if( Kiface().IsSingle() || aSaveAs )
+        {
             // Add a "Create a project" checkbox in standalone mode and one isn't loaded
             dlg.SetExtraControlCreator( &CREATE_PROJECT_CHECKBOX::Create );
+        }
 
         if( dlg.ShowModal() == wxID_CANCEL )
             return false;
 
-        wxFileName newFileName = dlg.GetPath();
+        newFileName = dlg.GetPath();
-
-        if( newFileName.GetExt().IsEmpty() )
         newFileName.SetExt( KiCadSchematicFileExtension );
 
-        if( ( !newFileName.DirExists() && !newFileName.Mkdir() ) || !newFileName.IsDirWritable() )
+        if( ( !newFileName.DirExists() && !newFileName.Mkdir() ) ||
+            !newFileName.IsDirWritable() )
         {
             msg.Printf( _( "Folder '%s' could not be created.\n\n"
                            "Make sure you have write permissions and try again." ),
@@ -874,11 +802,18 @@ bool SCH_EDIT_FRAME::SaveProject()
             return false;
         }
 
+        if( wxWindow* ec = dlg.GetExtraControl() )
+            createNewProject = static_cast<CREATE_PROJECT_CHECKBOX*>( ec )->GetValue();
+
+        if( !saveCopyAs )
+        {
             Schematic().Root().SetFileName( newFileName.GetFullName() );
             Schematic().RootScreen()->SetFileName( newFileName.GetFullPath() );
-
+        }
-        if( wxWindow* extraControl = dlg.GetExtraControl() )
+        else
-            createNewProject = static_cast<CREATE_PROJECT_CHECKBOX*>( extraControl )->GetValue();
+        {
+            filenameMap[Schematic().RootScreen()] = newFileName.GetFullPath();
+        }
 
         // Set the base path to all new sheets.
         for( size_t i = 0; i < screens.GetCount(); i++ )
@@ -934,6 +869,9 @@ bool SCH_EDIT_FRAME::SaveProject()
                 return false;
             }
 
+            if( saveCopyAs )
+                filenameMap[screen] = tmp.GetFullPath();
+            else
                 screen->SetFileName( tmp.GetFullPath() );
         }
 
@@ -949,6 +887,12 @@ bool SCH_EDIT_FRAME::SaveProject()
         }
     }
 
+    if( filenameMap.empty() || !saveCopyAs )
+    {
+        for( size_t i = 0; i < screens.GetCount(); i++ )
+            filenameMap[screens.GetScreen( i )] = screens.GetScreen( i )->GetFileName();
+    }
+
     // Warn user on potential file overwrite.  This can happen on shared sheets.
     wxArrayString overwrittenFiles;
 
@@ -959,7 +903,7 @@ bool SCH_EDIT_FRAME::SaveProject()
         wxCHECK2( screen, continue );
 
         // Convert legacy schematics file name extensions for the new format.
-        wxFileName tmpFn = screen->GetFileName();
+        wxFileName tmpFn = filenameMap[screen];
 
         if( !tmpFn.IsOk() )
             continue;
@@ -1004,7 +948,7 @@ bool SCH_EDIT_FRAME::SaveProject()
         wxCHECK2( screen, continue );
 
         // Convert legacy schematics file name extensions for the new format.
-        wxFileName tmpFn = screen->GetFileName();
+        wxFileName tmpFn = filenameMap[screen];
 
         if( tmpFn.IsOk() && tmpFn.GetExt() != KiCadSchematicFileExtension )
         {
@@ -1024,6 +968,9 @@ bool SCH_EDIT_FRAME::SaveProject()
                 UpdateItem( sheet );
             }
 
+            filenameMap[screen] = tmpFn.GetFullPath();
+
+            if( !saveCopyAs )
                 screen->SetFileName( tmpFn.GetFullPath() );
         }
 
@@ -1034,9 +981,16 @@ bool SCH_EDIT_FRAME::SaveProject()
         else
             screen->SetVirtualPageNumber( 0 );  // multiple uses; no way to store the real sheet #
 
-        success &= SaveEEFile( screens.GetSheet( i ) );
+        // This is a new schematic file so make sure it has a unique ID.
+        if( !saveCopyAs && tmpFn.GetFullPath() != screen->GetFileName() )
+            screen->AssignNewUuid();
+
+        success &= saveSchematicFile( screens.GetSheet( i ), tmpFn.GetFullPath() );
     }
 
+    if( aSaveAs && success )
+        LockFile( Schematic().RootScreen()->GetFileName() );
+
     if( updateFileType )
         UpdateFileHistory( Schematic().RootScreen()->GetFileName() );
 
@@ -1065,16 +1019,22 @@ bool SCH_EDIT_FRAME::SaveProject()
         }
     }
 
-    if( !Prj().IsNullProject() )
+    wxASSERT( filenameMap.count( Schematic().RootScreen() ) );
+    wxFileName projectPath( filenameMap.at( Schematic().RootScreen() ) );
+    projectPath.SetExt( ProjectFileExtension );
+
+    if( Prj().IsNullProject() || ( aSaveAs && !saveCopyAs ) )
     {
-        GetSettingsManager()->SaveProject();
+        Prj().SetReadOnly( !createNewProject );
+        GetSettingsManager()->SaveProjectAs( projectPath.GetFullPath() );
+    }
+    else if( saveCopyAs && createNewProject )
+    {
+        GetSettingsManager()->SaveProjectCopy( projectPath.GetFullPath() );
     }
     else
     {
-        Prj().SetReadOnly( !createNewProject );
+        GetSettingsManager()->SaveProject();
-        wxFileName projectPath( Schematic().GetFileName() );
-        projectPath.SetExt( ProjectFileExtension );
-        GetSettingsManager()->SaveProjectAs( projectPath.GetFullPath() );
     }
 
     if( !Kiface().IsSingle() )
@@ -1131,14 +1091,10 @@ bool SCH_EDIT_FRAME::doAutoSave()
         // Auto save file name is the normal file name prefixed with GetAutoSavePrefix().
         fn.SetName( GetAutoSaveFilePrefix() + fn.GetName() );
 
-        screens.GetScreen( i )->SetFileName( fn.GetFullPath() );
+        if( saveSchematicFile( screens.GetSheet( i ), fn.GetFullPath() ) )
-
-        if( SaveEEFile( screens.GetSheet( i ), false ) )
             screens.GetScreen( i )->SetContentModified();
         else
             autoSaveOk = false;
-
-        screens.GetScreen( i )->SetFileName( tmpFileName.GetFullPath() );
     }
 
     if( autoSaveOk )

eeschema/menubar.cpp

@@ -81,7 +81,11 @@ void SCH_EDIT_FRAME::ReCreateMenuBar()
     }
 
     fileMenu->Add( ACTIONS::save );
+
+    if( Kiface().IsSingle() )
         fileMenu->Add( ACTIONS::saveAs );
+    else
+        fileMenu->Add( ACTIONS::saveCopyAs );
 
     fileMenu->AppendSeparator();
 

eeschema/sch_edit_frame.h

@@ -477,9 +477,13 @@ public:
     void NewProject();
     void LoadProject();
 
-    void Save_File( bool doSaveAs = false );
+    /**
-
+     * Saves the currently-open schematic (including its hierarchy) and associated project
-    bool SaveProject();
+     * @param aSaveAs is true to perform a Save As operation (rename the schematic and project).
+     *                This may only be done in standalone mode.
+     * @return true if the schematic was saved
+     */
+    bool SaveProject( bool aSaveAs = false );
 
     bool OpenProjectFiles( const std::vector<wxString>& aFileSet, int aCtl = 0 ) override;
 
@@ -492,21 +496,6 @@ public:
      */
     bool AppendSchematic();
 
-    /**
-     * Save \a aSheet to a schematic file.
-     *
-     * @param aSheet is the #SCH_SHEET object to save.  A NULL pointer saves the
-     *               current screen only.
-     * @param aSaveUnderNewName Controls how the file is to be saved;: using  previous name
-     *                          or under a new name.
-     * @param aCreateProject will be filled with the user's choice of whether to create a new
-     *                       project file if aSaveUnderNewName is true.  This value will not be
-     *                       changed if aSaveUnderNewName is false.
-     * @return True if the file has been saved.
-     */
-    bool SaveEEFile( SCH_SHEET* aSheet, bool aSaveUnderNewName = false,
-                     bool* aCreateProject = nullptr );
-
     /**
      * Check if any of the screens has unsaved changes and asks the user whether to save or
      * drop them.
@@ -906,6 +895,15 @@ private:
      */
     bool importFile( const wxString& aFileName, int aFileType );
 
+    /**
+     * Save \a aSheet to a schematic file.
+     *
+     * @param aSheet is the #SCH_SHEET object to save.
+     * @param aSavePath is the full path of the destination file
+     * @return True if the file has been saved.
+     */
+    bool saveSchematicFile( SCH_SHEET* aSheet, const wxString& aSavePath );
+
     /**
      * Fill a map of uuid -> reference from the currently loaded schematic.
      *

eeschema/tools/sch_editor_control.cpp

@@ -86,7 +86,7 @@ int SCH_EDITOR_CONTROL::Save( const TOOL_EVENT& aEvent )
 
 int SCH_EDITOR_CONTROL::SaveAs( const TOOL_EVENT& aEvent )
 {
-    m_frame->Save_File( true );
+    m_frame->SaveProject( true );
     return 0;
 }
 
@@ -2033,6 +2033,7 @@ void SCH_EDITOR_CONTROL::setTransitions()
     Go( &SCH_EDITOR_CONTROL::Open,                  ACTIONS::open.MakeEvent() );
     Go( &SCH_EDITOR_CONTROL::Save,                  ACTIONS::save.MakeEvent() );
     Go( &SCH_EDITOR_CONTROL::SaveAs,                ACTIONS::saveAs.MakeEvent() );
+    Go( &SCH_EDITOR_CONTROL::SaveAs,                ACTIONS::saveCopyAs.MakeEvent() );
     Go( &SCH_EDITOR_CONTROL::ShowSchematicSetup,    EE_ACTIONS::schematicSetup.MakeEvent() );
     Go( &SCH_EDITOR_CONTROL::PageSetup,             ACTIONS::pageSettings.MakeEvent() );
     Go( &SCH_EDITOR_CONTROL::Print,                 ACTIONS::print.MakeEvent() );