The Trunk: Tools-cmm.357.mcz

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

The Trunk: Tools-cmm.357.mcz

commits-2
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!


Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tools-cmm.357.mcz

Frank Shearar-3
On 4 June 2011 23:09,  <[hidden email]> 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.

Thanks,

frank

Reply | Threaded
Open this post in threaded view
|

Re: The Trunk: Tools-cmm.357.mcz

Frank Shearar-3
On 5 June 2011 13:49, Frank Shearar <[hidden email]> wrote:

> On 4 June 2011 23:09,  <[hidden email]> 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