From ba4974749cdccd2e817243f7eb1ae1b38ff88ff6 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Fri, 3 May 2024 13:21:25 -0700 Subject: [PATCH] Fix renaming sheet check If we are renaming a sheet that exists only once in the schematic, then we don't need to warn about case sensitivity. If the sheet exists multiple times, we do. We can't check this using SCREENS because the screens don't maintain full state information about where they are used. Fixes https://gitlab.com/kicad/code/kicad/-/issues/17901 --- eeschema/dialogs/dialog_sheet_properties.cpp | 4 +- eeschema/sch_edit_frame.h | 2 +- eeschema/sch_screen.cpp | 27 ------------ eeschema/sch_screen.h | 13 ------ eeschema/sheet.cpp | 43 +++++++++++++++++--- 5 files changed, 41 insertions(+), 48 deletions(-) diff --git a/eeschema/dialogs/dialog_sheet_properties.cpp b/eeschema/dialogs/dialog_sheet_properties.cpp index b5c90f4b47..10cee24761 100644 --- a/eeschema/dialogs/dialog_sheet_properties.cpp +++ b/eeschema/dialogs/dialog_sheet_properties.cpp @@ -502,7 +502,7 @@ bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilena if( m_sheet->GetScreen() == nullptr ) // New just created sheet. { - if( !m_frame->AllowCaseSensitiveFileNameClashes( newAbsoluteFilename ) ) + if( !m_frame->AllowCaseSensitiveFileNameClashes( m_sheet->GetFileName(), newAbsoluteFilename ) ) return false; if( useScreen || loadFromFile ) // Load from existing file. @@ -527,7 +527,7 @@ bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilena { isExistingSheet = true; - if( !m_frame->AllowCaseSensitiveFileNameClashes( newAbsoluteFilename ) ) + if( !m_frame->AllowCaseSensitiveFileNameClashes( m_sheet->GetFileName(), newAbsoluteFilename ) ) return false; // We are always using here a case insensitive comparison to avoid issues diff --git a/eeschema/sch_edit_frame.h b/eeschema/sch_edit_frame.h index 315a9f6aa2..8ffca4cf66 100644 --- a/eeschema/sch_edit_frame.h +++ b/eeschema/sch_edit_frame.h @@ -546,7 +546,7 @@ public: * @param aSchematicFileName is the absolute path and file name of the file to test. * @return true if the user accepts the potential file name clash risk. */ - bool AllowCaseSensitiveFileNameClashes( const wxString& aSchematicFileName ); + bool AllowCaseSensitiveFileNameClashes( const wxString& aOldName, const wxString& aSchematicFileName ); /** * Edit an existing sheet or add a new sheet to the schematic. diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp index d5026266ec..bcd5b7e533 100644 --- a/eeschema/sch_screen.cpp +++ b/eeschema/sch_screen.cpp @@ -2084,33 +2084,6 @@ bool SCH_SCREENS::HasSchematic( const wxString& aSchematicFileName ) } -bool SCH_SCREENS::CanCauseCaseSensitivityIssue( const wxString& aSchematicFileName ) const -{ - wxString lhsLower; - wxString rhsLower; - wxFileName lhs; - wxFileName rhs = aSchematicFileName; - - wxCHECK( rhs.IsAbsolute(), false ); - - for( const SCH_SCREEN* screen : m_screens ) - { - lhs = screen->GetFileName(); - - if( lhs.GetPath() != rhs.GetPath() ) - continue; - - lhsLower = lhs.GetFullName().Lower(); - rhsLower = rhs.GetFullName().Lower(); - - if( lhsLower == rhsLower && lhs.GetFullName() != rhs.GetFullName() ) - return true; - } - - return false; -} - - void SCH_SCREENS::BuildClientSheetPathList() { SCH_SCREEN* first = GetFirst(); diff --git a/eeschema/sch_screen.h b/eeschema/sch_screen.h index 41b8eb8279..c84df1b194 100644 --- a/eeschema/sch_screen.h +++ b/eeschema/sch_screen.h @@ -812,19 +812,6 @@ public: */ void BuildClientSheetPathList(); - /** - * Check \a aSchematicFileName for a potential file name case sensitivity issue. - * - * On platforms where file names are case sensitive, it is possible to schematic sheet - * file names that would cause issues on platforms where file name are case insensitive. - * File names foo.sch and Foo.sch are unique files on Linux and MacOS but on Windows - * this would result in a broken schematic. - * - * @param[in] aSchematicFileName is the absolute path and file name of the file to test. - * @return true if \a aSchematicFileName would cause an issue. - */ - bool CanCauseCaseSensitivityIssue( const wxString& aSchematicFileName ) const; - /** * Update the symbol value and footprint instance data for legacy designs. */ diff --git a/eeschema/sheet.cpp b/eeschema/sheet.cpp index 7b0c1c8951..38a3b8f993 100644 --- a/eeschema/sheet.cpp +++ b/eeschema/sheet.cpp @@ -690,16 +690,49 @@ void SCH_EDIT_FRAME::DrawCurrentSheetToClipboard() } -bool SCH_EDIT_FRAME::AllowCaseSensitiveFileNameClashes( const wxString& aSchematicFileName ) +bool SCH_EDIT_FRAME::AllowCaseSensitiveFileNameClashes( const wxString& aOldName, const wxString& aSchematicFileName ) { wxString msg; - SCH_SCREENS screens( Schematic().Root() ); + SCH_SHEET_LIST sheets( &Schematic().Root() ); wxFileName fn = aSchematicFileName; wxCHECK( fn.IsAbsolute(), false ); - if( eeconfig()->m_Appearance.show_sheet_filename_case_sensitivity_dialog - && screens.CanCauseCaseSensitivityIssue( aSchematicFileName ) ) + auto can_cause_issues = [&]() -> bool + { + wxFileName lhs; + wxFileName rhs = aSchematicFileName; + wxFileName old = aOldName; + wxString oldLower = old.GetFullName().Lower(); + wxString rhsLower = rhs.GetFullName().Lower(); + wxString lhsLower; + + size_t count = 0; + + wxCHECK( rhs.IsAbsolute(), false ); + + for( SCH_SHEET_PATH& sheet : sheets ) + { + lhs = sheet.LastScreen()->GetFileName(); + + if( lhs.GetPath() != rhs.GetPath() ) + continue; + + lhsLower = lhs.GetFullName().Lower(); + + if( lhsLower == rhsLower && lhs.GetFullName() != rhs.GetFullName() ) + count++; + } + + // If we are renaming a sheet that is only used once, then we are not going to cause + // a case sensitivity issue. + if( oldLower == rhsLower ) + return count > 1; + + return count > 0; + }; + + if( eeconfig()->m_Appearance.show_sheet_filename_case_sensitivity_dialog && can_cause_issues() ) { msg.Printf( _( "The file name '%s' can cause issues with an existing file name\n" "already defined in the schematic on systems that support case\n" @@ -712,7 +745,7 @@ bool SCH_EDIT_FRAME::AllowCaseSensitiveFileNameClashes( const wxString& aSchemat wxYES_NO | wxNO_DEFAULT | wxICON_QUESTION ); dlg.ShowCheckBox( _( "Do not show this message again." ) ); dlg.SetYesNoLabels( wxMessageDialog::ButtonLabel( _( "Create New Sheet" ) ), - wxMessageDialog::ButtonLabel( _( "Discard New Sheet" ) ) ); + wxMessageDialog::ButtonLabel( _( "Cancel" ) ) ); if( dlg.ShowModal() == wxID_NO ) return false;