Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-23 Thread Benjamin
On 23 Jan 2014, at 12:53, b...@openinworld.com wrote:

> I gave this a try and indeed #whenBuiltDo: works from #initialize.  However 
> in PharoLauncher  stopped updating the list.  After a bit of a 
> muddling around I found that I am prevented from registering the event 
> "wherever I wanted", since the subscription does not go to theValueHolder but 
> to the contents of theValueHolder.

That sounds strange :(

> That is as far as I can push this for now.  I have a working solution with 
> previously proposed ComposableModel>>topWindowHolder and 
> ComposableModel>>whenWindowClosed: so I will proceed with that.  Any 
> objections to pushing these into Pharo 3 at this late stage :) and I'll just 
> make them extensions from PharoLauncher.

If you submit a slice, I will review it :)
Note that you can also do it on top of the bleeding edge Spec[1]

Ben

[1]https://github.com/SpecForPharo/spec

> 
> cheers -ben
> 
>> 
>>   
>>> I tried to avoid calling in #windowHolder but would that preferred? 
>>> Do you have another suggestion?  
>>> 
>> 
>> I was not talking about the name, more the implementation :P
>> (but not more than #window  :) )
>> 
>> 
>>   
>>> btw, the 'window' i-var of ComposableModel seems like it should be 
>>> 'windowHolder' 
>>> 
>> 
>> I think all the other variable should be name without holder at the end :)
>> 
>> Ben
>> 
>>   
> 




Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-23 Thread btc




Benjamin wrote:

  On 20 Jan 2014, at 14:37, b...@openinworld.com wrote:

  
  
I don't want to unsubscribe #refresh in PhLTitledTreeModel>>initialize.
I _really_ want to unsubscribe #refresh immediately after the subscribe #refresh is set up.

  
  
But it does not prevent you to register to the event whenever you want :)
Could you give a try and tell me?
  

I gave this a try and indeed #whenBuiltDo: works from #initialize. 
However in PharoLauncher  stopped updating the
list.  After a bit of a muddling around I found that I am prevented
from registering the event "wherever I wanted", since the subscription
does not go to theValueHolder but to the contents of theValueHolder.  
So the event really can only be registered when the ValueHolder content
is set in the #repository: method. 

That is as far as I can push this for now.  I have a working solution
with previously proposed ComposableModel>>topWindowHolder and
ComposableModel>>whenWindowClosed: so I will proceed with that. 
Any objections to pushing these into Pharo 3 at this late stage :) and
I'll just make them extensions from PharoLauncher.

cheers -ben


  

  
  
I tried to avoid calling in #windowHolder but would that preferred? 
Do you have another suggestion?  

  
  
I was not talking about the name, more the implementation :P
(but not more than #window  :) )


  
  
btw, the 'window' i-var of ComposableModel seems like it should be 'windowHolder' 

  
  
I think all the other variable should be name without holder at the end :)

Ben

  








Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-20 Thread Benjamin
On 20 Jan 2014, at 14:37, b...@openinworld.com wrote:

> I don't want to unsubscribe #refresh in PhLTitledTreeModel>>initialize.
> I _really_ want to unsubscribe #refresh immediately after the subscribe 
> #refresh is set up.

But it does not prevent you to register to the event whenever you want :)
Could you give a try and tell me?


> I tried to avoid calling in #windowHolder but would that preferred? 
> Do you have another suggestion?  

I was not talking about the name, more the implementation :P
(but not more than #window  :) )


> btw, the 'window' i-var of ComposableModel seems like it should be 
> 'windowHolder' 

I think all the other variable should be name without holder at the end :)

Ben


Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-20 Thread btc




Benjamin wrote:

  
On 20 Jan 2014, at 13:51, b...@openinworld.com wrote:
  
  
  
When I try...
    self whenBuiltDo: [ self halt. self window whenClosedDo: [ self
halt. self repository unsubscribe: self ] ].
none of the halts occur.

However I got what I needed with the following…
  
  
  
  did you try to register in the initialize method ?
  Because if you register once the application is already open, it
