> On 31.05.2018, at 19:32, Chris Muller <asqueaker at gmail.com> wrote:
> 
> Hi Marcel,
> 
> (Note:  I did not see this thread until after I sent the other private email).
> 
> You can't tell what I "changed" because the whole method was re-written.  The formatting I use is referred to as "Rectangular Block" and is a documented Smalltalk best-practice since the 1990's you can read about in Kent Beck's, "Smalltalk Best Practice Patterns".

Rectangular style is one thing, but another thing is having deep control flow nestings.

IMHO, that Method should actually be three  methods.

-t

>  Calling it "bad" is simply your opinion, bringing back the old confusing procedural code did break one case of functionality -- when a Morph is blue-clicked when the Shift modifier is pressed, it should select the innermost morph, *regardless* whether another morph has halos or not.
> 
> Please fix that.  I would recommend simply reverting back to my method, which is 10X easier to read and understand, but its your call.  I care more about the _functionality_ than the code style, but I do think my effort brought a significant improvement to that method.
> 
> Best,
>   Chris
> 
> On Thu, May 31, 2018 at 2:09 AM, Marcel Taeumel <marcel.taeumel at hpi.de> wrote:
> Sorry Chris, but I tend to revert your change in #tryInvokeHalo:, too. Or you do it. Please. :-/
> 
> You changed ALL the formatting, making it close to impossible to make out what you actually changed. 
> 
> Before:
> 
> <image.png>
> 
> After:
> 
> <image.png>
> 
> This is bad. :-(
> 
>  You might have fixed a bug, which one? Maybe I can help here.
> 
> Best,
> Marcel
>> Am 31.05.2018 08:04:48 schrieb H. Hirzel <hannes.hirzel at gmail.com>:
>> 
>> Hi Chris
>> 
>> Are you working on getting the Connectors [1] add-on to work fine
>> with Squeak 5.2a?
>> Recently I loaded Connectors into Squeak6.0a and it loaded fine.
>> However I did not do any tests yet.
>> 
>> --Hannes
>> 
>> [1] Connectors
>> Connectors is a package by Ned Konz that turns Morphic into a drawing
>> environment for making connected drawings.
>> ...
>> This project has been ported to Squeak 4.x by Chris Muller, and the
>> updated code is available at SqueakSource
>> 
>> Current and old versions are available at SqueakMap!
>> 
>> 
>> 
>> On 5/31/18, Marcel Taeumel wrote:
>> > Hey Chris,
>> >
>> > both #layoutChanged and #changed MUST work for all morphs even if the morph
>> > is not in a world or has no owner. Please revert this change and fix
>> > NCAAConnectorMorph instad. Otherwise, this could break test code easily.
>> >
>> > Best,
>> > Marcel
>> > Am 30.05.2018 23:51:51 schrieb commits at source.squeak.org
>> > :
>> > Chris Muller uploaded a new version of Morphic to project The Trunk:
>> > http://source.squeak.org/trunk/Morphic-cmm.1443.mcz
>> >
>> > ==================== Summary ====================
>> >
>> > Name: Morphic-cmm.1443
>> > Author: cmm
>> > Time: 30 May 2018, 4:50:16.716113 pm
>> > UUID: 2f269941-1afa-4615-9242-9e485ecd986e
>> > Ancestors: Morphic-mt.1442
>> >
>> > - When updating a borderStyle:, only signal changes if the receiver is in
>> > the world for them to be seen. This fixes:
>> > NCAAConnectorMorph new
>> > - When halo's are on a background Morph, and a foreground morph covering it
>> > is blue-clicked, transfer the halos to the clicked morph.
>> > - Disable the new #balanceOffsets when Smart Splitters is enabled,
>> > unfortunately they currently don't play nicely together.
>> >
>> > =============== Diff against Morphic-mt.1442 ===============
>> >
>> > Item was changed:
>> > ----- Method: Morph>>borderStyle: (in category 'accessing') -----
>> > borderStyle: aBorderStyle
>> > -
>> > aBorderStyle = self borderStyle ifTrue: [^ self].
>> > -
>> > "If we cannot draw the new border, accept at least its color and width."
>> > ((self canDrawBorder: aBorderStyle) or: [aBorderStyle isNil])
>> > ifTrue: [self setProperty: #borderStyle toValue: aBorderStyle]
>> > ifFalse: [
>> > self borderStyle
>> > width: aBorderStyle width;
>> > baseColor: aBorderStyle baseColor].
>> > -
>> > self borderStyle trackColorFrom: self.
>> > + self isInWorld ifTrue:
>> > + [ self
>> > + layoutChanged;
>> > + changed ]!
>> > -
>> > - self
>> > - layoutChanged;
>> > - changed.!
>> >
>> > Item was changed:
>> > ----- Method: PasteUpMorph>>tryInvokeHalo: (in category 'events-processing')
>> > -----
>> > + tryInvokeHalo: aUserInputEvent
>> > + "Invoke halos around the top-most world container at aUserInputEvent's
>> > #position. If it was already halo'd, zero-in on its next inward component
>> > morph at that position. Holding Shift during the click reverses this
>> > traversal order."
>> > + | stack target owners |
>> > + Preferences noviceMode ifTrue: [^self ].
>> > + Morph haloForAll ifFalse: [^self].
>> > + "the stack is the top-most morph to bottom-most."
>> > + stack := (self morphsAt: aUserInputEvent position unlocked: true) select:
>> > [ : each | each wantsHaloFromClick ].
>> > + stack ifEmpty: [^self].
>> > + target :=
>> > + aUserInputEvent hand halo
>> > + ifNil: [ stack first ]
>> > + ifNotNil:
>> > + [ : existingHalo |
>> > + stack allButFirst "halo's are always topmost on the stack"
>> > + detect: [ : each | each owner == self ]
>> > + ifFound:
>> > + [ : topMostWhereClicked |
>> > + (existingHalo target withAllOwners includes: topMostWhereClicked)
>> > + ifTrue: [ "No invocation needed. Halo will handle transfer itself." ^self
>> > ]
>> > + ifFalse:
>> > + [ "Transfer halo to new world container."
>> > + aUserInputEvent hand removeHalo.
>> > + aUserInputEvent shiftPressed
>> > + ifTrue: [ stack second ]
>> > + ifFalse: [ topMostWhereClicked ] ] ]
>> > + ifNone: ["Okay, invoke halos on the world." self ] ].
>> > + "With a modifier, we want the innermost, otherwise the outermost."
>> > + owners := target withAllOwners select: [ : each | each wantsHaloFromClick
>> > ].
>> > + "the last owner is expected to be self."
>> > + target := aUserInputEvent shiftPressed ifTrue: [ owners first ] ifFalse: [
>> > owners at: owners size-1 ifAbsent: [self] ].
>> > + "Now that we have the target, show the halo. Abort event dispatching, too,
>> > to avoid confusion."
>> > + aUserInputEvent hand newMouseFocus: target event: aUserInputEvent.
>> > + target invokeHaloOrMove: aUserInputEvent.
>> > + "aUserInputEvent has been consumed, don't let it cause any further
>> > side-effects."
>> > + aUserInputEvent ignore!
>> > - tryInvokeHalo: anEvent
>> > -
>> > - | innerMost target |
>> > - anEvent hand halo ifNotNil: [^ self "No invocation needed. Halo will
>> > handle transfer itself."].
>> > - Preferences noviceMode ifTrue: [^ self "No halo in novice mode."].
>> > - Morph haloForAll ifFalse: [^ self].
>> > -
>> > - innerMost := (self morphsAt: anEvent position unlocked: true) first.
>> > -
>> > - "1) Try to use innermost morph but skip all the ones that do not want to
>> > show a halo along the owner chain."
>> > - target := innerMost.
>> > - [target isNil or: [target wantsHaloFromClick]]
>> > - whileFalse: [target := target owner].
>> > - target ifNil: [^ self].
>> > -
>> > - "2) Without a modifier, which is normal, find the outermost container for
>> > that inner morph."
>> > - (innerMost == self or: [anEvent shiftPressed]) ifFalse: [
>> > - | previousTargets |
>> > - previousTargets := OrderedCollection new.
>> > - [target notNil and: [target owner ~~ self]] whileTrue: [
>> > - previousTargets add: target.
>> > - target := target owner].
>> > - target ifNil: [^ self].
>> > - [previousTargets isEmpty or: [target wantsHaloFromClick]] whileFalse: [
>> > - target := previousTargets removeLast].
>> > - target wantsHaloFromClick ifFalse: [^ self]].
>> > -
>> > - "3) Now that we have the target, show the halo. Abort event dispatching,
>> > too, to avoid confusion."
>> > - anEvent hand newMouseFocus: target event: anEvent.
>> > - target invokeHaloOrMove: anEvent.
>> > - anEvent ignore.!
>> >
>> > Item was changed:
>> > ----- Method: ProportionalSplitterMorph>>mouseUp: (in category 'events')
>> > -----
>> > mouseUp: anEvent
>> > + (self bounds containsPoint: anEvent cursorPoint) ifFalse: [ anEvent hand
>> > showTemporaryCursor: nil ].
>> > + self class fastSplitterResize ifTrue: [ self updateFromEvent: anEvent ].
>> > + traceMorph ifNotNil:
>> > + [ traceMorph delete.
>> > + traceMorph := nil ].
>> > - (self bounds containsPoint: anEvent cursorPoint)
>> > - ifFalse: [anEvent hand showTemporaryCursor: nil].
>> > - self class fastSplitterResize
>> > - ifTrue: [self updateFromEvent: anEvent].
>> > - traceMorph ifNotNil: [traceMorph delete. traceMorph := nil].
>> > self color: self getOldColor.
>> > + "balanceOffsets currently disrupts Smart Splitter behavior."
>> > + (ProportionalSplitterMorph smartVerticalSplitters or: [
>> > ProportionalSplitterMorph smartHorizontalSplitters ]) ifFalse: [ self
>> > balanceOffsets ]!
>> > -
>> > - self balanceOffsets!
>> >
>> > Item was changed:
>> > ----- Method: ProportionalSplitterMorph>>step (in category 'events') -----
>> > step
>> > splitsTopAndBottom
>> > ifTrue: [ self reduceTopBottomImbalance ]
>> > ifFalse:
>> > [ self reduceLeftRightImbalance abs > 1 ifTrue:
>> > [ self splittersLeftDo:
>> > [ : splitter | splitter reduceLeftRightImbalance ].
>> > self splittersRightDo:
>> > + [ : splitter | splitter reduceLeftRightImbalance ] ] ]!
>> > - [ : splitter | splitter reduceLeftRightImbalance ] ] ].
>> > - self balanceOffsets!
>> >
>> >
>> >
>> 
> 
> 
> 
> 
>