sch groups: clear group membership on destruction, add tests

eeschema/sch_screen.cpp

@@ -46,6 +46,7 @@
 #include <junction_helpers.h>
 #include <sch_pin.h>
 #include <sch_symbol.h>
+#include <sch_group.h>
 #include <sch_junction.h>
 #include <sch_line.h>
 #include <sch_marker.h>
@@ -305,6 +306,13 @@ void SCH_SCREEN::FreeDrawList()
     std::copy_if( m_rtree.begin(), m_rtree.end(), std::back_inserter( delete_list ),
             []( SCH_ITEM* aItem )
             {
+                // Untangle group parents before doing any deleting
+                if( aItem->Type() == SCH_GROUP_T )
+                {
+                    for( EDA_ITEM* item : static_cast<SCH_GROUP*>(aItem)->GetItems() )
+                        item->SetParentGroup( nullptr );
+                }
+
                 return ( aItem->Type() != SCH_SHEET_PIN_T && aItem->Type() != SCH_FIELD_T );
             } );
 

eeschema/sch_symbol.cpp

@@ -1797,6 +1797,7 @@ int SCH_SYMBOL::GetOrientation() const
     // Try to find the current transform option:
     TRANSFORM transform = m_transform;
     SCH_SYMBOL temp( *this );
+    temp.SetParentGroup( nullptr );
 
     for( int type_rotate : rotate_values )
     {

eeschema/symbol.cpp

@@ -22,6 +22,7 @@
  */
 
 #include <symbol.h>
+#include <sch_group.h>
 
 
 std::vector<int> SYMBOL::ViewGetLayers() const
@@ -53,3 +54,12 @@ std::vector<int> SYMBOL::ViewGetLayers() const
 }
 
 
+SYMBOL::~SYMBOL()
+{
+    // Untangle group parents before doing any deleting
+    RunOnChildren( []( EDA_ITEM* item )
+                        {
+                            if( item->Type() == SCH_GROUP_T)
+                                item->SetParentGroup( nullptr );
+                        }, RECURSE_MODE::RECURSE );
+}

eeschema/symbol.h

@@ -104,7 +104,7 @@ public:
         return *this;
     };
 
-    ~SYMBOL() override { };
+    ~SYMBOL() override;
 
     virtual const LIB_ID& GetLibId() const = 0;
     virtual wxString GetDescription() const = 0;

eeschema/tools/sch_edit_tool.cpp

