On 5 June 2011 13:49, Frank Shearar <frank.shearar at gmail.com> wrote:
> On 4 June 2011 23:09,  <commits at source.squeak.org> wrote:
>> Chris Muller uploaded a new version of Tools to project The Trunk:
>> http://source.squeak.org/trunk/Tools-cmm.357.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Tools-cmm.357
>> Author: cmm
>> Time: 4 June 2011, 5:08:48.648 pm
>> UUID: 08000000-1508-5e1f-1508-5e1f14000000
>> Ancestors: Tools-cmm.355, Tools-ul.356
>>
>> - Fixed problem where MessageSets were not keeping the autoSelectString selected when 'remove from this browser' was invoked.
>> - Merged Tools-cmm.355.
>>
>> =============== Diff against Tools-ul.356 ===============
>>
>> Item was changed:
>>  ----- Method: HierarchyBrowser>>initHierarchyForClass: (in category 'initialization') -----
>>  initHierarchyForClass: aClassOrMetaClass
>>        | nonMetaClass superclasses |
>>        centralClass := aClassOrMetaClass.
>>        nonMetaClass := aClassOrMetaClass theNonMetaClass.
>>        self systemOrganizer: SystemOrganization.
>>        metaClassIndicated := aClassOrMetaClass isMeta.
>>        classDisplayList := OrderedCollection new.
>>        (superclasses := nonMetaClass allSuperclasses reversed) withIndexDo:
>>                [ : each : indent | classDisplayList add:
>>                        (String streamContents:
>>                                [ : stream | indent - 1 timesRepeat: [ stream nextPutAll: '  ' ].
>>                                stream nextPutAll: each name ]) ].
>>        nonMetaClass
>>                allSubclassesWithLevelDo:
>>                        [ : eachClass : lvl | classDisplayList add:
>>                                (String streamContents:
>> +                                       [ : stream | lvl timesRepeat: [ stream nextPutAll: '  ' ].
>> -                                       [ : stream | lvl+1 timesRepeat: [ stream nextPutAll: '  ' ].
>>                                        stream nextPutAll: eachClass name ]) ]
>>                startingLevel: superclasses size.
>>        self selectClass: nonMetaClass!
>>
>> Item was changed:
>>  ----- Method: MessageSet>>reformulateList (in category 'message functions') -----
>>  reformulateList
>>        "The receiver's messageList has been changed; rebuild it"
>>        super reformulateList.
>> +       self
>> +                changed: #messageList ;
>> +                changed: #messageListIndex.
>> +       self contentsChanged.
>> +       self changed: #autoSelect!
>> -       self changed: #messageList.
>> -       self changed: #messageListIndex.
>> -       self contentsChanged!
>
> I'm not so worried about the double-indent, but could we also see a
> test for the #reformulateList please? There is a test for
> autoSelectString working, in a slightly different context, in
> MessageSetTest>>testAutoSelectString.
>
> As it happens, #selectMessageNamed: calls #autoSelect under certain conditions:
>
>        (selectedMessageName notNil and: [ autoSelectString notNil and: [
> self contents notEmpty ] ]) ifTrue: [ self changed: #autoSelect ].
>
> So I had a go at writing a test demonstrating the fix your code
> provides, but it's not quite right:
>
> testRemoveMessageFromBrowserPreservesAutoSelectString
>        browser autoSelectString: 'browseAllImplementorsOf:'.
>        self assert: browser autoSelectString = 'browseAllImplementorsOf:'.
>        browser messageListIndex: 2.
>        updates removeAll.
>        browser removeMessageFromBrowser.
>        self assert: (updates includes: #autoSelect).
>
> The test's wrong, but I'm not sure how. Wrong, because it passes
> before the above fix is applied.
>
> Would you mind being more specific about the conditions demonstrating
> the bug? Hopefully it's just a matter of tweaking the above test.

Ah. As it happens, the change breaks a test -
MessageSetTest>>testRemoveMessageBrowser breaks with because something
passes a nil SmalltalkEditor(TextEditor)>>setSearch:.

frank