#BUGFIX by cg
authorClaus Gittinger <cg@exept.de>
Wed, 13 Feb 2019 12:59:05 +0100
changeset 5998 bb38ed2e191a
parent 5997 dda46d10430a
child 5999 812567d6f683
#BUGFIX by cg class: AbstractHierarchicalItem added: #checkConsistency comment/format in: #canRecursiveCollapse #listIndex #numberOfVisibleChildren #recursiveSetCollapsedHelper changed: #expandLevels:max: #recursiveCollapse #recursiveExpand class: AbstractHierarchicalItem class changed: #version_CVS
AbstractHierarchicalItem.st
--- a/AbstractHierarchicalItem.st	Tue Feb 12 19:02:06 2019 +0100
+++ b/AbstractHierarchicalItem.st	Wed Feb 13 12:59:05 2019 +0100
@@ -358,17 +358,22 @@
 expandLevels:numLevels max:maxNumExpandedHolder
     "expand children numLevels down"
 
+    |childrenToExpand|
+    
     numLevels == 0 ifTrue:[^ self].
     maxNumExpandedHolder value <= 0 ifTrue:[^ self].
 
-    self children do:[:each |
+    childrenToExpand := self children.
+    childrenToExpand do:[:each |
         each expand.
     ].
 
-    maxNumExpandedHolder value:(maxNumExpandedHolder value - self children size).
-    self children do:[:each |
+    maxNumExpandedHolder value:(maxNumExpandedHolder value - childrenToExpand size).
+    childrenToExpand do:[:each |
         each expandLevels:(numLevels - 1) max:maxNumExpandedHolder.
     ].
+
+    "Modified: / 13-02-2019 / 12:33:40 / Claus Gittinger"
 !
 
 expandWithoutUpdate
@@ -405,9 +410,10 @@
 !
 
 recursiveCollapse
-    "collapse all item and sub items
-     **** must be expanded
-    "
+    "collapse item and recursively all sub items"
+    
+    self canCollapse ifFalse:[^ self].
+self checkConsistency.    
     self synchronized:[
         |nrOfVisibleChildren index|
 
@@ -415,44 +421,46 @@
             (index := self listIndex) notNil ifTrue:[
                 "/ do not call self size or self children here;
                 "/ otherwise, lazy children would be autocreated !!!!
-                (nrOfVisibleChildren := children size) ~~ 0 ifTrue:[
-                    children do:[:el|
-                        nrOfVisibleChildren := nrOfVisibleChildren + el numberOfVisibleChildren
-                    ].
-                ].
+                nrOfVisibleChildren := self numberOfVisibleChildren.
                 self recursiveSetCollapsedHelper.
 
                 nrOfVisibleChildren ~~ 0 ifTrue:[
-                    self model itemRemoveFromIndex:(index + 1) toIndex:(index + nrOfVisibleChildren)
+                    self model removeFromIndex:(index + 1) toIndex:(index + nrOfVisibleChildren)
                 ].
                 index ~~ 0 ifTrue:[
                     self hierarchyChanged
                 ]
             ] ifFalse:[
+                "/ not visible
                 self recursiveSetCollapsedHelper
             ]
         ]
-    ]
-
-    "Modified: / 12-02-2019 / 18:54:26 / Claus Gittinger"
+    ].
+self checkConsistency.
+
+    "Modified: / 13-02-2019 / 12:31:46 / Claus Gittinger"
 !
 
 recursiveExpand
-    "expand children and sub-children
+    "expand children and sub-children; 
+     WARNING: refetches children.
      Precondition: must be collapsed"
 
-    |index todo toExpand myList childrenOfToExpand|
-
-    self synchronized:[
+    |index todo toExpand wasExpanded myList prevChildren childrenOfToExpand|
+
+self checkConsistency.    
+    self synchronized:[        
         myList := self model.
         todo := OrderedCollection with:self.
         [todo notEmpty] whileTrue:[
             toExpand := todo removeFirst.
             
             "/ test whether the item already is expanded; #canExpand could be redefined
-            "/ without checking whether the node is expanded (happens actually) !!
-            (toExpand isExpanded or:[toExpand canExpand]) ifTrue:[
+            "/ without checking whether the node is already expanded (happens actually) !!
+            wasExpanded := toExpand isExpanded.
+            (wasExpanded or:[toExpand canExpand]) ifTrue:[
                 toExpand setExpanded:true.
+                prevChildren := toExpand getChildren.
                 childrenOfToExpand := toExpand children.
 
                 index := toExpand listIndex.    "/ get the visible list index
@@ -465,19 +473,36 @@
                     ].    
                 ] ifFalse:[
                     "/ visible
-                    myList itemAddAll:childrenOfToExpand afterIndex:index.
+                    wasExpanded ifTrue:[
+                        prevChildren = childrenOfToExpand ifTrue:[
+                            childrenOfToExpand do:[:eachChild |
+                                (eachChild isExpanded not and:[eachChild canExpand]) ifTrue:[
+                                    todo add:eachChild
+                                ].    
+                            ].
+                        ] ifFalse:[
+                            "/ remove the previous children
+                            prevChildren do:[:each | each recursiveCollapse].
+                           "/ add the new children
+                            myList itemAddAll:childrenOfToExpand afterIndex:index.
+                        ].    
+                    ] ifFalse:[
+                        "/ add the new children
+                        myList itemAddAll:childrenOfToExpand afterIndex:index.
+                    ].    
                     childrenOfToExpand do:[:eachChild |
-                        (eachChild isExpanded or:[eachChild canExpand]) ifTrue:[
-                            todo add:eachChild.
+                        (eachChild isExpanded not and:[eachChild canExpand]) ifTrue:[
+                            todo add:eachChild
                         ].    
-                    ].    
+                    ].
                 ].
             ]
         ]
     ].
