From 00aed2aa3ee19e784cfc05d2cb34948a0dccacc8 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 28 Jan 2024 15:21:14 +0000 Subject: [PATCH] Don't attempt to test for recursion on unsaved file. Fixes https://gitlab.com/kicad/code/kicad/-/issues/16722 --- eeschema/sch_edit_frame.h | 14 +++++++------- eeschema/sheet.cpp | 28 +++++++++++++++++----------- pcbnew/tools/pcb_actions.cpp | 1 + 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/eeschema/sch_edit_frame.h b/eeschema/sch_edit_frame.h index 79297629bb..aa959c45a1 100644 --- a/eeschema/sch_edit_frame.h +++ b/eeschema/sch_edit_frame.h @@ -526,13 +526,13 @@ public: void OnAnnotate( wxCommandEvent& event ); /** - * Verify that \a aSheet will not cause a recursion error in \a aHierarchy. + * Verify that \a aSheet will not cause a recursion error in \a aCurrentSheet. * * @param aSheet is the #SCH_SHEET object to test. - * @param aHierarchy is the #SCH_SHEET_PATH where \a aSheet is going to reside. - * @return true if \a aSheet will cause a recursion error in \a aHierarchy. + * @param aCurrentSheet is the #SCH_SHEET_PATH where \a aSheet is going to reside. + * @return true if \a aSheet will cause a recursion error in \a aCurrentSheet. */ - bool CheckSheetForRecursion( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy ); + bool CheckSheetForRecursion( SCH_SHEET* aSheet, SCH_SHEET_PATH* aCurrentSheet ); /** * Check \a aSchematicFileName for a potential file name case sensitivity clashes. @@ -617,14 +617,14 @@ public: * - Refresh the symbol library links. * * @param aSheet is the sheet to either append or load the schematic. - * @param aHierarchy is the current position in the schematic hierarchy used to test for - * possible file recursion issues. + * @param aCurrentSheet is the current position in the schematic hierarchy used to test for + * possible file recursion issues. * @param aFileName is the file name to load. The file name is expected to have an absolute * path. * * @return True if the schematic was imported properly. */ - bool LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy, + bool LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aCurrentSheet, const wxString& aFileName ); /** diff --git a/eeschema/sheet.cpp b/eeschema/sheet.cpp index e0689170d1..85feb76f0a 100644 --- a/eeschema/sheet.cpp +++ b/eeschema/sheet.cpp @@ -46,26 +46,32 @@ #include -bool SCH_EDIT_FRAME::CheckSheetForRecursion( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy ) +bool SCH_EDIT_FRAME::CheckSheetForRecursion( SCH_SHEET* aSheet, SCH_SHEET_PATH* aCurrentSheet ) { - wxASSERT( aSheet && aHierarchy ); + wxASSERT( aSheet && aCurrentSheet ); wxString msg; SCH_SHEET_LIST hierarchy = Schematic().GetSheets(); // The full schematic sheet hierarchy. SCH_SHEET_LIST sheetHierarchy( aSheet ); // This is the hierarchy of the loaded file. - wxFileName destFile = aHierarchy->LastScreen()->GetFileName(); + wxString destFilePath = aCurrentSheet->LastScreen()->GetFileName(); + + if( destFilePath.IsEmpty() ) + { + // If file is unsaved then there can't (yet) be any recursion. + return false; + } // SCH_SCREEN object file paths are expected to be absolute. If this assert fires, // something is seriously broken. - wxASSERT( destFile.IsAbsolute() ); + wxASSERT( wxFileName( destFilePath ).IsAbsolute() ); - if( hierarchy.TestForRecursion( sheetHierarchy, destFile.GetFullPath() ) ) + if( hierarchy.TestForRecursion( sheetHierarchy, destFilePath ) ) { msg.Printf( _( "The sheet changes cannot be made because the destination sheet already " "has the sheet '%s' or one of its subsheets as a parent somewhere in the " "schematic hierarchy." ), - destFile.GetFullPath() ); + destFilePath ); DisplayError( this, msg ); return true; } @@ -155,10 +161,10 @@ void SCH_EDIT_FRAME::InitSheet( SCH_SHEET* aSheet, const wxString& aNewFilename } -bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy, +bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aCurrentSheet, const wxString& aFileName ) { - wxASSERT( aSheet && aHierarchy ); + wxASSERT( aSheet && aCurrentSheet ); wxString msg; wxFileName currentSheetFileName; @@ -271,7 +277,7 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier SCH_SHEET_LIST hierarchy = Schematic().GetSheets(); // This is the schematic sheet hierarchy. // Make sure any new sheet changes do not cause any recursion issues. - if( CheckSheetForRecursion( tmpSheet.get(), aHierarchy ) + if( CheckSheetForRecursion( tmpSheet.get(), aCurrentSheet ) || checkForNoFullyDefinedLibIds( tmpSheet.get() ) ) { return false; @@ -564,10 +570,10 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier newScreen->MigrateSimModels(); // Attempt to create new symbol instances using the instance data loaded above. - sheetHierarchy.AddNewSymbolInstances( *aHierarchy ); + sheetHierarchy.AddNewSymbolInstances( *aCurrentSheet ); // Add new sheet instance data. - sheetHierarchy.AddNewSheetInstances( *aHierarchy, hierarchy.GetLastVirtualPageNumber() ); + sheetHierarchy.AddNewSheetInstances( *aCurrentSheet, hierarchy.GetLastVirtualPageNumber() ); // It is finally safe to add or append the imported schematic. if( aSheet->GetScreen() == nullptr ) diff --git a/pcbnew/tools/pcb_actions.cpp b/pcbnew/tools/pcb_actions.cpp index 3ab4ef315b..16098364ad 100644 --- a/pcbnew/tools/pcb_actions.cpp +++ b/pcbnew/tools/pcb_actions.cpp @@ -611,6 +611,7 @@ TOOL_ACTION PCB_ACTIONS::filletLines( TOOL_ACTION_ARGS() TOOL_ACTION PCB_ACTIONS::chamferLines( TOOL_ACTION_ARGS() .Name( "pcbnew.InteractiveEdit.chamferLines" ) .Scope( AS_GLOBAL ) +// TODO: 9.0: Change to "Chamfer Lines..." .FriendlyName( _( "Chamfer Lines" ) ) .Tooltip( _( "Cut away corners between selected lines" ) ) .Icon( BITMAPS::chamfer ) );