may not work :)
  
  

I don't want to unsubscribe #refresh in
PhLTitledTreeModel>>initialize.
I _really_ want to unsubscribe #refresh immediately after the subscribe
#refresh is set up.

  
  
  
--
   ComposableModel>>topWindowHolder
     ^ owner 
        ifNil: [ window ]
        ifNotNil: [ :o | o topWindowHolder ].
--
  ComposableModel>>whenWindowClosed: aBlock
        self topWindowHolder value
        ifNotNil: [ :w | w whenClosedDo: aBlock ]
        ifNil: [  self topWindowHolder whenChangedDo: [  :w  | w
whenClosedDo: aBlock ] ].
--
    PhLTitledTreeModel>>repository: aRepository
       self assert: self repository isNil description: 'Changing the
repository is not allowed because we would have to change the context
as well'.
        repositoryHolder value: aRepository.
        self repository whenChangedSend: #refresh to: self.
        self whenWindowClosed: [ self repository unsubscribe: self.  
self inform: 'Works for composed sub-items'. ] .
        self refresh
--
    PharoLauncher>>initialize
    super initialize.
    self whenWindowClosed: [ self inform: 'Works for top level
owner' ].
--
And optionally you could now have...
    ComposableModel>>window
        ^ self topWindowHolder value
—
  
  
  
  
  topWindowHolder sounds a bit
hackish :)
  
  
  

I tried to avoid calling in #windowHolder but would that preferred? 
Do you have another suggestion?  

btw, the 'window' i-var of ComposableModel seems like it should be
'windowHolder' 


  
  
  
  I think this can be achieved
with the whenBuiltDo:, I should investigate :)
  
  
  


  
  
  

If those ComposableModel additions are okay, I'll submit a slice on
Case 12677 (renamed to "ComposableModel subcomponents need to act on
window close")
https://pharo.fogbugz.com/f/cases/12677/

  
  
  
  You can also submitted here: https://github.com/SpecForPharo/spec if
you want :)
  
  
  (and become a Spec contributor :P)
  
  
  Ben








Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-20 Thread Benjamin
On 20 Jan 2014, at 13:51, b...@openinworld.com wrote:

> Sure thing. That was part of my original intention.

Cool :) Thanks

> When I try...
> self whenBuiltDo: [ self halt. self window whenClosedDo: [ self halt. 
> self repository unsubscribe: self ] ].
> none of the halts occur.
> 
> However I got what I needed with the following…

did you try to register in the initialize method ?
Because if you register once the application is already open, it may not work :)

> --
>ComposableModel>>topWindowHolder
>  ^ owner 
> ifNil: [ window ]
> ifNotNil: [ :o | o topWindowHolder ].
> --
>   ComposableModel>>whenWindowClosed: aBlock
> self topWindowHolder value
> ifNotNil: [ :w | w whenClosedDo: aBlock ]
> ifNil: [  self topWindowHolder whenChangedDo: [  :w  | w 
> whenClosedDo: aBlock ] ].
> --
> PhLTitledTreeModel>>repository: aRepository
>self assert: self repository isNil description: 'Changing the 
> repository is not allowed because we would have to change the context as 
> well'.
> repositoryHolder value: aRepository.
> self repository whenChangedSend: #refresh to: self.
> self whenWindowClosed: [ self repository unsubscribe: self.   self 
> inform: 'Works for composed sub-items'. ] .
> self refresh
> --
> PharoLauncher>>initialize
> super initialize.
> self whenWindowClosed: [ self inform: 'Works for top level owner' ].
> --
> And optionally you could now have...
> ComposableModel>>window
> ^ self topWindowHolder value
> —

topWindowHolder sounds a bit hackish :)
I think this can be achieved with the whenBuiltDo:, I should investigate :)

> 
> If those ComposableModel additions are okay, I'll submit a slice on Case 
> 12677 (renamed to "ComposableModel subcomponents need to act on window close")
> https://pharo.fogbugz.com/f/cases/12677/

