The Inbox: ToolBuilder-Morphic-dtl.100.mcz

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

The Inbox: ToolBuilder-Morphic-dtl.100.mcz

commits-2
David T. Lewis uploaded a new version of ToolBuilder-Morphic to project The Inbox:
http://source.squeak.org/inbox/ToolBuilder-Morphic-dtl.100.mcz

==================== Summary ====================

Name: ToolBuilder-Morphic-dtl.100
Author: dtl
Time: 20 March 2015, 4:01:04.899 pm
UUID: 10696acd-74dc-4047-898e-77fed9121b6e
Ancestors: ToolBuilder-Morphic-mt.99

Remove toolBuilder instance variable from UIManager.

A UIManager knows what kind of ToolBuilder it should use, so it does not need to hold on to a single instance. Create ToolBuilder instances as required, and do not reuse a ToolBuilder instance after it has done its job. This prevents confusing accumulation of registered widgets in a single ToolBuilder instance

=============== Diff against ToolBuilder-Morphic-mt.99 ===============

Item was removed:
- ----- Method: MorphicUIManager>>initialize (in category 'initialize-release') -----
- initialize
- toolBuilder := MorphicToolBuilder new!

Item was added:
+ ----- Method: MorphicUIManager>>toolBuilder (in category 'builder') -----
+ toolBuilder
+ ^ MorphicToolBuilder new!


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: ToolBuilder-Morphic-dtl.100.mcz

marcel.taeumel (old)
Hmm... I just realized that the instance variable in the UIManager is always nil in my images. Seems that the "new" initialization code was never executed in a migration script.

Please make sure that the logic of #findDefault in ToolBuilder does not get broken. There, you can create subclasses and use those as tool builders. Using this fix, subclassing would not work anymore.

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: ToolBuilder-Morphic-dtl.100.mcz

David T. Lewis
Thanks, I had forgotten about #findDefault. I will take a look at it.

Dave

> Hmm... I just realized that the instance variable in the UIManager is
> always
> nil in my images. Seems that the "new" initialization code was never
> executed in a migration script.
>
> Please make sure that the logic of #findDefault in ToolBuilder does not
> get
> broken. There, you can create subclasses and use those as tool builders.
> Using this fix, subclassing would not work anymore.
>
> Best,
> Marcel
>
>
>
> --
> View this message in context:
> http://forum.world.st/The-Inbox-ToolBuilder-Morphic-dtl-100-mcz-tp4813610p4813645.html
> Sent from the Squeak - Dev mailing list archive at Nabble.com.
>



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: ToolBuilder-Morphic-dtl.100.mcz

David T. Lewis
In reply to this post by marcel.taeumel (old)
On Fri, Mar 20, 2015 at 10:07:45AM -0700, Marcel Taeumel wrote:
> Hmm... I just realized that the instance variable in the UIManager is always
> nil in my images. Seems that the "new" initialization code was never
> executed in a migration script.
>
> Please make sure that the logic of #findDefault in ToolBuilder does not get
> broken. There, you can create subclasses and use those as tool builders.
> Using this fix, subclassing would not work anymore.
>

I put another update in the inbox. The #findDefault logic is now used to
find the ToolBuilder class to use, and this normally happens the first time
that a ToolBuiilder is needed in a new Project. The mechanism should now
work as before, but the UIManager no longer needs to have a dedicated
ToolBuilder instance.

Dave
 

Reply | Threaded
Open this post in threaded view
|

Instantiating a ToolBuilder for a UIManager (was: Re: The Inbox: ToolBuilder-Morphic-dtl.100.mcz)

David T. Lewis
On Sat, Mar 21, 2015 at 12:21:09PM -0400, David T. Lewis wrote:

> On Fri, Mar 20, 2015 at 10:07:45AM -0700, Marcel Taeumel wrote:
> > Hmm... I just realized that the instance variable in the UIManager is always
> > nil in my images. Seems that the "new" initialization code was never
> > executed in a migration script.
> >
> > Please make sure that the logic of #findDefault in ToolBuilder does not get
> > broken. There, you can create subclasses and use those as tool builders.
> > Using this fix, subclassing would not work anymore.
> >
>
> I put another update in the inbox. The #findDefault logic is now used to
> find the ToolBuilder class to use, and this normally happens the first time
> that a ToolBuiilder is needed in a new Project. The mechanism should now
> work as before, but the UIManager no longer needs to have a dedicated
> ToolBuilder instance.
>

I should note for the record that I am the person who added the #toolBuilder
instance variable a few years ago. At the time, I did not notice the side
effect of garbage accumulating in the tool builder's registry. Hopefully
the changes in the inbox will make it right.