+self checkConsistency.
 
     "Modified (comment): / 02-08-2018 / 16:16:17 / Stefan Vogel"
-    "Modified: / 12-02-2019 / 18:39:49 / Claus Gittinger"
+    "Modified: / 13-02-2019 / 12:31:56 / Claus Gittinger"
 !
 
 recursiveToggleExpand
@@ -1394,6 +1419,36 @@
     "Modified: / 12-02-2019 / 19:00:25 / Claus Gittinger"
 !
 
+checkConsistency
+    |allChildrenDo idx expectedIndex|
+
+    allChildrenDo :=
+        [:item :action |
+            action value:item.
+            (item getChildren ? #()) do:[:subItem |
+                allChildrenDo value:subItem value:action
+            ].    
+        ].
+        
+    idx := self listIndex.
+    self isExpanded ifFalse:[
+        (self getChildren ? #()) do:[:eachChild |
+            allChildrenDo value:eachChild value:[:subItem |
+                self assert:(subItem listIndex isNil).
+            ].    
+        ].
+        ^ self.
+    ].
+    
+    expectedIndex := idx+1.
+    self getChildren do:[:eachChild |
+        self assert:(eachChild listIndex == expectedIndex).
+        expectedIndex := expectedIndex + 1 + eachChild numberOfVisibleChildren.
+    ].
+
+    "Created: / 13-02-2019 / 12:31:24 / Claus Gittinger"
+!
+
 clearExpandedWhenLastChildWasRemoved
     "https://expeccoalm.exept.de/D227397 
      Do not set #isExpanded to false just because #children is empty (may children appear 'again' later).
@@ -1416,7 +1471,8 @@
 !
 
 listIndex
-    "returns the visible index or nil; for a non-visible root, 0 is returned"
+    "returns the index in the full list or nil; 
+     for a non-visible root, 0 is returned"
 
     |index model|
 
@@ -1428,13 +1484,16 @@
         ]
     ].
     ^ nil
+
+    "Modified (comment): / 13-02-2019 / 11:34:56 / Claus Gittinger"
 !
 
 numberOfVisibleChildren
     "returns the number of all visible children including subchildren,
-     but excluding myself"
-
-    |count todo toCount|
+     but excluding myself.
+     PseudoRecursive to avoid recursionError for very deep hierarchies."
+
+    |count todo toCount itsChildren|
 
     self isExpanded ifFalse:[^ 0].
 
@@ -1442,8 +1501,10 @@
     todo := OrderedCollection with:self.
     [todo notEmpty] whileTrue:[
         toCount := todo removeFirst.
-        count := count + toCount getChildren size. 
-        toCount getChildren do:[:each |
+        
+        itsChildren := toCount getChildren.
+        count := count + itsChildren size. 
+        itsChildren do:[:each |
             each isExpanded ifTrue:[
                 todo add:each
             ].    
@@ -1451,7 +1512,7 @@
     ].
     ^ count
 
-    "Modified: / 12-02-2019 / 18:42:05 / Claus Gittinger"
+    "Modified (comment): / 13-02-2019 / 12:01:48 / Claus Gittinger"
 !
 
 parentOrModel
@@ -1745,15 +1806,19 @@
     "private helper.
      collapse all children and sub-children without notifications.
      Helper; not synchronized - should only be called internally
-     within a synchronized region."
+     within a synchronized region.
+     PseudoRecursive to avoid recursionError with deep hierarchies."
 
     |todo toCollapse|
+
+    self canRecursiveCollapse ifFalse:[^ self].
     
     todo := OrderedCollection with:self.
     [todo notEmpty] whileTrue:[
         "/ do not call self size or self children;
         "/ otherwise, children might will be autocreated !!!!
         toCollapse := todo removeFirst.
+        
         toCollapse setExpanded:false.
         toCollapse getChildren do:[:eachChild |
             eachChild canRecursiveCollapse ifTrue:[
@@ -1763,7 +1828,7 @@
     ].
 
     "Modified: / 02-08-2018 / 16:07:35 / Stefan Vogel"
-    "Modified: / 12-02-2019 / 17:42:11 / Claus Gittinger"
+    "Modified (format): / 13-02-2019 / 12:15:00 / Claus Gittinger"
 !
 
 recursiveSetExpandedAndAddToList:aList
@@ -2043,9 +2108,12 @@
 
 canRecursiveCollapse
     "called before collapsing the item; can be redefined
-     by subclass to omit the collapse operation "
+     by subclass to omit the collapse operation.
+     Q: why is there an extra canRecursiveCollapse (i.e. not always using canCollapse)?"
 
     ^ self canCollapse
+
+    "Modified (comment): / 13-02-2019 / 12:14:03 / Claus Gittinger"
 !
 
 canRecursiveExpand