You can also submitted here: https://github.com/SpecForPharo/spec if you want :)
(and become a Spec contributor :P)

Ben

Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-20 Thread btc




Benjamin wrote:

  
Hi Ben,

I've gave that a go and had some success, but it raised an architectural question I ask near the end.  I've documented my investigation here in case it is of use to others new to Spec or the Spec documentation being developed. 

  
  
Spec documentation is actually under heavy construction :)
May I keep this content for further quotation?
  

Sure thing. That was part of my original intention.

  

  
  
So the question this leads up to is... 
Should child ComposableModels all reference the same WindowModel as their owner?

  
  
The thing is, the isn’t var window doesn’t point to the same, but the method `window` does :)
Could you try something like:

 PhLTitledTreeModel>>repository: aRepository
repositoryHolder value: aRepository.
self repository whenChangedSend: #refresh to: self.  "<--subscription-of-concern"
self refresh.
	self whenBuiltDo: [ self window whenClosed: [ self repository unsubscribe: self ] ]


Thanks for your investigation :)
Ben

  

When I try...
    self whenBuiltDo: [ self halt. self window whenClosedDo: [ self
halt. self repository unsubscribe: self ] ].
none of the halts occur.

However I got what I needed with the following...
--
   ComposableModel>>topWindowHolder
     ^ owner 
        ifNil: [ window ]
        ifNotNil: [ :o | o topWindowHolder ].
--
  ComposableModel>>whenWindowClosed: aBlock
        self topWindowHolder value
        ifNotNil: [ :w | w whenClosedDo: aBlock ]
        ifNil: [  self topWindowHolder whenChangedDo: [  :w  | w
whenClosedDo: aBlock ] ].
--
    PhLTitledTreeModel>>repository: aRepository
       self assert: self repository isNil description: 'Changing the
repository is not allowed because we would have to change the context
as well'.
        repositoryHolder value: aRepository.
        self repository whenChangedSend: #refresh to: self.
        self whenWindowClosed: [ self repository unsubscribe: self.  
self inform: 'Works for composed sub-items'. ] .
        self refresh
--
    PharoLauncher>>initialize
    super initialize.
    self whenWindowClosed: [ self inform: 'Works for top level
owner' ].
--
And optionally you could now have...
    ComposableModel>>window
        ^ self topWindowHolder value
--

If those ComposableModel additions are okay, I'll submit a slice on
Case 12677 (renamed to "ComposableModel subcomponents need to act on
window close")
https://pharo.fogbugz.com/f/cases/12677/

cheers -ben






Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-19 Thread Benjamin
> Hi Ben,
> 
> I've gave that a go and had some success, but it raised an architectural 
> question I ask near the end.  I've documented my investigation here in case 
> it is of use to others new to Spec or the Spec documentation being developed. 

Spec documentation is actually under heavy construction :)
May I keep this content for further quotation?


> So the question this leads up to is... 
> Should child ComposableModels all reference the same WindowModel as their 
> owner?

The thing is, the isn’t var window doesn’t point to the same, but the method 
`window` does :)
Could you try something like:

 PhLTitledTreeModel>>repository: aRepository
repositoryHolder value: aRepository.
self repository whenChangedSend: #refresh to: self.  
"<--subscription-of-concern"
self refresh.
self whenBuiltDo: [ self window whenClosed: [ self repository 
unsubscribe: self ] ]


Thanks for your investigation :)
Ben


Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-19 Thread btc




On 18 Jan 2014, at 12:09, b...@openinworld.com wrote:

  
  PharoLauncher is currently leaking
AnnouncementSubscriptions when its window closes.  I'm not sure with
Spec how to ensure actions are performed when the window is closed, but
I have been able to achieve this with the following modification [1]
which is...
-
  WindowModel>>windowIsClosing
  isClosedHolder value: true
  self model windowIsClosing  ">close to trace through
and discover that it calls...
SpecWindow(StandardWindow)>>delete, which in my
non-fullscreen-case calls...
SpecWindow(SystemWindow)>>delete, which "model windowIsClosing"
calls...
MorphicWindowAdaptor>>windowIsClosing, which "self model
windowIsClosing" calls...
WindowModel>>windowIsClosing, which does only "isClosedHolder
value: true"

