schematic: fix broken undo of sheets, new and changed

Fixes: https://gitlab.com/kicad/code/kicad/-/issues/11855
This commit is contained in:
Mike Williams 2024-05-30 10:31:54 -04:00
parent 549ea93be2
commit 32289ffce7
6 changed files with 38 additions and 32 deletions

View File

@ -47,10 +47,11 @@
#include "wx/dcclient.h"
DIALOG_SHEET_PROPERTIES::DIALOG_SHEET_PROPERTIES( SCH_EDIT_FRAME* aParent, SCH_SHEET* aSheet,
bool* aClearAnnotationNewItems,
bool* aIsUndoable, bool* aClearAnnotationNewItems,
bool* aUpdateHierarchyNavigator ) :
DIALOG_SHEET_PROPERTIES_BASE( aParent ),
m_frame( aParent ),
m_isUndoable( aIsUndoable ),
m_clearAnnotationNewItems( aClearAnnotationNewItems ),
m_updateHierarchyNavigator( aUpdateHierarchyNavigator ),
m_borderWidth( aParent, m_borderWidthLabel, m_borderWidthCtrl, m_borderWidthUnits ),
@ -275,7 +276,8 @@ bool DIALOG_SHEET_PROPERTIES::TransferDataFromWindow()
commit.Modify( m_sheet, m_frame->GetScreen() );
bool isUndoable = true;
if( m_isUndoable )
*m_isUndoable = true;
// Sheet file names can be relative or absolute.
wxString sheetFileName = m_fields->at( SHEETFILENAME ).GetText();
@ -345,7 +347,7 @@ bool DIALOG_SHEET_PROPERTIES::TransferDataFromWindow()
}
}
if( !onSheetFilenameChanged( newRelativeFilename, &isUndoable ) )
if( !onSheetFilenameChanged( newRelativeFilename ) )
{
if( clearFileName )
currentScreen->SetFileName( wxEmptyString );
@ -436,26 +438,12 @@ bool DIALOG_SHEET_PROPERTIES::TransferDataFromWindow()
for( SCH_ITEM* item : m_frame->GetScreen()->Items().OfType( SCH_SHEET_T ) )
m_frame->UpdateItem( item );
if( isUndoable )
{
commit.Push( _( "Edit Sheet Properties" ) );
}
else
{
// If we are renaming files, the undo/redo list becomes invalid and must be cleared.
m_frame->ClearUndoRedoList();
m_frame->OnModify();
}
return true;
}
bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilename,
bool* aIsUndoable )
bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilename )
{
wxCHECK( aIsUndoable, false );
wxString msg;
wxFileName sheetFileName( EnsureFileExtension( aNewFilename,
FILEEXT::KiCadSchematicFileExtension ) );
@ -553,7 +541,8 @@ bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilena
if( newAbsoluteFilename.Cmp( oldAbsoluteFilename ) != 0 )
{
// Sheet file name changes cannot be undone.
*aIsUndoable = false;
if( m_isUndoable )
*m_isUndoable = false;
if( useScreen || loadFromFile ) // Load from existing file.
{

View File

@ -37,14 +37,13 @@ class SCH_EDIT_FRAME;
class DIALOG_SHEET_PROPERTIES : public DIALOG_SHEET_PROPERTIES_BASE
{
public:
DIALOG_SHEET_PROPERTIES( SCH_EDIT_FRAME* aParent, SCH_SHEET* aSheet,
bool* aClearAnnotationNewItems,
bool* aUpdateHierarchyNavigator );
DIALOG_SHEET_PROPERTIES( SCH_EDIT_FRAME* aParent, SCH_SHEET* aSheet, bool* aIsUndoable,
bool* aClearAnnotationNewItems, bool* aUpdateHierarchyNavigator );
~DIALOG_SHEET_PROPERTIES() override;
private:
bool onSheetFilenameChanged( const wxString& aNewFilename, bool* aIsUndoable );
bool onSheetFilenameChanged( const wxString& aNewFilename );
bool TransferDataToWindow() override;
bool TransferDataFromWindow() override;
@ -66,6 +65,7 @@ private:
private:
SCH_EDIT_FRAME* m_frame;
SCH_SHEET* m_sheet;
bool* m_isUndoable;
bool* m_clearAnnotationNewItems;
bool* m_updateHierarchyNavigator;

View File

@ -573,6 +573,8 @@ public:
*
* @param aSheet is the sheet to edit
* @param aHierarchy is the current hierarchy containing aSheet
* @param aIsUndoable is a reference to a bool to know if this operation must clear
* the undo-redo list, since the operation is not reversible.
* @param aClearAnnotationNewItems is a reference to a bool to know if the items managed by
* this sheet need to have their annotation cleared i.e. when an existing item list is used.
* it can happens when the edited sheet used an existing file, or becomes a new instance
@ -581,7 +583,7 @@ public:
* the hierarchy navigator panel to be updated.
*/
bool EditSheetProperties( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy,
bool* aClearAnnotationNewItems,
bool* aIsUndoable = nullptr, bool* aClearAnnotationNewItems = nullptr,
bool* aUpdateHierarchyNavigator = nullptr );
void InitSheet( SCH_SHEET* aSheet, const wxString& aNewFilename );

View File

@ -591,14 +591,14 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aCurr
bool SCH_EDIT_FRAME::EditSheetProperties( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy,
bool* aClearAnnotationNewItems,
bool* aIsUndoable, bool* aClearAnnotationNewItems,
bool* aUpdateHierarchyNavigator )
{
if( aSheet == nullptr || aHierarchy == nullptr )
return false;
// Get the new texts
DIALOG_SHEET_PROPERTIES dlg( this, aSheet, aClearAnnotationNewItems,
DIALOG_SHEET_PROPERTIES dlg( this, aSheet, aIsUndoable, aClearAnnotationNewItems,
aUpdateHierarchyNavigator );
if( dlg.ShowModal() == wxID_CANCEL )

View File

@ -2585,7 +2585,7 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent )
getViewControls()->CaptureCursor( false );
if( m_frame->EditSheetProperties( static_cast<SCH_SHEET*>( sheet ),
&m_frame->GetCurrentSheet(), nullptr ) )
&m_frame->GetCurrentSheet() ) )
{
m_view->ClearPreview();

View File

@ -1293,7 +1293,7 @@ int SCH_EDIT_TOOL::RepeatDrawItem( const TOOL_EVENT& aEvent )
// Clear out the filename so that the user can pick a new one
sheet->SetFileName( wxEmptyString );
sheet->GetScreen()->SetFileName( wxEmptyString );
restore_state = !m_frame->EditSheetProperties( sheet, currentSheet, nullptr );
restore_state = !m_frame->EditSheetProperties( sheet, currentSheet );
}
}
@ -1864,8 +1864,9 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent )
case SCH_SHEET_T:
{
SCH_SHEET* sheet = static_cast<SCH_SHEET*>( curr_item );
bool doClearAnnotation;
bool doRefresh = false;
bool isUndoable = false;
bool doClearAnnotation = false;
bool okPressed = false;
bool updateHierarchyNavigator = false;
// Keep track of existing sheet paths. EditSheet() can modify this list.
@ -1873,9 +1874,23 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent )
// we've got a valid hierarchy to begin with.
SCH_SHEET_LIST originalHierarchy( &m_frame->Schematic().Root(), true );
doRefresh = m_frame->EditSheetProperties( sheet, &m_frame->GetCurrentSheet(),
SCH_COMMIT commit( m_toolMgr );
commit.Modify( sheet, m_frame->GetScreen() );
okPressed = m_frame->EditSheetProperties( sheet, &m_frame->GetCurrentSheet(), &isUndoable,
&doClearAnnotation, &updateHierarchyNavigator );
if( okPressed )
{
if( isUndoable )
commit.Push( _( "Edit Sheet Properties" ) );
}
else
{
// If we are renaming files, the undo/redo list becomes invalid and must be cleared.
m_frame->ClearUndoRedoList();
m_frame->OnModify();
}
// If the sheet file is changed and new sheet contents are loaded then we have to
// clear the annotations on the new content (as it may have been set from some other
// sheet path reference)
@ -1892,7 +1907,7 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent )
sheet->GetScreen()->ClearAnnotation( &m_frame->GetCurrentSheet(), false );
}
if( doRefresh )
if( okPressed )
m_frame->GetCanvas()->Refresh();
if( updateHierarchyNavigator )