Totally agree.  It's a bug in the formatter I wasn't able to fix when
I tried once a while back.

On Tue, Feb 18, 2014 at 4:43 PM, Frank Shearar <frank.shearar at gmail.com> wrote:
> On 17 February 2014 22:35,  <commits at source.squeak.org> wrote:
>> Chris Muller uploaded a new version of Tests to project The Trunk:
>> http://source.squeak.org/trunk/Tests-cmm.290.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Tests-cmm.290
>> Author: cmm
>> Time: 17 February 2014, 4:35:24.565 pm
>> UUID: f3fccfae-6baf-4093-ba62-e15ef110a687
>> Ancestors: Tests-cwp.289
>>
>> Minor factoring.
>>
>> =============== Diff against Tests-cwp.289 ===============
>>
>> Item was changed:
>>   ----- Method: BlockLocalTemporariesRemovalTest>>assert:isChangedDuringParsingTo:withRemovalOfTemporariesNamed: (in category 'test helper') -----
>> + assert: someCode isChangedDuringParsingTo: someOtherCode withRemovalOfTemporariesNamed: someTempNames
>> - assert: someCode isChangedDuringParsingTo: someOtherCode withRemovalOfTemporariesNamed: someTempNames
>> -
>>         | failBlock |
>>         self sourceCode: someCode.
>> +       failBlock := [ self fail ].
>> +       [ self class
>> +               compile: self sourceCode
>> +               notifying: self
>> +               trailer: self class defaultMethodTrailer
>> +               ifFail: failBlock ]
>> -       failBlock := [self fail].
>> -       [self class
>> -                       compile: self sourceCode
>> -                       notifying: self
>> -                       trailer: self class defaultMethodTrailer
>> -                       ifFail: failBlock]
>>                 on: UnusedVariable
>> +               do:
>> +                       [ : aNotification | aNotification openMenuIn:
>> +                               [ : options : emptyCollection : someText | aNotification resume:
>> +                                       (someTempNames anySatisfy:
>> +                                               [ : tempName | someText beginsWith: tempName ]) ] ].
>
> I normally hold my tongue (ok, it's me, I _bite_ my tongue), but in
> this case I find it really hard to read this block when there's a
> message send in the same line as the temp declaration.
>
> I'd expect to see this:
>
>     [:options :emptyCollection :someText |
>         aNotification resume:
>
> Other than that, the change looks great.
>
> (Chris replaced the boolExpr ifTrue: [aNotification resume: true]
> ifFalse: [aNotification resume: false] with aNotification resume:
> boolExpr)
>
> frank
>
>> -               do: [:aNotification | aNotification
>> -                               openMenuIn: [:options :emptyCollection :someText |
>> -                                       (someTempNames anySatisfy: [:tempName | someText startsWith: tempName])
>> -                                               ifTrue: [aNotification resume: true]
>> -                                               ifFalse: [aNotification resume: false]]].
>>         self assert: self sourceCode = someOtherCode!
>>
>>
>