Don't attempt to test for recursion on unsaved file.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/16722
This commit is contained in:
Jeff Young 2024-01-28 15:21:14 +00:00
parent a5ffcd0a55
commit 00aed2aa3e
3 changed files with 25 additions and 18 deletions

View File

@ -526,13 +526,13 @@ public:
void OnAnnotate( wxCommandEvent& event ); 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 aSheet is the #SCH_SHEET object to test.
* @param aHierarchy is the #SCH_SHEET_PATH where \a aSheet is going to reside. * @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 aHierarchy. * @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. * Check \a aSchematicFileName for a potential file name case sensitivity clashes.
@ -617,14 +617,14 @@ public:
* - Refresh the symbol library links. * - Refresh the symbol library links.
* *
* @param aSheet is the sheet to either append or load the schematic. * @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 * @param aCurrentSheet is the current position in the schematic hierarchy used to test for
* possible file recursion issues. * possible file recursion issues.
* @param aFileName is the file name to load. The file name is expected to have an absolute * @param aFileName is the file name to load. The file name is expected to have an absolute
* path. * path.
* *
* @return True if the schematic was imported properly. * @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 ); const wxString& aFileName );
/** /**

View File

@ -46,26 +46,32 @@
#include <wx/log.h> #include <wx/log.h>
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; wxString msg;
SCH_SHEET_LIST hierarchy = Schematic().GetSheets(); // The full schematic sheet hierarchy. SCH_SHEET_LIST hierarchy = Schematic().GetSheets(); // The full schematic sheet hierarchy.
SCH_SHEET_LIST sheetHierarchy( aSheet ); // This is the hierarchy of the loaded file. 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, // SCH_SCREEN object file paths are expected to be absolute. If this assert fires,
// something is seriously broken. // 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 " 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 " "has the sheet '%s' or one of its subsheets as a parent somewhere in the "
"schematic hierarchy." ), "schematic hierarchy." ),
destFile.GetFullPath() ); destFilePath );
DisplayError( this, msg ); DisplayError( this, msg );
return true; 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 ) const wxString& aFileName )
{ {
wxASSERT( aSheet && aHierarchy ); wxASSERT( aSheet && aCurrentSheet );
wxString msg; wxString msg;
wxFileName currentSheetFileName; 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. SCH_SHEET_LIST hierarchy = Schematic().GetSheets(); // This is the schematic sheet hierarchy.
// Make sure any new sheet changes do not cause any recursion issues. // 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() ) ) || checkForNoFullyDefinedLibIds( tmpSheet.get() ) )
{ {
return false; return false;
@ -564,10 +570,10 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier
newScreen->MigrateSimModels(); newScreen->MigrateSimModels();
// Attempt to create new symbol instances using the instance data loaded above. // Attempt to create new symbol instances using the instance data loaded above.
sheetHierarchy.AddNewSymbolInstances( *aHierarchy ); sheetHierarchy.AddNewSymbolInstances( *aCurrentSheet );
// Add new sheet instance data. // 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. // It is finally safe to add or append the imported schematic.
if( aSheet->GetScreen() == nullptr ) if( aSheet->GetScreen() == nullptr )

View File

@ -611,6 +611,7 @@ TOOL_ACTION PCB_ACTIONS::filletLines( TOOL_ACTION_ARGS()
TOOL_ACTION PCB_ACTIONS::chamferLines( TOOL_ACTION_ARGS() TOOL_ACTION PCB_ACTIONS::chamferLines( TOOL_ACTION_ARGS()
.Name( "pcbnew.InteractiveEdit.chamferLines" ) .Name( "pcbnew.InteractiveEdit.chamferLines" )
.Scope( AS_GLOBAL ) .Scope( AS_GLOBAL )
// TODO: 9.0: Change to "Chamfer Lines..."
.FriendlyName( _( "Chamfer Lines" ) ) .FriendlyName( _( "Chamfer Lines" ) )
.Tooltip( _( "Cut away corners between selected lines" ) ) .Tooltip( _( "Cut away corners between selected lines" ) )
.Icon( BITMAPS::chamfer ) ); .Icon( BITMAPS::chamfer ) );