Ticket #108: libtool_fix_1_of_3_rev_c95ae929a0b1_Issue__108__Fixed_bug_causing_method_selection_loss_in_some_cases.patch

Tools__MethodList.st

@@ -369 +369 @@
 
 delayedUpdate:something with:aParameter from:changedObject
     |cls clsName sel oldMethod newMethod methods newSelection
-     selectionHolder selection needSelectionChange|
+     selection needSelectionChange|
 
-    selectionHolder := self selectedMethods.
-    selection := selectionHolder value.
+    selection := self selectionHolder value.
 
     changedObject == environment ifTrue:[
         classes notNil ifTrue:[
@@ -415 +414 @@
                          however, ensure that the refs to the old method are updated
                         "
                         methods := selection.
-                        methods size > 0 ifTrue:[
+                        selection notEmpty ifTrue:[
                             (methods includesIdentical:oldMethod) ifTrue:[
                                 needSelectionChange := true.
                             ]
@@ -424 +423 @@
                         lastSelectedMethods notNil ifTrue:[
                             lastSelectedMethods replaceAllIdentical:oldMethod with:newMethod
                         ].
-                        methods size > 0 ifTrue:[
+                        selection notEmpty ifTrue:[
                             methods := methods asOrderedCollection.
                             methods replaceAllIdentical:oldMethod with:newMethod.
                         ].
@@ -498 +497 @@
 
                 (something == #methodTrap) ifTrue:[
                     newMethod isWrapped ifTrue:[
-                        oldMethod := newMethod originalMethod
+                        oldMethod := newMethod originalMethod.
                     ] ifFalse:[
-                        selection size > 0 ifTrue:[
+                        selection notEmpty ifTrue:[
                             oldMethod := selection detect:[:each | each isWrapped and:[each originalMethod == newMethod]] ifNone:nil.
                         ]
                     ].
 
-                    selection size > 0 ifTrue:[
+                    selection notEmpty ifTrue:[
                         (selection includesIdentical:oldMethod) ifTrue:[
                             needSelectionChange := true.
                         ]
@@ -514 +513 @@
                     lastSelectedMethods notNil ifTrue:[
                         lastSelectedMethods replaceAllIdentical:oldMethod with:newMethod
                     ].
-                    selection size > 0 ifTrue:[
-                        selection := selection asOrderedCollection.
-                        selection replaceAllIdentical:oldMethod with:newMethod.
+                    selection notEmpty ifTrue:[
+                        "/ Used to be:
+                        "/ 
+                        "/     selection := selection asOrderedCollection,
+                        "/     selection replaceAllIdentical:oldMethod with:newMethod.
+                        "/ 
+                        "/ but that's WRONG!! If the selection is not an ordered collection,
+                        "/ then asOrderedCollection would create new collection noone else
+                        "/ references and thus the replace is useless. If the selection 
+                        "/ is already an OrderedCollection, then #asOrderedCollection is
+                        "/ itself useless.
+                        "/ 
+                        "/ The former case (when selection was for example an array) caused
+                        "/ selection loss on some cases, hard to chase down!!
+                        selection isSequenceable ifTrue:[ 
+                            selection replaceAllIdentical:oldMethod with:newMethod.
+                        ] ifFalse:[ 
+                            "/ Now, are we sure selection is always a sequenceable? 
+                            "/ I (JV) am not sure so handle (Identity)Set cases too:
+                            self breakPoint: #jv.  
+                            (selection includesIdentical: oldMethod) ifTrue:[ 
+                                selection removeIdentical: oldMethod.
+                                selection add: newMethod.
+                            ].
+                        ].
                     ].
                     needSelectionChange == true ifTrue:[
                         selectionHolder changed.
@@ -709 +730 @@
 
     "Created: / 05-02-2000 / 13:42:14 / cg"
     "Modified: / 05-06-2012 / 23:47:15 / cg"
-    "Modified: / 24-08-2013 / 00:41:21 / Jan Vrany <jan.vrany@fit.cvut.cz>"
+    "Modified: / 15-10-2017 / 23:49:53 / Jan Vrany <jan.vrany@fit.cvut.cz>"
 !
 
 selectedMethodsChanged