For background, here is where that original set of changes were made:

  Name: ToolBuilder-Kernel-dtl.46
  Author: dtl
  Time: 5 March 2011, 1:05:35.352 pm
 
  A Project has a UIManager, and a UIManager has a ToolBuilder, so add
  #toolBuilder ivar to UIManager and initialize accordingly. This
  facilitates setting up the appropriate UIManager and ToolBuilder to
  allow SMxMorphicProject to host a SimpleMorphic world.
 
  Change Toolbuilder class>>default to always ask the default UI
  manager for its tool builder. Remove class var Default (this was
  provided in the ToolBuilder package but never used in Squeak).
  Deprecate ToolBuilder class>>default:
 
  Background: In previous Squeak usage, ToolBuilder class>>default always
  invoked a search for the appropriate ToolBuilder subclass, and class var
  Default was unused (this is awkward if more than one kind of ToolBuilder
  could be used in a project that #isMorphic). This change makes the default
  tool builder an explicit attibute of the active UI manager.

Dave
 

Reply | Threaded
Open this post in threaded view
|

Re: Instantiating a ToolBuilder for a UIManager (was: Re: The Inbox: ToolBuilder-Morphic-dtl.100.mcz)

Eliot Miranda-2
David,

   thanks so much for addressing this arcana so promptly!  

On Sat, Mar 21, 2015 at 12:04 PM, David T. Lewis <[hidden email]> wrote:
On Sat, Mar 21, 2015 at 12:21:09PM -0400, David T. Lewis wrote:
> On Fri, Mar 20, 2015 at 10:07:45AM -0700, Marcel Taeumel wrote:
> > Hmm... I just realized that the instance variable in the UIManager is always
> > nil in my images. Seems that the "new" initialization code was never
> > executed in a migration script.
> >
> > Please make sure that the logic of #findDefault in ToolBuilder does not get
> > broken. There, you can create subclasses and use those as tool builders.
> > Using this fix, subclassing would not work anymore.
> >
>
> I put another update in the inbox. The #findDefault logic is now used to
> find the ToolBuilder class to use, and this normally happens the first time
> that a ToolBuiilder is needed in a new Project. The mechanism should now
> work as before, but the UIManager no longer needs to have a dedicated
> ToolBuilder instance.
>

I should note for the record that I am the person who added the #toolBuilder
instance variable a few years ago. At the time, I did not notice the side
effect of garbage accumulating in the tool builder's registry. Hopefully
the changes in the inbox will make it right.

For background, here is where that original set of changes were made:

  Name: ToolBuilder-Kernel-dtl.46
  Author: dtl
  Time: 5 March 2011, 1:05:35.352 pm

  A Project has a UIManager, and a UIManager has a ToolBuilder, so add
  #toolBuilder ivar to UIManager and initialize accordingly. This
  facilitates setting up the appropriate UIManager and ToolBuilder to
  allow SMxMorphicProject to host a SimpleMorphic world.

  Change Toolbuilder class>>default to always ask the default UI
  manager for its tool builder. Remove class var Default (this was
  provided in the ToolBuilder package but never used in Squeak).
  Deprecate ToolBuilder class>>default:

  Background: In previous Squeak usage, ToolBuilder class>>default always
  invoked a search for the appropriate ToolBuilder subclass, and class var
  Default was unused (this is awkward if more than one kind of ToolBuilder
  could be used in a project that #isMorphic). This change makes the default
  tool builder an explicit attibute of the active UI manager.

Dave





--
best,
Eliot


Reply | Threaded
Open this post in threaded view
|

Re: Instantiating a ToolBuilder for a UIManager (was: Re: The Inbox: ToolBuilder-Morphic-dtl.100.mcz)

marcel.taeumel (old)
In reply to this post by David T. Lewis
Almost there. :) There is no setter #builderClass:. Could you add one? I don't see the need for creating a new project whenever a new builder is introduced to the system. It should be possible to reuse the current project.

Best,
Marcel
Reply | Threaded
Open this post in threaded view
|

Re: Instantiating a ToolBuilder for a UIManager (was: Re: The Inbox: ToolBuilder-Morphic-dtl.100.mcz)

David T. Lewis
On Sun, Mar 22, 2015 at 10:38:13PM -0700, Marcel Taeumel wrote:
> Almost there. :) There is no setter #builderClass:. Could you add one? I
> don't see the need for creating a new project whenever a new builder is
> introduced to the system. It should be possible to reuse the current
> project.

Done.

Thanks for the reviews :-)

Dave