Don't add things to the selection that aren't in the commit.

Don't process line-pairs when one has already been deleted.

Clean up created line segments when operation is cancelled.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/18122

pcbnew/tools/edit_tool.cpp

@@ -1181,7 +1181,7 @@ int EDIT_TOOL::FilletTracks( const TOOL_EVENT& aEvent )
  * @return std::optional<int> the fillet radius or std::nullopt if no
  * valid fillet specified
  */
-static std::optional<int> GetFilletParams( PCB_BASE_EDIT_FRAME& aFrame, wxString& aErrorMsg )
+static std::optional<int> GetFilletParams( PCB_BASE_EDIT_FRAME& aFrame )
 {
     // Store last used fillet radius to allow pressing "enter" if repeat fillet is required
     static int filletRadius = 0;
@@ -1205,8 +1205,7 @@ static std::optional<int> GetFilletParams( PCB_BASE_EDIT_FRAME& aFrame, wxString
  * @return std::optional<int> the chamfer parameters or std::nullopt if no
  * valid fillet specified
  */
-static std::optional<CHAMFER_PARAMS> GetChamferParams( PCB_BASE_EDIT_FRAME& aFrame,
+static std::optional<CHAMFER_PARAMS> GetChamferParams( PCB_BASE_EDIT_FRAME& aFrame )
-                                                       wxString&            aErrorMsg )
 {
     // Non-zero and the KLC default for Fab layer chamfers
     const int default_setback = pcbIUScale.mmToIU( 1 );
@@ -1304,19 +1303,45 @@ int EDIT_TOOL::ModifyLines( const TOOL_EVENT& aEvent )
         }
     }
 
-    for( PCB_SHAPE* item : lines_to_add )
+    int segmentCount = selection.CountType( PCB_SHAPE_LOCATE_SEGMENT_T ) + lines_to_add.size();
-        selection.Add( item );
 
-    for( PCB_SHAPE* item : items_to_remove )
+    if( aEvent.IsAction( &PCB_ACTIONS::extendLines ) && segmentCount != 2 )
-        selection.Remove( item );
-
-    if( selection.CountType( PCB_SHAPE_LOCATE_SEGMENT_T ) < 2 )
     {
-        frame()->ShowInfoBarMsg( _( "A shape with least two lines must be selected." ) );
+        frame()->ShowInfoBarMsg( _( "Exactly two lines must be selected to extend them." ) );
+
+        for( PCB_SHAPE* line : lines_to_add )
+            delete line;
+
+        return 0;
+    }
+    else if( segmentCount < 2 )
+    {
+        frame()->ShowInfoBarMsg( _( "A shape with at least two lines must be selected." ) );
+
+        for( PCB_SHAPE* line : lines_to_add )
+            delete line;
+
         return 0;
     }
 
-    BOARD_COMMIT commit{ this };
+    BOARD_COMMIT commit( this );
+
+    // Items created like lines from a rectangle
+    for( PCB_SHAPE* item : lines_to_add )
+    {
+        commit.Add( item );
+        selection.Add( item );
+    }
+
+    // Remove items like rectangles that we decomposed into lines
+    for( PCB_SHAPE* item : items_to_remove )
+    {
+        selection.Remove( item );
+        commit.Remove( item );
+    }
+
+    for( EDA_ITEM* item : selection )
+        item->ClearFlags( STRUCT_DELETED );
 
     // List of thing to select at the end of the operation
     // (doing it as we go will invalidate the iterator)
@@ -1352,6 +1377,7 @@ int EDIT_TOOL::ModifyLines( const TOOL_EVENT& aEvent )
     auto item_removal_handler =
             [&]( PCB_SHAPE& aItem )
             {
+                aItem.SetFlags( STRUCT_DELETED );
                 any_items_removed = true;
                 items_to_deselect_on_success.push_back( &aItem );
                 commit.Remove( &aItem );
@@ -1363,11 +1389,10 @@ int EDIT_TOOL::ModifyLines( const TOOL_EVENT& aEvent )
 
     // Construct an appropriate tool
     std::unique_ptr<PAIRWISE_LINE_ROUTINE> pairwise_line_routine;
-    wxString                               error_message;
 
     if( aEvent.IsAction( &PCB_ACTIONS::filletLines ) )
     {
-        std::optional<int> filletRadiusIU = GetFilletParams( *frame(), error_message );
+        std::optional<int> filletRadiusIU = GetFilletParams( *frame() );
 
         if( filletRadiusIU.has_value() )
         {
@@ -1378,7 +1403,7 @@ int EDIT_TOOL::ModifyLines( const TOOL_EVENT& aEvent )
     }
     else if( aEvent.IsAction( &PCB_ACTIONS::chamferLines ) )
     {
-        std::optional<CHAMFER_PARAMS> chamfer_params = GetChamferParams( *frame(), error_message );
+        std::optional<CHAMFER_PARAMS> chamfer_params = GetChamferParams( *frame() );
 
         if( chamfer_params.has_value() )
         {
@@ -1389,23 +1414,14 @@ int EDIT_TOOL::ModifyLines( const TOOL_EVENT& aEvent )
     }
     else if( aEvent.IsAction( &PCB_ACTIONS::extendLines ) )
     {
-        if( selection.CountType( PCB_SHAPE_LOCATE_SEGMENT_T ) != 2 )
+        pairwise_line_routine = std::make_unique<LINE_EXTENSION_ROUTINE>( frame()->GetModel(),
-        {
+                                                                          change_handler );
-            error_message = _( "Exactly two lines must be selected to extend them." );
-        }
-        else
-        {
-            pairwise_line_routine = std::make_unique<LINE_EXTENSION_ROUTINE>( frame()->GetModel(),
-                                                                              change_handler );
-        }
     }
 
     if( !pairwise_line_routine )
     {
-        // Didn't construct any mofication routine - could be an error or cancellation
+        // Didn't construct any mofication routine - user must have cancelled
-        if( !error_message.empty() )
+        commit.Revert();
-            frame()->ShowInfoBarMsg( error_message );
-
         return 0;
     }
 
@@ -1413,28 +1429,16 @@ int EDIT_TOOL::ModifyLines( const TOOL_EVENT& aEvent )
     alg::for_all_pairs( selection.begin(), selection.end(),
                         [&]( EDA_ITEM* a, EDA_ITEM* b )
                         {
-                            PCB_SHAPE* line_a = static_cast<PCB_SHAPE*>( a );
+                            if( ( a->GetFlags() & STRUCT_DELETED ) == 0
-                            PCB_SHAPE* line_b = static_cast<PCB_SHAPE*>( b );
+                                && ( b->GetFlags() & STRUCT_DELETED ) == 0 )
+                            {
+                                PCB_SHAPE* line_a = static_cast<PCB_SHAPE*>( a );
+                                PCB_SHAPE* line_b = static_cast<PCB_SHAPE*>( b );
 
-                            pairwise_line_routine->ProcessLinePair( *line_a, *line_b );
+                                pairwise_line_routine->ProcessLinePair( *line_a, *line_b );
+                            }
                         } );
 
-    // Items created like lines from a rectangle
-    // Some of these might have been changed by the tool, but we need to
-    // add *all* of them to the selection and commit them
-    for( PCB_SHAPE* item : lines_to_add )
-    {
-        m_selectionTool->AddItemToSel( item, true );
-        commit.Add( item );
-    }
-
-    // Remove items like rectangles that we decomposed into lines
-    for( PCB_SHAPE* item : items_to_remove )
-    {
-        commit.Remove( item );
-        m_selectionTool->RemoveItemFromSel( item, true );
-    }
-
     // Select added and modified items
     for( PCB_SHAPE* item : items_to_select_on_success )
         m_selectionTool->AddItemToSel( item, true );

pcbnew/tools/item_modification_routine.cpp

@@ -43,6 +43,7 @@ bool ITEM_MODIFICATION_ROUTINE::ModifyLineOrDeleteIfZeroLength( PCB_SHAPE& aLine
     wxASSERT_MSG( aLine.GetShape() == SHAPE_T::SEGMENT, "Can only modify segments" );
 
     const bool removed = aSeg.Length() == 0;
+
     if( !removed )
     {
         // Mark modified, then change it
@@ -55,6 +56,7 @@ bool ITEM_MODIFICATION_ROUTINE::ModifyLineOrDeleteIfZeroLength( PCB_SHAPE& aLine
         // The line has become zero length - delete it
         GetHandler().DeleteItem( aLine );
     }
+
     return removed;
 }