That last line happens to hold aPharoLauncher in its 'model' instance
variable, so with the above proposal I can implement
PharoLauncher>>windowIsClosing to unregister the announcer.
Any problem expected with that modification?

[1] https://pharo.fogbugz.com/f/cases/12677

cheers -ben

  
  

Benjamin wrote:

  
I think i would be more elegant that the model register to the
isClosedHolder changes :)
  
  Ben


Hi Ben,

I've gave that a go and had some success, but it raised an
architectural question I ask near the end.  I've documented my
investigation here in case it is of use to others new to Spec or the
Spec documentation being developed. 

So references to i-var 'isClosedHolder' indicate this operates through
#whenClosedDo: 
which has three senders:
* InspectorNavigator>>initialize 
* SpecDebugger>>initialize 
* SpecPreDebugWindow>>initialize 
All of which use the pattern...
    self whenWindowChanged: [ :w | w whenClosedDo: [ "do stuff" ] ].

I understand this I inserted the following breakpoints...
InspectorNavigator>>initialize
    ...
    self haltOnce. self whenWindowChanged:  [ :w | "outerBlock" self
halt. w whenClosedDo: 
    [ "innerBlock" self halt. self inspector close  ] ]. 

After enabling haltOnce and doing "1 inspect"... 

Trace Part 1. 
The first breakpoint (haltOnce) occurs in
InspectorNavigator(ComposableModel)>>whenWindowChanged: 
At this point before the window is created, the i-var 'window' holds
"NewValueHolder [ nil ]".  
A few steps later it registers "outerBlock" on i-var 'window' for
announcement ValueChanged .
It is interesting that the action is registered before the window is
opened/exists.

Trace Part 2. 
After a , the "outerBlock" breakpoint occurs.
Down(or up?) the stack
InspectorNavigator(ComposableModel)>>openWithSpecLayout:
does "window value: (WindowModel new model: self)"
so that a few steps up NewValueHolder>>valueChanged: effectively
announces ValueChanged on i-var 'window'  .
such that WindowModel>>whenClosedDo:
executes to register the "innerBlock" on WindowModel's i-var
'isClosedHolder' for announcement ValueChanged.
The inspector then appears.

Trace Part 3. 
When the close button on the inspector is clicked, the "innerBlock"
breakpoint occurs.
Down the stack WindowModel>>windowIsClosing 
does "isClosedHolder value: true"
so that a few steps up NewValueHolder>>valueChanged: announces
ValueChanged on i-var 'isClosedHolder'
such that "self inspector close" is executed.

Armed with that understanding I found the following worked fine...
    PharoLauncher>>initialize
        super initialize.
        self whenWindowChanged: [ :w | w whenClosedDo: [ self
releaseSubscriptions ] ].
with support from 
    PharoLauncher>>releaseSubscriptions
        imagesModel releaseSubscriptions  
    PhLTitledTreeModel>>releaseSubscriptions
        self repository unsubscribe: self.

Where PharoLauncher and PhLTitledTreeModel are both subclassed from
ComposableModel, and PhLTitledTreeModel is composed into PharoLauncher
by the i-var 'imagesModel'.

However I could not help make the comparison of the simplicity of: 
    PharoLauncher>>windowIsClosing
       self releaseSubscriptons
which is 1 stack level in the debugger away from
WindowModel>>windowIsClosing and took about an hour to track down
and understand...

versus isClosedHolder:
which has 22 stack levels for both Trace Part 1 and Trace Part 2 from
the two levels of redirection with Announcements and taken many hours
of play to get to my current point of understanding.

So what is gained in elegance from doing it the isClosedHolder way? 

