realAscot/kicad-source
1
0
Fork 0
mirror of https://gitlab.com/kicad/code/kicad.git synced 2025-09-14 02:03:12 +02:00
Code Issues Packages Projects Releases Wiki Activity

Be more pedantic about RECURSE_MODE.

Browse Source 
Also fixes an invalid iterator bug.

Also adds mirroring support for PCB_GROUPs.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/21107
This commit is contained in:
Jeff Young 2025-06-12 11:20:33 +01:00
parent c159b06f57
commit 7fbf51b17c
24 changed files with 73 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
common/commit.cpp
View File

@ -63,7 +63,7 @@ COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN* aS
makeEntry( aItem, CHT_REMOVE | flag, makeImage( aItem ), aScreen );
if( EDA_GROUP* parentGroup = aItem->GetParentGroup() )
Modify( parentGroup->AsEdaItem(), aScreen );
Modify( parentGroup->AsEdaItem(), aScreen, RECURSE_MODE::NO_RECURSE );
break;

2
common/dialogs/dialog_group_properties.cpp
View File

@ -95,7 +95,7 @@ bool DIALOG_GROUP_PROPERTIES::TransferDataFromWindow()
m_commit->Modify( item, m_frame->GetScreen() );
if( existingGroup )
m_commit->Modify( existingGroup->AsEdaItem(), m_frame->GetScreen() );
m_commit->Modify( existingGroup->AsEdaItem(), m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
}
}

21
common/tool/group_tool.cpp
View File

