From 8c0cf3550b40de476d359afa331eb317f50a01f6 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Wed, 20 Aug 2025 10:07:46 -0700 Subject: [PATCH] Update schematic save as functionality Default to copying subsheet data if it lives in the current project. Allows options for other behavior. Fixes https://gitlab.com/kicad/code/kicad/-/issues/21518 --- eeschema/files-io.cpp | 145 ++++++----- eeschema/save_project_utils.h | 17 ++ eeschema/widgets/filedlg_hook_save_project.h | 19 ++ .../qa_utils/wx_utils/unit_test_utils.h | 37 +++ qa/tests/eeschema/CMakeLists.txt | 1 + .../eeschema/test_saveas_copy_subsheets.cpp | 236 ++++++++++++++++++ 6 files changed, 392 insertions(+), 63 deletions(-) create mode 100644 eeschema/save_project_utils.h create mode 100644 qa/tests/eeschema/test_saveas_copy_subsheets.cpp diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index d399ae0c98..c80a484141 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -81,6 +81,7 @@ #include #include "widgets/filedlg_hook_save_project.h" +#include "save_project_utils.h" bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, int aCtl ) { @@ -848,6 +849,75 @@ bool SCH_EDIT_FRAME::saveSchematicFile( SCH_SHEET* aSheet, const wxString& aSave } +bool PrepareSaveAsFiles( SCHEMATIC& aSchematic, SCH_SCREENS& aScreens, + const wxFileName& aOldRoot, const wxFileName& aNewRoot, + bool aSaveCopy, bool aCopySubsheets, bool aIncludeExternSheets, + std::unordered_map& aFilenameMap, + wxString& aErrorMsg ) +{ + SCH_SCREEN* screen; + + for( size_t i = 0; i < aScreens.GetCount(); i++ ) + { + screen = aScreens.GetScreen( i ); + + wxCHECK2( screen, continue ); + + if( screen == aSchematic.RootScreen() ) + continue; + + wxFileName src = screen->GetFileName(); + + if( !src.IsAbsolute() ) + src.MakeAbsolute( aOldRoot.GetPath() ); + + bool internalSheet = src.GetPath().StartsWith( aOldRoot.GetPath() ); + + if( aCopySubsheets && ( internalSheet || aIncludeExternSheets ) ) + { + wxFileName dest = src; + + if( internalSheet && dest.MakeRelativeTo( aOldRoot.GetPath() ) ) + dest.MakeAbsolute( aNewRoot.GetPath() ); + else + dest.Assign( aNewRoot.GetPath(), dest.GetFullName() ); + + wxLogTrace( tracePathsAndFiles, + wxS( "Moving schematic from '%s' to '%s'." ), + screen->GetFileName(), + dest.GetFullPath() ); + + if( !dest.DirExists() && !dest.Mkdir() ) + { + aErrorMsg.Printf( _( "Folder '%s' could not be created.\n\n" + "Make sure you have write permissions and try again." ), + dest.GetPath() ); + return false; + } + + if( aSaveCopy ) + aFilenameMap[screen] = dest.GetFullPath(); + else + screen->SetFileName( dest.GetFullPath() ); + } + else + { + if( aSaveCopy ) + aFilenameMap[screen] = wxString(); + + screen->SetFileName( src.GetFullPath() ); + } + } + + for( SCH_SHEET_PATH& sheet : aSchematic.Hierarchy() ) + { + if( !sheet.Last()->IsRootSheet() ) + sheet.MakeFilePathRelativeToParentSheet(); + } + + return true; +} + bool SCH_EDIT_FRAME::SaveProject( bool aSaveAs ) { wxString msg; @@ -857,6 +927,8 @@ bool SCH_EDIT_FRAME::SaveProject( bool aSaveAs ) bool success = true; bool updateFileHistory = false; bool createNewProject = false; + bool copySubsheets = false; + bool includeExternSheets = false; // I want to see it in the debugger, show me the string! Can't do that with wxFileName. wxString fileName = Prj().AbsolutePath( Schematic().Root().GetFileName() ); @@ -920,7 +992,11 @@ bool SCH_EDIT_FRAME::SaveProject( bool aSaveAs ) } if( newProjectHook.IsAttachedToDialog() ) + { createNewProject = newProjectHook.GetCreateNewProject(); + copySubsheets = newProjectHook.GetCopySubsheets(); + includeExternSheets = newProjectHook.GetIncludeExternSheets(); + } if( !saveCopy ) { @@ -933,71 +1009,14 @@ bool SCH_EDIT_FRAME::SaveProject( bool aSaveAs ) filenameMap[Schematic().RootScreen()] = newFileName.GetFullPath(); } - // Set the base path to all new sheets. - for( size_t i = 0; i < screens.GetCount(); i++ ) + if( !PrepareSaveAsFiles( Schematic(), screens, fn, newFileName, saveCopy, + copySubsheets, includeExternSheets, filenameMap, msg ) ) { - screen = screens.GetScreen( i ); + wxMessageDialog dlgBadFilePath( this, msg, _( "Error" ), + wxOK | wxICON_EXCLAMATION | wxCENTER ); - wxCHECK2( screen, continue ); - - // The root screen file name has already been set. - if( screen == Schematic().RootScreen() ) - continue; - - wxFileName tmp = screen->GetFileName(); - - // Assume existing sheet files are being reused and do not save them to the new - // path. Maybe in the future, add a user option to copy schematic files to the - // new project path. - if( tmp.FileExists() ) - continue; - - if( tmp.GetPath().IsEmpty() ) - { - tmp.SetPath( newFileName.GetPath() ); - } - else if( tmp.GetPath() == fn.GetPath() ) - { - tmp.SetPath( newFileName.GetPath() ); - } - else if( tmp.GetPath().StartsWith( fn.GetPath() ) ) - { - // NOTE: this hasn't been tested because the sheet properties dialog no longer - // allows adding a path specifier in the file name field. - wxString newPath = newFileName.GetPath(); - newPath += tmp.GetPath().Right( fn.GetPath().Length() ); - tmp.SetPath( newPath ); - } - - wxLogTrace( tracePathsAndFiles, - wxS( "Moving schematic from '%s' to '%s'." ), - screen->GetFileName(), - tmp.GetFullPath() ); - - if( !tmp.DirExists() && !tmp.Mkdir() ) - { - msg.Printf( _( "Folder '%s' could not be created.\n\n" - "Make sure you have write permissions and try again." ), - newFileName.GetPath() ); - - wxMessageDialog dlgBadFilePath( this, msg, _( "Error" ), - wxOK | wxICON_EXCLAMATION | wxCENTER ); - - dlgBadFilePath.ShowModal(); - return false; - } - - if( saveCopy ) - filenameMap[screen] = tmp.GetFullPath(); - else - screen->SetFileName( tmp.GetFullPath() ); - } - - // Attempt to make sheet file name paths relative to the new root schematic path. - for( SCH_SHEET_PATH& sheet : Schematic().Hierarchy() ) - { - if( !sheet.Last()->IsRootSheet() ) - sheet.MakeFilePathRelativeToParentSheet(); + dlgBadFilePath.ShowModal(); + return false; } } else if( !fn.FileExists() ) diff --git a/eeschema/save_project_utils.h b/eeschema/save_project_utils.h new file mode 100644 index 0000000000..a80b82103f --- /dev/null +++ b/eeschema/save_project_utils.h @@ -0,0 +1,17 @@ +#ifndef SAVE_PROJECT_UTILS_H +#define SAVE_PROJECT_UTILS_H + +#include +#include + +class SCHEMATIC; +class SCH_SCREEN; +class SCH_SCREENS; + +bool PrepareSaveAsFiles( SCHEMATIC& aSchematic, SCH_SCREENS& aScreens, + const wxFileName& aOldRoot, const wxFileName& aNewRoot, + bool aSaveCopy, bool aCopySubsheets, bool aIncludeExternSheets, + std::unordered_map& aFilenameMap, + wxString& aErrorMsg ); + +#endif diff --git a/eeschema/widgets/filedlg_hook_save_project.h b/eeschema/widgets/filedlg_hook_save_project.h index f1a7f4db8e..c4743c8054 100644 --- a/eeschema/widgets/filedlg_hook_save_project.h +++ b/eeschema/widgets/filedlg_hook_save_project.h @@ -36,25 +36,44 @@ public: m_cb = customizer.AddCheckBox( _( "Create a new project for this schematic" ) ); m_cb->SetValue( true ); + wxString choices[] = { + _( "Do not copy subsheets" ), + _( "Copy internal subsheets only" ), + _( "Copy all subsheets" ) + }; + + m_choice = customizer.AddChoice( 3, choices ); + m_choice->SetSelection( 1 ); // Default to copying internal subsheets only m_controlsAttached = true; } virtual void TransferDataFromCustomControls() override { m_createNewProject = m_cb->GetValue(); + m_copySubsheets = m_choice->GetSelection() > 0; + m_includeExternal = m_choice->GetSelection() > 1; } ///< Gets the selected state of the create new project option bool GetCreateNewProject() const { return m_createNewProject; } + ///< Gets the selected state of the copy subsheets option + bool GetCopySubsheets() const { return m_copySubsheets; } + + ///< Gets the selected state of the include external sheets option + bool GetIncludeExternSheets() const { return m_includeExternal; } + ///< Gets if this hook has attached controls to a dialog box bool IsAttachedToDialog() const { return m_controlsAttached; } private: bool m_createNewProject = true; + bool m_copySubsheets = true; + bool m_includeExternal = false; bool m_controlsAttached = false; wxFileDialogCheckBox* m_cb = nullptr; + wxFileDialogChoice* m_choice = nullptr; wxDECLARE_NO_COPY_CLASS( FILEDLG_HOOK_SAVE_PROJECT ); }; diff --git a/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h b/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h index 64d7cd8a7c..c067de3c81 100644 --- a/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h +++ b/qa/qa_utils/include/qa_utils/wx_utils/unit_test_utils.h @@ -36,6 +36,7 @@ #include #include +#include @@ -136,6 +137,42 @@ std::ostream& boost_test_print_type( std::ostream& os, std::pair const& aP } // namespace std +//-----------------------------------------------------------------------------+ +// Boost.Test printing helpers for wx types / wide string literals +//-----------------------------------------------------------------------------+ +namespace boost { namespace test_tools { namespace tt_detail { + +template<> +struct print_log_value +{ + void operator()( std::ostream& os, wxString const& v ) + { +#if wxUSE_UNICODE + os << v.ToUTF8().data(); +#else + os << v; +#endif + } +}; + +// Wide string literal arrays +template +struct print_log_value +{ + void operator()( std::ostream& os, const wchar_t (&ws)[ N ] ) + { + wxString tmp( ws ); +#if wxUSE_UNICODE + os << tmp.ToUTF8().data(); +#else + os << tmp; +#endif + } +}; + +}}} // namespace boost::test_tools::tt_detail + + /** * Boost print helper for wxPoint. Note operator<< for this type doesn't * exist in non-DEBUG builds. diff --git a/qa/tests/eeschema/CMakeLists.txt b/qa/tests/eeschema/CMakeLists.txt index f81169b253..7e64fb712c 100644 --- a/qa/tests/eeschema/CMakeLists.txt +++ b/qa/tests/eeschema/CMakeLists.txt @@ -93,6 +93,7 @@ set( QA_EESCHEMA_SRCS test_schematic.cpp test_symbol_library_manager.cpp test_resolve_drivers.cpp + test_saveas_copy_subsheets.cpp test_update_items_connectivity.cpp ) diff --git a/qa/tests/eeschema/test_saveas_copy_subsheets.cpp b/qa/tests/eeschema/test_saveas_copy_subsheets.cpp new file mode 100644 index 0000000000..ffedcd0e54 --- /dev/null +++ b/qa/tests/eeschema/test_saveas_copy_subsheets.cpp @@ -0,0 +1,236 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright The KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +/** + * @file + * Tests for Save As subsheet copy options + */ + +#include +#include "eeschema_test_utils.h" + +#include +#include +#include +#include +#include +#include +#include +#include + +class SAVEAS_SUBSHEET_FIXTURE : public KI_TEST::SCHEMATIC_TEST_FIXTURE +{ +public: + SAVEAS_SUBSHEET_FIXTURE(); + + wxFileName GetSchematicPath( const wxString& aRelativePath ) override; + + wxFileName m_baseDir; + wxFileName m_srcDir; + wxFileName m_externalDir; +}; + +SAVEAS_SUBSHEET_FIXTURE::SAVEAS_SUBSHEET_FIXTURE() +{ + wxFileName tmp( wxFileName::CreateTempFileName( wxS( "saveas" ) ) ); + wxString dirPath = tmp.GetPath(); // /tmp + wxString dirName = tmp.GetName(); // saveasXXXX + wxRemoveFile( tmp.GetFullPath() ); + + wxFileName base( dirPath, wxEmptyString ); // represents /tmp + base.AppendDir( dirName ); // now /tmp/saveasXXXX + base.Mkdir(); // create the directory + m_baseDir = base; + + m_srcDir = m_baseDir; + m_srcDir.AppendDir( wxS( "src" ) ); + m_srcDir.Mkdir(); + + m_externalDir = m_baseDir; + m_externalDir.AppendDir( wxS( "external" ) ); + m_externalDir.Mkdir(); + + wxFileName dataDir( KI_TEST::GetEeschemaTestDataDir() ); + + wxCopyFile( dataDir.GetFullPath() + wxS( "/issue13212.kicad_sch" ), + m_srcDir.GetFullPath() + wxS( "/issue13212.kicad_sch" ) ); + wxCopyFile( dataDir.GetFullPath() + wxS( "/issue13212_subsheet_1.kicad_sch" ), + m_srcDir.GetFullPath() + wxS( "/issue13212_subsheet_1.kicad_sch" ) ); + wxCopyFile( dataDir.GetFullPath() + wxS( "/issue13212_subsheet_2.kicad_sch" ), + m_externalDir.GetFullPath() + wxS( "/issue13212_subsheet_2.kicad_sch" ) ); + + wxFileName rootFile( m_srcDir ); + rootFile.SetFullName( wxS( "issue13212.kicad_sch" ) ); + + wxFFile file( rootFile.GetFullPath(), wxS( "rb" ) ); + wxString content; + file.ReadAll( &content ); + file.Close(); + content.Replace( wxS( "issue13212_subsheet_2.kicad_sch" ), + wxS( "../external/issue13212_subsheet_2.kicad_sch" ) ); + wxFFile outFile( rootFile.GetFullPath(), wxS( "wb" ) ); + outFile.Write( content ); + outFile.Close(); +} + +wxFileName SAVEAS_SUBSHEET_FIXTURE::GetSchematicPath( const wxString& aRelativePath ) +{ + wxFileName fn( m_srcDir ); + fn.SetName( aRelativePath ); + fn.SetExt( FILEEXT::KiCadSchematicFileExtension ); + return fn; +} + +BOOST_FIXTURE_TEST_SUITE( SaveAsSubsheetCopy, SAVEAS_SUBSHEET_FIXTURE ) + +BOOST_AUTO_TEST_CASE( CopyInternalReferenceExternal ) +{ + LoadSchematic( wxS( "issue13212" ) ); + + SCH_SCREENS screens( m_schematic->Root() ); + wxFileName srcRoot = GetSchematicPath( wxS( "issue13212" ) ); + + wxFileName destRoot( m_baseDir ); + destRoot.AppendDir( wxS( "new" ) ); + destRoot.AppendDir( wxS( "location" ) ); + destRoot.SetName( wxS( "issue13212" ) ); + destRoot.SetExt( FILEEXT::KiCadSchematicFileExtension ); + { wxFileName destDir( destRoot ); destDir.SetFullName( wxEmptyString ); if( !destDir.DirExists() ) destDir.Mkdir( 0777, wxPATH_MKDIR_FULL ); } + + std::unordered_map filenameMap; + wxString msg; + + m_schematic->Root().SetFileName( destRoot.GetFullName() ); + m_schematic->RootScreen()->SetFileName( destRoot.GetFullPath() ); + + bool ok = PrepareSaveAsFiles( *m_schematic, screens, srcRoot, destRoot, false, true, + false, filenameMap, msg ); + BOOST_CHECK( ok ); + + SCH_SCREEN* internal = nullptr; + SCH_SCREEN* external = nullptr; + + for( size_t i = 0; i < screens.GetCount(); i++ ) + { + SCH_SCREEN* s = screens.GetScreen( i ); + + if( wxString( s->GetFileName() ).EndsWith( wxS( "issue13212_subsheet_1.kicad_sch" ) ) ) + internal = s; + else if( wxString( s->GetFileName() ).EndsWith( wxS( "issue13212_subsheet_2.kicad_sch" ) ) ) + external = s; + } + + wxFileName internalExpected( destRoot.GetPath(), wxS( "issue13212_subsheet_1.kicad_sch" ) ); + BOOST_CHECK_EQUAL( internal->GetFileName(), internalExpected.GetFullPath() ); + + wxFileName externalExpected( m_externalDir.GetFullPath(), wxS( "issue13212_subsheet_2.kicad_sch" ) ); + BOOST_CHECK_EQUAL( external->GetFileName(), externalExpected.GetFullPath() ); + + SCH_SHEET_LIST sheetList = m_schematic->BuildSheetListSortedByPageNumbers(); + + wxString externalSheetPath; + + for( const SCH_SHEET_PATH& path : sheetList ) + { + if( path.Last()->GetFileName().Contains( wxS( "issue13212_subsheet_2" ) ) ) + externalSheetPath = path.Last()->GetFileName(); + } + + BOOST_CHECK_EQUAL( externalSheetPath, wxS( "../../external/issue13212_subsheet_2.kicad_sch" ) ); +} + +BOOST_AUTO_TEST_CASE( CopyIncludingExternal ) +{ + LoadSchematic( wxS( "issue13212" ) ); + + SCH_SCREENS screens( m_schematic->Root() ); + wxFileName srcRoot = GetSchematicPath( wxS( "issue13212" ) ); + + wxFileName destRoot( m_baseDir ); + destRoot.AppendDir( wxS( "destall" ) ); + destRoot.SetName( wxS( "issue13212" ) ); + destRoot.SetExt( FILEEXT::KiCadSchematicFileExtension ); + { wxFileName destDir( destRoot ); destDir.SetFullName( wxEmptyString ); if( !destDir.DirExists() ) destDir.Mkdir( 0777, wxPATH_MKDIR_FULL ); } + + std::unordered_map filenameMap; + wxString msg; + + m_schematic->Root().SetFileName( destRoot.GetFullName() ); + m_schematic->RootScreen()->SetFileName( destRoot.GetFullPath() ); + + bool ok = PrepareSaveAsFiles( *m_schematic, screens, srcRoot, destRoot, false, true, + true, filenameMap, msg ); + BOOST_CHECK( ok ); + + SCH_SCREEN* external = nullptr; + + for( size_t i = 0; i < screens.GetCount(); i++ ) + { + SCH_SCREEN* s = screens.GetScreen( i ); + + if( wxString( s->GetFileName() ).EndsWith( wxS( "issue13212_subsheet_2.kicad_sch" ) ) ) + external = s; + } + + wxFileName externalExpected( destRoot.GetPath(), wxS( "issue13212_subsheet_2.kicad_sch" ) ); + BOOST_CHECK_EQUAL( external->GetFileName(), externalExpected.GetFullPath() ); +} + +BOOST_AUTO_TEST_CASE( NoCopyKeepsOriginalPaths ) +{ + LoadSchematic( wxS( "issue13212" ) ); + + SCH_SCREENS screens( m_schematic->Root() ); + wxFileName srcRoot = GetSchematicPath( wxS( "issue13212" ) ); + + wxFileName destRoot( m_baseDir ); + destRoot.AppendDir( wxS( "nocopy" ) ); + destRoot.SetName( wxS( "issue13212" ) ); + destRoot.SetExt( FILEEXT::KiCadSchematicFileExtension ); + { wxFileName destDir( destRoot ); destDir.SetFullName( wxEmptyString ); if( !destDir.DirExists() ) destDir.Mkdir( 0777, wxPATH_MKDIR_FULL ); } + + std::unordered_map filenameMap; + wxString msg; + + m_schematic->Root().SetFileName( destRoot.GetFullName() ); + m_schematic->RootScreen()->SetFileName( destRoot.GetFullPath() ); + + bool ok = PrepareSaveAsFiles( *m_schematic, screens, srcRoot, destRoot, false, false, + false, filenameMap, msg ); + BOOST_CHECK( ok ); + + SCH_SCREEN* internal = nullptr; + + for( size_t i = 0; i < screens.GetCount(); i++ ) + { + SCH_SCREEN* s = screens.GetScreen( i ); + + if( wxString( s->GetFileName() ).EndsWith( wxS( "issue13212_subsheet_1.kicad_sch" ) ) ) + internal = s; + } + + wxFileName internalExpected( m_srcDir.GetFullPath(), wxS( "issue13212_subsheet_1.kicad_sch" ) ); + BOOST_CHECK_EQUAL( internal->GetFileName(), internalExpected.GetFullPath() ); +} + +BOOST_AUTO_TEST_SUITE_END()