From 38534bd33da716347cf39e8fda831567448a4849 Mon Sep 17 00:00:00 2001
From: Roy-043 <70520633+Roy-043@users.noreply.github.com>
Date: Wed, 29 Dec 2021 15:30:22 +0100
Subject: [PATCH] Draft: improve get_group_contents function in groups.py

For consistency the get_group_contents function should use is_group to identify groups. The code dedicated to Site objects and Drawing pages was removed.
---
 src/Mod/Draft/draftutils/groups.py | 56 +++++++++++-------------------
 1 file changed, 21 insertions(+), 35 deletions(-)

diff --git a/src/Mod/Draft/draftutils/groups.py b/src/Mod/Draft/draftutils/groups.py
index e58c557721..a1e5f15789 100644
--- a/src/Mod/Draft/draftutils/groups.py
+++ b/src/Mod/Draft/draftutils/groups.py
@@ -37,7 +37,7 @@ the objects created with this workbench work like groups.
 import FreeCAD as App
 import draftutils.utils as utils
 
-from draftutils.translate import _tr
+from draftutils.translate import translate
 from draftutils.messages import _msg, _err
 
 
@@ -58,15 +58,16 @@ def is_group(obj):
         a `'LayerContainer'`.
 
         Or the object is of the type `'Project'`, `'Site'`, `'Building'`,
-        `'Floor'` or `'BuildingPart'` from the Arch Workbench.
+        `'Floor'`, `'BuildingPart'` or `'Space'` from the Arch Workbench.
+        Note that `'Floor'` and `'Building'` are obsolete types.
 
         Otherwise returns `False`.
     """
-
+    typ = utils.get_type(obj)
     return ((obj.isDerivedFrom("App::DocumentObjectGroup")
-                and obj.Name != "LayerContainer")
-            or utils.get_type(obj) in ("Project", "Site", "Building",
-                                       "Floor", "BuildingPart"))
+                and typ != "LayerContainer")
+            or typ in ("Project", "Site", "Building",
+                       "Floor", "BuildingPart", "Space"))
 
 
 def get_group_names(doc=None):
@@ -91,7 +92,7 @@ def get_group_names(doc=None):
         found, doc = utils.find_doc(App.activeDocument())
 
     if not found:
-        _err(_tr("No active document. Aborting."))
+        _err(translate("draft", "No active document. Aborting."))
         return []
 
     glist = []
@@ -128,7 +129,7 @@ def ungroup(obj):
     found, obj = utils.find_object(obj, doc=App.activeDocument())
     if not found:
         _msg("obj: {}".format(obj_str))
-        _err(_tr("Wrong input: object not in document."))
+        _err(translate("draft", "Wrong input: object not in document."))
         return None
 
     doc = obj.Document
@@ -200,12 +201,9 @@ def get_group_contents(objectslist,
     Parameters
     ----------
     objectslist: list
-        If any object in the list is a group, its contents (`obj.Group`)
-        are extracted and added to the output list.
-
-        The "groups" are objects derived from `App::DocumentObjectGroup`,
-        but they can also be `'App::Part'`, or `'Building'`, `'BuildingPart'`,
-        `'Space'`, and `'Site'` from the Arch Workbench.
+        If any object in the list is considered a group, see the `is_group`
+        function, its contents (`obj.Group`) are extracted and added to the
+        output list.
 
         Single items that aren't groups are added to the output list
         as is.
@@ -242,28 +240,16 @@ def get_group_contents(objectslist,
 
     for obj in objectslist:
         if obj:
-            if (obj.isDerivedFrom("App::DocumentObjectGroup")
-                    or (utils.get_type(obj) in ("Building", "BuildingPart",
-                                                "Space", "Site")
-                        and hasattr(obj, "Group"))):
-                if utils.get_type(obj) == "Site":
-                    if obj.Shape:
-                        newlist.append(obj)
-                if obj.isDerivedFrom("Drawing::FeaturePage"):
-                    # Skip if the group is a Drawing page
-                    # Note: the Drawing workbench is obsolete
+            if is_group(obj):
+                if addgroups or (spaces
+                                 and utils.get_type(obj) == "Space"):
                     newlist.append(obj)
-                else:
-                    if addgroups or (spaces
-                                     and utils.get_type(obj) == "Space"):
-                        newlist.append(obj)
-                    if (noarchchild
-                            and utils.get_type(obj) in ("Building",
-                                                        "BuildingPart")):
-                        pass
-                    else:
-                        newlist.extend(get_group_contents(obj.Group,
-                                                          walls, addgroups))
+                if not (noarchchild
+                        and utils.get_type(obj) in ("Building",
+                                                    "BuildingPart")):
+                    newlist.extend(get_group_contents(obj.Group,
+                                                      walls, addgroups,
+                                                      spaces, noarchchild))
             else:
                 # print("adding ", obj.Name)
                 newlist.append(obj)