@@ -760,6 +760,7 @@ const std::vector<KICAD_T> SCH_EDIT_TOOL::RotatableItems = {
     SCH_TABLECELL_T,    // will be promoted to parent table(s)
     SCH_LABEL_T,
     SCH_GLOBAL_LABEL_T,
+    SCH_GROUP_T,
     SCH_HIER_LABEL_T,
     SCH_DIRECTIVE_LABEL_T,
     SCH_FIELD_T,
@@ -778,7 +779,7 @@ const std::vector<KICAD_T> SCH_EDIT_TOOL::RotatableItems = {
 int SCH_EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
 {
     bool           clockwise = ( aEvent.Matches( SCH_ACTIONS::rotateCW.MakeEvent() ) );
-    SCH_SELECTION& selection = m_selectionTool->RequestSelection( RotatableItems, true );
+    SCH_SELECTION& selection = m_selectionTool->RequestSelection( RotatableItems, true, true );
 
     if( selection.GetSize() == 0 )
         return 0;
@@ -1056,7 +1057,7 @@ int SCH_EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
 
 int SCH_EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
 {
-    SCH_SELECTION& selection = m_selectionTool->RequestSelection( RotatableItems );
+    SCH_SELECTION& selection = m_selectionTool->RequestSelection( RotatableItems, false, true );
 
     if( selection.GetSize() == 0 )
         return 0;

eeschema/tools/sch_selection_tool.cpp

@@ -1847,7 +1847,8 @@ void SCH_SELECTION_TOOL::GuessSelectionCandidates( SCH_COLLECTOR& collector, con
 
 
 SCH_SELECTION& SCH_SELECTION_TOOL::RequestSelection( const std::vector<KICAD_T>& aScanTypes,
-                                                     bool aPromoteCellSelections )
+                                                     bool aPromoteCellSelections,
+                                                     bool aPromoteGroups )
 {
     bool anyUnselected = false;
     bool anySelected = false;
@@ -1881,6 +1882,38 @@ SCH_SELECTION& SCH_SELECTION_TOOL::RequestSelection( const std::vector<KICAD_T>&
             updateReferencePoint();
     }
 
+    if( aPromoteGroups )
+    {
+        for( int i = (int) m_selection.GetSize() - 1; i >= 0; --i )
+        {
+            EDA_ITEM* item = (EDA_ITEM*) m_selection.GetItem( i );
+
+            std::set<EDA_ITEM*> selectedChildren;
+
+            if( item->Type() == SCH_GROUP_T )
+            {
+                static_cast<SCH_ITEM*>(item)
+                    ->RunOnChildren( [&]( SCH_ITEM* item )
+                                        {
+                                            if( item->IsType( aScanTypes ) )
+                                                selectedChildren.insert( item );
+                                        },
+                                        RECURSE_MODE::RECURSE );
+                unselect( item );
+                anyUnselected = true;
+            }
+
+            for( EDA_ITEM* child : selectedChildren )
+            {
+                if( !child->IsSelected() )
+                {
+                    select( child );
+                    anySelected = true;
+                }
+            }
+        }
+    }
+
     if( aPromoteCellSelections )
     {
         std::set<EDA_ITEM*> parents;

eeschema/tools/sch_selection_tool.h

@@ -112,9 +112,14 @@ public:
      *
      * @param aScanTypes [optional] List of item types that are acceptable for selection.
      * @return either the current selection or, if empty, the selection at the cursor.
+     *
+     * @param aPromoteCellSelections [optional] If true, cell selections are promoted to their parent
+     *
+     * @param aPromoteGroups [optional] If true, group selections are promoted the items within the group
      */
     SCH_SELECTION& RequestSelection( const std::vector<KICAD_T>& aScanTypes = { SCH_LOCATE_ANY_T },
-                                     bool                        aPromoteCellSelections = false );
+                                     bool aPromoteCellSelections = false,
+                                     bool aPromoteGroups = false );
 
     /**
      * Perform a click-type selection at a point (usually the cursor position).

qa/tests/eeschema/CMakeLists.txt

@@ -71,6 +71,7 @@ set( QA_EESCHEMA_SRCS
     test_ee_item.cpp
     test_incremental_netlister.cpp
     test_legacy_power_symbols.cpp
+    test_sch_group.cpp
     test_pin_numbers.cpp
     test_sch_netclass.cpp
     test_sch_pin.cpp

qa/tests/eeschema/test_sch_group.cpp

@@ -0,0 +1,127 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright The KiCad Developers, see AUTHORS.TXT for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+/**
+ * @file
+ * Test suite for SCH_SHEET
+ */
+
+#include <qa_utils/wx_utils/unit_test_utils.h>
+
+// Code under test
+#include <schematic.h>
+#include <sch_sheet.h>
+#include <sch_screen.h>
+#include <sch_group.h>
+#include <sch_symbol.h>
+#include <sch_pin.h>
+#include <lib_symbol.h>
+
+#include <qa_utils/uuid_test_utils.h>
+#include <qa_utils/wx_utils/wx_assert.h>
+
+#include "eeschema_test_utils.h"
+
+class TEST_SCH_GROUP_FIXTURE : public KI_TEST::SCHEMATIC_TEST_FIXTURE
+{
+public:
+    TEST_SCH_GROUP_FIXTURE() :
+            m_schematic( nullptr ),
+            m_screen( &m_schematic )
+    {
+        m_manager.LoadProject( "" );
+        m_schematic.SetProject( &m_manager.Prj() );
+
+        m_sheet = new SCH_SHEET( &m_schematic );
+        m_schematic.SetRoot( m_sheet );
+        m_sheet->SetScreen( &m_screen );
+
+        m_parent_part = new LIB_SYMBOL( "parent_part", nullptr );
+
+        m_lib_pin = new SCH_PIN( m_parent_part );
+        m_parent_part->AddDrawItem( m_lib_pin );
+
+        // give the pin some kind of data we can use to test
+        m_lib_pin->SetNumber( "42" );
+        m_lib_pin->SetName( "pinname" );
+        m_lib_pin->SetType( ELECTRICAL_PINTYPE::PT_INPUT );
+        m_lib_pin->SetPosition( VECTOR2I( 1, 2 ) );
+
+        SCH_SHEET_PATH path;
+        m_parent_symbol = new SCH_SYMBOL( *m_parent_part, m_parent_part->GetLibId(), &path, 0, 0, VECTOR2I( 1, 2 ) );
+        m_parent_symbol->SetRef( &path, "U2" );
+        m_parent_symbol->UpdatePins();
+
+        m_sch_pin = m_parent_symbol->GetPins( &path )[0];
+
+        m_screen.Append( m_parent_symbol );
+    }
+
+    ~TEST_SCH_GROUP_FIXTURE() {}
+
+    ///< Dummy schematic to attach the test sheet to
+    SCHEMATIC m_schematic;
+
+    SCH_SCREEN m_screen;
+
+    SCH_SHEET* m_sheet;
+
+    LIB_SYMBOL* m_parent_part;
+    SCH_PIN*    m_lib_pin;
+
+    SCH_SYMBOL* m_parent_symbol;
+    SCH_PIN*    m_sch_pin; // owned by m_parent_symbol, not us
+};
+
+/**
+ * Declare the test suite
+ */
+BOOST_FIXTURE_TEST_SUITE( SchGroup, TEST_SCH_GROUP_FIXTURE )
+
+
+/**
+ * Check default properties
+ */
+BOOST_AUTO_TEST_CASE( Default )
+{
+    //BOOST_CHECK_NOT_EQUAL( m_sheet.GetParent(), nullptr );
+    BOOST_CHECK_EQUAL( m_sheet->IsRootSheet(), true );
+    BOOST_CHECK_EQUAL( m_sheet->GetPosition(), VECTOR2I( 0, 0 ) );
+    BOOST_CHECK_EQUAL( m_sheet->CountSheets(), 1 );
+    BOOST_CHECK_EQUAL( m_sheet->SymbolCount(), 1 );
+
+    BOOST_CHECK_EQUAL( m_sheet->GetScreenCount(), 1 );
+}
+
+/**
+ * Check create group (and deletion clearing group parent properly)
+ */
+BOOST_AUTO_TEST_CASE( CreateGroup )
+{
+    SCH_GROUP* group = new SCH_GROUP( &m_screen );
+    group->AddItem( m_parent_symbol );
+
+    m_screen.Append( group );
+}
+
+BOOST_AUTO_TEST_SUITE_END()