I thought that maybe... since all these components are meant to be
composable, PhLTitledTreeModel should be self-contained about about
releasing its subscriptions, by registering on isClosedHolder itself at
the point where the subscription-of-concern is created. Otherwise
relying on its owner PharoLauncher to call PhLTitledTreeModel
>>releaseSubscriptions means if PhLTitledTreeModel is composed
elsewhere in ClassX, then the onus is on ClassX to do the same thing. I
had hoped that adding the last line here would work..

Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-18 Thread btc




Thanks, I'll look into it.
cheers -ben

Benjamin wrote:

  
I think i would be more elegant that the model register to the
isClosedHolder changes :)
  
  Ben
  
  
  
  On 18 Jan 2014, at 12:09, b...@openinworld.com wrote:
  
  PharoLauncher is currently leaking
AnnouncementSubscriptions when its window closes.  I'm not sure with
Spec how to ensure actions are performed when the window is closed, but
I have been able to achieve this with the following modification [1]
which is...
-
  WindowModel>>windowIsClosing
  isClosedHolder value: true
  self model windowIsClosing  ">close to trace through
and discover that it calls...
SpecWindow(StandardWindow)>>delete, which in my
non-fullscreen-case calls...
SpecWindow(SystemWindow)>>delete, which "model windowIsClosing"
calls...
MorphicWindowAdaptor>>windowIsClosing, which "self model
windowIsClosing" calls...
WindowModel>>windowIsClosing, which does only "isClosedHolder
value: true"

That last line happens to hold aPharoLauncher in its 'model' instance
variable, so with the above proposal I can implement
PharoLauncher>>windowIsClosing to unregister the announcer.
Any problem expected with that modification?

[1] https://pharo.fogbugz.com/f/cases/12677

cheers -ben



  
  
  







Re: [Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-18 Thread Benjamin
I think i would be more elegant that the model register to the isClosedHolder 
changes :)

Ben

On 18 Jan 2014, at 12:09, b...@openinworld.com wrote:

> PharoLauncher is currently leaking AnnouncementSubscriptions when its window 
> closes.  I'm not sure with Spec how to ensure actions are performed when the 
> window is closed, but I have been able to achieve this with the following 
> modification [1] which is...
> -
>   WindowModel>>windowIsClosing
>   isClosedHolder value: true
>   self model windowIsClosing  " -
> 
> I investigated with a halt in SpecWindow>>close to trace through and discover 
> that it calls...
> SpecWindow(StandardWindow)>>delete, which in my non-fullscreen-case calls...
> SpecWindow(SystemWindow)>>delete, which "model windowIsClosing" calls...
> MorphicWindowAdaptor>>windowIsClosing, which "self model windowIsClosing" 
> calls...
> WindowModel>>windowIsClosing, which does only "isClosedHolder value: true"
> 
> That last line happens to hold aPharoLauncher in its 'model' instance 
> variable, so with the above proposal I can implement 
> PharoLauncher>>windowIsClosing to unregister the announcer.
> Any problem expected with that modification?
> 
> [1] https://pharo.fogbugz.com/f/cases/12677
> 
> cheers -ben
> 
> 
> 



[Pharo-dev] #windowIsClosing not sent to model of WindowModel

2014-01-18 Thread btc
PharoLauncher is currently leaking AnnouncementSubscriptions when its 
window closes.  I'm not sure with Spec how to ensure actions are 
performed when the window is closed, but I have been able to achieve 
this with the following modification [1] which is...

-
   WindowModel>>windowIsClosing
   isClosedHolder value: true
   self model windowIsClosing  ">close to trace through and 
discover that it calls...

SpecWindow(StandardWindow)>>delete, which in my non-fullscreen-case calls...
SpecWindow(SystemWindow)>>delete, which "model windowIsClosing" calls...
MorphicWindowAdaptor>>windowIsClosing, which "self model 
windowIsClosing" calls...

WindowModel>>windowIsClosing, which does only "isClosedHolder value: true"

That last line happens to hold aPharoLauncher in its 'model' instance 
variable, so with the above proposal I can implement 
PharoLauncher>>windowIsClosing to unregister the announcer.

Any problem expected with that modification?

[1] https://pharo.fogbugz.com/f/cases/12677

cheers -ben