@ -171,7 +171,7 @@ int GROUP_TOOL::Ungroup( const TOOL_EVENT& aEvent )
for( EDA_ITEM* member : group->GetItems() )
{
m_commit->Modify( member, m_frame->GetScreen() );
m_commit->Modify( member, m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
toSelect.push_back( member );
}
@ -194,8 +194,8 @@ int GROUP_TOOL::AddToGroup( const TOOL_EVENT& aEvent )
{
const SELECTION& selection = m_selectionTool->GetSelection();
EDA_ITEM* group = nullptr;
EDA_ITEMS toAdd;
EDA_GROUP* group = nullptr;
EDA_ITEMS toAdd;
for( EDA_ITEM* item : selection )
{
@ -205,7 +205,7 @@ int GROUP_TOOL::AddToGroup( const TOOL_EVENT& aEvent )
if( group != nullptr )
return 0;
group = item;
group = dynamic_cast<EDA_GROUP*>( item );
}
else if( !item->GetParentGroup() )
{
@ -218,25 +218,26 @@ int GROUP_TOOL::AddToGroup( const TOOL_EVENT& aEvent )
m_toolMgr->RunAction( ACTIONS::selectionClear );
m_commit->Modify( group, m_frame->GetScreen() );
m_commit->Modify( group->AsEdaItem(), m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
for( EDA_ITEM* item : toAdd )
{
EDA_GROUP* existingGroup = item->GetParentGroup();
KIID existingGroupId = existingGroup ? existingGroup->AsEdaItem()->m_Uuid : niluuid;
if( existingGroupId != group->m_Uuid )
if( existingGroup != group )
{
m_commit->Modify( item, m_frame->GetScreen() );
if( existingGroup )
m_commit->Modify( existingGroup->AsEdaItem(), m_frame->GetScreen() );
m_commit->Modify( existingGroup->AsEdaItem(), m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
group->AddItem( item );
}
}
m_commit->Push( _( "Add Items to Group" ) );
m_selectionTool->AddItemToSel( group );
m_selectionTool->AddItemToSel( group->AsEdaItem() );
m_toolMgr->PostEvent( EVENTS::SelectedItemsModified );
m_frame->OnModify();
@ -255,7 +256,7 @@ int GROUP_TOOL::RemoveFromGroup( const TOOL_EVENT& aEvent )
{
if( EDA_GROUP* group = item->GetParentGroup() )
{
m_commit->Modify( group->AsEdaItem(), m_frame->GetScreen() );
m_commit->Modify( group->AsEdaItem(), m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
m_commit->Modify( item, m_frame->GetScreen() );
group->RemoveItem( item );
}

6
eeschema/sch_item.cpp
View File

@ -182,10 +182,10 @@ SCH_ITEM* SCH_ITEM::Duplicate( bool addToParentGroup, SCH_COMMIT* aCommit, bool
{
wxCHECK_MSG( aCommit, newItem, "Must supply a commit to update parent group" );
if( newItem->GetParentGroup() )
if( EDA_GROUP* group = newItem->GetParentGroup() )
{
aCommit->Modify( newItem->GetParentGroup()->AsEdaItem() );
newItem->GetParentGroup()->AddItem( newItem );
aCommit->Modify( group->AsEdaItem(), nullptr, RECURSE_MODE::NO_RECURSE );
group->AddItem( newItem );
}
}

2
eeschema/tools/assign_footprints.cpp
View File

@ -99,7 +99,7 @@ void SCH_EDITOR_CONTROL::AssignFootprints( const std::string& aChangedSetOfRefer
isChanged = true;
SCH_SCREEN* screen = refs[ii].GetSheetPath().LastScreen();
commit.Modify( symbol, screen );
commit.Modify( symbol, screen, RECURSE_MODE::NO_RECURSE );
footprintField->SetText( footprint );
}
}

2
eeschema/tools/backannotate.cpp
View File

@ -382,7 +382,7 @@ void BACK_ANNOTATE::applyChangelist()
return b ? _( "true" ) : _( "false" );
};
if( !m_dryRun )
commit.Modify( symbol, screen );
commit.Modify( symbol, screen, RECURSE_MODE::NO_RECURSE );
if( m_processReferences && ref.GetRef() != fpData.m_ref && !skip )
{

2
eeschema/tools/sch_edit_tool.cpp
View File

@ -1530,7 +1530,7 @@ int SCH_EDIT_TOOL::RepeatDrawItem( const TOOL_EVENT& aEvent )
{
if( newItem->IsGroupableType() )
{
commit.Modify( enteredGroup );
commit.Modify( enteredGroup, m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
enteredGroup->AddItem( newItem );
}
}

4
eeschema/tools/sch_editor_control.cpp
View File

@ -471,7 +471,7 @@ int SCH_EDITOR_CONTROL::ExportSymbolsToLibrary( const TOOL_EVENT& aEvent )
wxCHECK2( parentScreen, continue );
commit.Modify( symbol, parentScreen );
commit.Modify( symbol, parentScreen, RECURSE_MODE::NO_RECURSE );
symbol->SetLibId( id );
append = true;
}
@ -2492,7 +2492,7 @@ int SCH_EDITOR_CONTROL::IncrementAnnotations( const TOOL_EVENT& aEvent )
num += dlg.m_Increment->GetValue();
fullRef << num;
commit.Modify( ref.GetSymbol(), sheet.LastScreen() );
commit.Modify( ref.GetSymbol(), sheet.LastScreen(), RECURSE_MODE::NO_RECURSE );
ref.GetSymbol()->SetRef( &sheet, From_UTF8( fullRef.c_str() ) );
}
}

4
eeschema/tools/sch_find_replace_tool.cpp
View File

@ -358,7 +358,7 @@ int SCH_FIND_REPLACE_TOOL::ReplaceAndFindNext( const TOOL_EVENT& aEvent )
SCH_COMMIT commit( m_frame );
SCH_ITEM* sch_item = static_cast<SCH_ITEM*>( item );
commit.Modify( sch_item, sheet->LastScreen() );
commit.Modify( sch_item, sheet->LastScreen(), RECURSE_MODE::NO_RECURSE );
if( item->Replace( data, sheet ) )
{
@ -398,7 +398,7 @@ int SCH_FIND_REPLACE_TOOL::ReplaceAll( const TOOL_EVENT& aEvent )
auto doReplace =
[&]( SCH_ITEM* aItem, SCH_SHEET_PATH* aSheet, EDA_SEARCH_DATA& aData )
{
commit.Modify( aItem, aSheet->LastScreen() );
commit.Modify( aItem, aSheet->LastScreen(), RECURSE_MODE::NO_RECURSE );
if( aItem->Replace( aData, aSheet ) )
{

21
eeschema/tools/sch_group_tool.cpp
View File

@ -131,15 +131,19 @@ int SCH_GROUP_TOOL::Group( const TOOL_EVENT& aEvent )
SCH_SELECTION_TOOL* selTool = m_toolMgr->GetTool<SCH_SELECTION_TOOL>();
SCH_SELECTION selection = selTool->RequestSelection();
for( EDA_ITEM* item : selection.GetItems() )
// Iterate from the back so we don't have to worry about removals.
for( int ii = selection.GetSize() - 1; ii >= 0; --ii )
{
SCH_ITEM* schItem = static_cast<SCH_ITEM*>( item );
if( !selection[ii]->IsSCH_ITEM() )
{
selection.Remove( selection[ii] );
continue;
}
SCH_ITEM* schItem = static_cast<SCH_ITEM*>( selection[ii] );
if( !schItem->IsGroupableType() )
selection.Remove( item );
if( isSymbolEditor && schItem->GetParentSymbol() )
selection.Remove( item );
selection.Remove( schItem );
}
if( selection.Empty() )
@ -155,7 +159,10 @@ int SCH_GROUP_TOOL::Group( const TOOL_EVENT& aEvent )
for( EDA_ITEM* eda_item : selection )
{
m_commit->Modify( eda_item, screen );
if( EDA_GROUP* existingGroup = eda_item->GetParentGroup() )
m_commit->Modify( existingGroup->AsEdaItem(), screen, RECURSE_MODE::NO_RECURSE );
m_commit->Modify( eda_item, screen, RECURSE_MODE::NO_RECURSE );
group->AddItem( eda_item );
}

4
eeschema/tools/sch_move_tool.cpp
View File

@ -637,7 +637,7 @@ bool SCH_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aComm
{
if( schItem->IsGroupableType() && !schItem->GetParentGroup() )
{
aCommit->Modify( enteredGroup );
aCommit->Modify( enteredGroup, m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
enteredGroup->AddItem( schItem );
}
}
@ -648,7 +648,7 @@ bool SCH_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aComm
}
else
{
aCommit->Modify( schItem, m_frame->GetScreen() );
aCommit->Modify( schItem, m_frame->GetScreen(), RECURSE_MODE::RECURSE );
}
schItem->SetFlags( IS_MOVING );

2
eeschema/tools/symbol_editor_move_tool.cpp
View File

@ -244,7 +244,7 @@ bool SYMBOL_EDITOR_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COM
{
if( schItem->IsGroupableType() && !item->GetParentGroup() )
{
aCommit->Modify( enteredGroup );
aCommit->Modify( enteredGroup, m_frame->GetScreen(), RECURSE_MODE::NO_RECURSE );
enteredGroup->AddItem( schItem );
}
}

5
eeschema/widgets/sch_properties_panel.cpp
View File

@ -198,8 +198,11 @@ void SCH_PROPERTIES_PANEL::valueChanged( wxPropertyGridEvent& aEvent )
for( EDA_ITEM* edaItem : selection )
{
if( !edaItem->IsSCH_ITEM() )
continue;
SCH_ITEM* item = static_cast<SCH_ITEM*>( edaItem );
changes.Modify( item, screen );
changes.Modify( item, screen, RECURSE_MODE::NO_RECURSE );
item->Set( property, newValue );
if( SCH_SYMBOL* symbol = dynamic_cast<SCH_SYMBOL*>( item ) )

6
pcbnew/board_item.cpp
View File

@ -277,10 +277,10 @@ BOARD_ITEM* BOARD_ITEM::Duplicate( bool addToParentGroup, BOARD_COMMIT* aCommit
{
wxCHECK_MSG( aCommit, dupe, "Must supply a commit to update parent group" );
if( dupe->GetParentGroup() )
if( EDA_GROUP* group = dupe->GetParentGroup() )
{
aCommit->Modify( dupe->GetParentGroup()->AsEdaItem() );
dupe->GetParentGroup()->AddItem( dupe );
aCommit->Modify( group->AsEdaItem(), nullptr, RECURSE_MODE::NO_RECURSE );
group->AddItem( dupe );
}
}

8
pcbnew/netlist_reader/board_netlist_updater.cpp
View File

@ -878,7 +878,7 @@ bool BOARD_NETLIST_UPDATER::updateFootprintGroup( FOOTPRINT* aPcbFootprint,
EscapeHTML( existingGroup->GetName() ) );
changed = true;
m_commit.Modify( existingGroup );
m_commit.Modify( existingGroup, nullptr, RECURSE_MODE::NO_RECURSE );
existingGroup->RemoveItem( aPcbFootprint );
}
@ -915,7 +915,7 @@ bool BOARD_NETLIST_UPDATER::updateFootprintGroup( FOOTPRINT* aPcbFootprint,
}
else
{
m_commit.Modify( newGroup->AsEdaItem() );
m_commit.Modify( newGroup->AsEdaItem(), nullptr, RECURSE_MODE::NO_RECURSE );
}
newGroup->AddItem( aPcbFootprint );
@ -1385,7 +1385,7 @@ bool BOARD_NETLIST_UPDATER::updateGroups( NETLIST& aNetlist )
wxString msg;
msg.Printf( _( "Changed group name to '%s' to '%s'." ), EscapeHTML( pcbGroup->GetName() ),
EscapeHTML( netlistGroup->name ) );
m_commit.Modify( pcbGroup->AsEdaItem() );
m_commit.Modify( pcbGroup->AsEdaItem(), nullptr, RECURSE_MODE::NO_RECURSE );
pcbGroup->SetName( netlistGroup->name );
}
}
@ -1404,7 +1404,7 @@ bool BOARD_NETLIST_UPDATER::updateGroups( NETLIST& aNetlist )
wxString msg;
msg.Printf( _( "Changed group library link to '%s'." ),
EscapeHTML( netlistGroup->libId.GetUniStringLibId() ) );
m_commit.Modify( pcbGroup->AsEdaItem() );
m_commit.Modify( pcbGroup->AsEdaItem(), nullptr, RECURSE_MODE::NO_RECURSE );
pcbGroup->SetDesignBlockLibId( netlistGroup->libId );
}
}

2
pcbnew/pcb_design_block_utils.cpp
View File

@ -435,7 +435,7 @@ bool PCB_EDIT_FRAME::SaveSelectionToDesignBlock( const LIB_ID& aLibId )
{
BOARD_COMMIT commit( m_toolManager );
commit.Modify( group );
commit.Modify( group, nullptr, RECURSE_MODE::NO_RECURSE );
group->SetDesignBlockLibId( aLibId );
commit.Push( "Set Group Design Block Link" );

2
pcbnew/pcb_edit_frame.cpp
View File

@ -2311,7 +2311,7 @@ void PCB_EDIT_FRAME::ExchangeFootprint( FOOTPRINT* aExisting, FOOTPRINT* aNew,
if( parentGroup )
{
aCommit.Modify( parentGroup->AsEdaItem() );
aCommit.Modify( parentGroup->AsEdaItem(), nullptr, RECURSE_MODE::NO_RECURSE );
parentGroup->RemoveItem( aExisting );
parentGroup->AddItem( aNew );
}

2
pcbnew/pcb_generator.cpp
View File

@ -58,7 +58,7 @@ PCB_GENERATOR* PCB_GENERATOR::DeepClone() const
void PCB_GENERATOR::EditStart( GENERATOR_TOOL* aTool, BOARD* aBoard, BOARD_COMMIT* aCommit )
{
aCommit->Modify( this );
aCommit->Modify( this, nullptr, RECURSE_MODE::NO_RECURSE );
}

4
pcbnew/tools/array_tool.cpp
View File

@ -202,7 +202,7 @@ void ARRAY_TOOL::onDialogClosed( wxCloseEvent& aEvent )
if( item == nullptr )
break;
commit.Modify( item );
commit.Modify( item, nullptr, RECURSE_MODE::RECURSE );
// Transform is a relative move, so when arranging the transform needs to start from
// the same point for each item, e.g. the first item's position
@ -263,7 +263,7 @@ void ARRAY_TOOL::onDialogClosed( wxCloseEvent& aEvent )
// we might still modify it (position or label)
this_item = item;
commit.Modify( this_item );
commit.Modify( this_item, nullptr, RECURSE_MODE::RECURSE );
TransformItem( *m_array_opts, arraySize - 1, *this_item );
}

7
pcbnew/tools/edit_tool.cpp
View File

@ -2190,6 +2190,7 @@ const std::vector<KICAD_T> EDIT_TOOL::MirrorableItems = {
PCB_TRACE_T,
PCB_ARC_T,
PCB_VIA_T,
PCB_GROUP_T,
PCB_GENERATOR_T,
};
@ -2251,7 +2252,7 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
continue;
if( !item->IsNew() && !item->IsMoving() )
commit->Modify( item );
commit->Modify( item, nullptr, RECURSE_MODE::RECURSE );
// modify each object as necessary
switch( item->Type() )
@ -2287,6 +2288,10 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
static_cast<PCB_TRACK*>( item )->Mirror( mirrorPoint, flipDirection );
break;
case PCB_GROUP_T:
static_cast<PCB_GROUP*>( item )->Mirror( mirrorPoint, flipDirection );
break;
case PCB_GENERATOR_T:
static_cast<PCB_GENERATOR*>( item )->Mirror( mirrorPoint, flipDirection );
break;

16
pcbnew/tools/edit_tool_move_fct.cpp
View File

@ -194,11 +194,11 @@ int EDIT_TOOL::PackAndMoveFootprints( const TOOL_EVENT& aEvent )
BOX2I footprintsBbox;
for( FOOTPRINT* item : footprintsToPack )
for( FOOTPRINT* fp : footprintsToPack )
{
commit.Modify( item );
item->SetFlags( IS_MOVING );
footprintsBbox.Merge( item->GetBoundingBox( false ) );
commit.Modify( fp );
fp->SetFlags( IS_MOVING );
footprintsBbox.Merge( fp->GetBoundingBox( false ) );
}
SpreadFootprints( &footprintsToPack, footprintsBbox.Normalize().GetOrigin(), false );
@ -582,13 +582,9 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit
m_toolMgr->RunSynchronousAction( PCB_ACTIONS::genStartEdit, aCommit,
static_cast<PCB_GENERATOR*>( item ) );
}
else if( item->Type() == PCB_GROUP_T || item->Type() == PCB_GENERATOR_T )
{
aCommit->Modify( item, nullptr, RECURSE_MODE::RECURSE );
}
else
{
aCommit->Modify( item );
aCommit->Modify( item, nullptr, RECURSE_MODE::RECURSE );
}
item->SetFlags( IS_MOVING );
@ -762,7 +758,7 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit
updateStatusPopup( nextItem, itemIdx + 1, orig_items.size() );
// Pick up new item
aCommit->Modify( nextItem );
aCommit->Modify( nextItem, nullptr, RECURSE_MODE::RECURSE );
nextItem->Move( controls->GetCursorPosition( true ) - nextItem->GetPosition() );
continue;

2
pcbnew/tools/pcb_control.cpp
View File

@ -1546,7 +1546,7 @@ bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, std::vector<BOARD_ITEM
{
if( item->IsGroupableType() && !item->GetParentGroup() )
{
aCommit->Modify( enteredGroup );
aCommit->Modify( enteredGroup, nullptr, RECURSE_MODE::NO_RECURSE );
enteredGroup->AddItem( item );
}
}

3
pcbnew/tools/pcb_group_tool.cpp
View File

@ -176,6 +176,9 @@ int PCB_GROUP_TOOL::Group( const TOOL_EVENT& aEvent )
{
if( eda_item->IsBOARD_ITEM() )
{
if( EDA_GROUP* existingGroup = eda_item->GetParentGroup() )
m_commit->Modify( existingGroup->AsEdaItem(), nullptr, RECURSE_MODE::NO_RECURSE );
m_commit->Modify( eda_item );
group->AddItem( eda_item );
}

2
pcbnew/widgets/pcb_properties_panel.cpp
View File

@ -233,7 +233,7 @@ void PCB_PROPERTIES_PANEL::valueChanged( wxPropertyGridEvent& aEvent )
continue;
BOARD_ITEM* item = static_cast<BOARD_ITEM*>( edaItem );
changes.Modify( item );
changes.Modify( item, nullptr, RECURSE_MODE::NO_RECURSE );
item->Set( property, newValue );
}