Refactoring Browser Enhancements
Locked
Refactoring Browser Enhancements
 
|
Hi lukas
with Stef we refactored the Browser environment: - now it can be parametrized by a system dictionary - we also move selectionIn* to the AST-Core so that Environment can be loaded independently. Could you integrate the packages in rb? There are AST-Core Refactoring-Critics Refactoring-Environment Refactoring-Test-Envrionment In DirtyExperiments on SqueakSource Ben PS: All the test are green ;) |
Re: Refactoring Browser Enhancements
|
|
On 18 March 2011 18:39, Benjamin <[hidden email]> wrote:
> Hi lukas > > with Stef we refactored the Browser environment: > - now it can be parametrized by a system dictionary > - we also move selectionIn* to the AST-Core so that Environment can be loaded independently. > > Could you integrate the packages in rb? No, please read the mail I sent this morning to the list. 1. 'BrowserEnvironment' is supposed to be *stateless*. BrowserEnvironment is supposed to *always* represent exactly the same thing. The only refactoring that would make sense is to extract all senders of 'Smalltalk globals' to a separate method so that subclasses could override it. 2. The change where you move code from 'Refactoring-Enviornment' to 'AST-Core' introduces a bad dependency. 'AST-Core' is at the lowest level, it should not depend on anything else. Ideally the dependency between 'Refactoring-Enviornment' to 'AST-Core' should be cut, but that would involve a third package. In other Smalltalks the code in question is packaged with the UI. Lukas -- Lukas Renggli www.lukas-renggli.ch |
refactoring.pdf (83K)
Re: Refactoring Browser Enhancements
|
|
On Mar 18, 2011, at 7:29 PM, Lukas Renggli wrote: > On 18 March 2011 18:39, Benjamin <[hidden email]> wrote: >> Hi lukas >> >> with Stef we refactored the Browser environment: >> - now it can be parametrized by a system dictionary >> - we also move selectionIn* to the AST-Core so that Environment can be loaded independently. >> >> Could you integrate the packages in rb? > > No, please read the mail I sent this morning to the list. > > 1. 'BrowserEnvironment' is supposed to be *stateless*. > BrowserEnvironment is supposed to *always* represent exactly the same > thing. Exaclty the environment we want to browse. When I import Pharo 1.3 in Moose running in Pharo 1.2 I want to be able to refer to the environment that contain Pharo 1.3 so I just pass this environment to the browser environment and all the others tools. I do not see why I would have to have another subclass. > The only refactoring that would make sense is to extract all > senders of 'Smalltalk globals' to a separate method so that subclasses > could override it. Why? Frankly you have so absolute statements. This is good to feel like idiots: "the only refactoring that makes sense" We are making ****ALL**** the tools environment aware: SystemNavigation, Compiler, TestRunner.... so why browserEnvironment would escape to the rule? We need that to be able to browse, compile other code than currently installed and we will do it. Why having a state is a problem? Sorry but we will use this version and we will probably introduce it in core. Because we want to have all the tool chain been able to work on a given environment. > > 2. The change where you move code from 'Refactoring-Enviornment' to > 'AST-Core' introduces a bad dependency. 'AST-Core' is at the lowest > level, it should not depend on anything else. Ideally the dependency > between 'Refactoring-Enviornment' to 'AST-Core' should be cut, but > that would involve a third package. In other Smalltalks the code in > question is packaged with the UI. Correct I did not pay attention, we will introduce a third package. Now if you do not like our changes we will fork. This is sad because we tried to make sure that RB could work on them but there is no problem we can fork also RB. Just let us know. Stef |
Re: Refactoring Browser Enhancements
|
|
In reply to this post by Lukas Renggli
Benjamin can you publish a new package named Refactoring-Enviroment-Extension as you started to do.
Probably too much music or cookies (or magic card discussions). Stef >> 2. The change where you move code from 'Refactoring-Enviornment' to > 'AST-Core' introduces a bad dependency. 'AST-Core' is at the lowest > level, it should not depend on anything else. Ideally the dependency > between 'Refactoring-Enviornment' to 'AST-Core' should be cut, but > that would involve a third package. In other Smalltalks the code in > question is packaged with the UI. > > Lukas > > -- > Lukas Renggli > www.lukas-renggli.ch > <refactoring.pdf> |
Re: Refactoring Browser Enhancements
|
|
In reply to this post by Stéphane Ducasse
>> The only refactoring that would make sense is to extract all
>> senders of 'Smalltalk globals' to a separate method so that subclasses >> could override it. > > Why? > Frankly you have so absolute statements. > This is good to feel like idiots: "the only refactoring that makes sense" > > We are making ****ALL**** the tools environment aware: SystemNavigation, Compiler, > TestRunner.... so why browserEnvironment would escape to the rule? > We need that to be able to browse, compile other code than currently installed and > we will do it. > > Why having a state is a problem? Sorry but we will use this version and we will probably introduce it in core. > Because we want to have all the tool chain been able to work on a given environment. You clearly break the following methods and potentially all 55 references to BrowserEnvironment: - BrowserEnvironment>>#copyEmpty - BrowserEnvironment>>#& - BrowserEnvironment>>#I - BrowserEnvironment>>#not - BrowserEnvironment>>#isSystem - ClassEnvironment>>#classesDo: - SelectorEnvironment>>#classesDo: Not sure how you are going to meaningfully fix these when BrowserEnvironment can point to different environments? I can only repeat myself: BrowserEnvironment encapsulates the complete system (world). All that you can potentially see within a given image at a given time. If you have Pharo 1.2 and Pharo 1.3 in your image then BrowserEnvironment would enumerate Object from Pharo 1.2 and Object from Pharo 1.3. > Now if you do not like our changes we will fork. This is sad because we tried to make sure that > RB could work on them but there is no problem we can fork also RB. Just let us know. I don't know why you get angry? I am just telling you how John Brant and Don Roberts implemented the system. Just fundamentally changing core classes of a framework without understanding the implications doesn't help. If you don't believe me ask John or compare the code with the one in VisualWorks where you could actually have different system versions at the same time in the image. So again: If you want a view onto your namespace create a BrowserEnvironmentWrapper subclass that holds a reference to your namespace like PackageEnvironment with its package. That is how it is ment to be done. Lukas -- Lukas Renggli www.lukas-renggli.ch |
Re: Refactoring Browser Enhancements
|
|
On Mar 18, 2011, at 10:09 PM, Lukas Renggli wrote: >>> The only refactoring that would make sense is to extract all >>> senders of 'Smalltalk globals' to a separate method so that subclasses >>> could override it. >> >> Why? >> Frankly you have so absolute statements. >> This is good to feel like idiots: "the only refactoring that makes sense" >> >> We are making ****ALL**** the tools environment aware: SystemNavigation, Compiler, >> TestRunner.... so why browserEnvironment would escape to the rule? >> We need that to be able to browse, compile other code than currently installed and >> we will do it. >> >> Why having a state is a problem? Sorry but we will use this version and we will probably introduce it in core. >> Because we want to have all the tool chain been able to work on a given environment. > > You clearly break the following methods and potentially all 55 > references to BrowserEnvironment: > > - BrowserEnvironment>>#copyEmpty > - BrowserEnvironment>>#& > - BrowserEnvironment>>#I > - BrowserEnvironment>>#not > - BrowserEnvironment>>#isSystem > - ClassEnvironment>>#classesDo: > - SelectorEnvironment>>#classesDo: We will fix them. We forgot to look at BrowserEnvironement references. > Not sure how you are going to meaningfully fix these when > BrowserEnvironment can point to different environments? > > I can only repeat myself: BrowserEnvironment encapsulates the complete > system (world). All that you can potentially see within a given image > at a given time. If you have Pharo 1.2 and Pharo 1.3 in your image > then BrowserEnvironment would enumerate Object from Pharo 1.2 and > Object from Pharo 1.3. No it should not. And how about browsing remote images? This is exactly the same that SystemNavigation. Now systemNavigation works on an systemDictionary instance and of course if you work on two different ones you get something silly but else you can pass an systemDictionary instance and contextualize your queries. >> Now if you do not like our changes we will fork. This is sad because we tried to make sure that >> RB could work on them but there is no problem we can fork also RB. Just let us know. > > I don't know why you get angry? I am just telling you how John Brant > and Don Roberts implemented the system. Because you do not realize that your mails are often aggressive. Probably you are right in 80% of the case but consider that your mails can be perceived as aggressive. Trust lukas this is the way they can be read. I know you so I often succeed to take them easy but pay attention. > Just fundamentally changing > core classes of a framework without understanding the implications > doesn't help. If you don't believe me ask John or compare the code > with the one in VisualWorks where you could actually have different > system versions at the same time in the image. > > So again: If you want a view onto your namespace create a > BrowserEnvironmentWrapper subclass that holds a reference to your > namespace like PackageEnvironment with its package. That is how it is > ment to be done. I know. I understand the design of this code. Now it does not work with the scenario I have. I want to have an environment parametrized IDE which can work on different versions of the same system (pharo for example). I want to browse a remote environment that is given to me via proxy. I want to have the possibility to browser Hazelkernel separatedly from the rest and all these classes should not be in the same system dictionary instances. > > Lukas > > -- > Lukas Renggli > www.lukas-renggli.ch > |
Re: Refactoring Browser Enhancements
|
|
In reply to this post by Lukas Renggli
BTW lukas I'm not angry at you :).
I will look at your point and see how we can fix them. I just need this behavior. I have been fixing SystemNavigation (which should be also rewritten or replace), next the compiler should be parametrized and also the test runner logic and probably MC and at the end we will have a system that can do what I need. Stef On Mar 18, 2011, at 10:09 PM, Lukas Renggli wrote: >>> The only refactoring that would make sense is to extract all >>> senders of 'Smalltalk globals' to a separate method so that subclasses >>> could override it. >> >> Why? >> Frankly you have so absolute statements. >> This is good to feel like idiots: "the only refactoring that makes sense" >> >> We are making ****ALL**** the tools environment aware: SystemNavigation, Compiler, >> TestRunner.... so why browserEnvironment would escape to the rule? >> We need that to be able to browse, compile other code than currently installed and >> we will do it. >> >> Why having a state is a problem? Sorry but we will use this version and we will probably introduce it in core. >> Because we want to have all the tool chain been able to work on a given environment. > > You clearly break the following methods and potentially all 55 > references to BrowserEnvironment: > > - BrowserEnvironment>>#copyEmpty > - BrowserEnvironment>>#& > - BrowserEnvironment>>#I > - BrowserEnvironment>>#not > - BrowserEnvironment>>#isSystem > - ClassEnvironment>>#classesDo: > - SelectorEnvironment>>#classesDo: > > Not sure how you are going to meaningfully fix these when > BrowserEnvironment can point to different environments? > > I can only repeat myself: BrowserEnvironment encapsulates the complete > system (world). All that you can potentially see within a given image > at a given time. If you have Pharo 1.2 and Pharo 1.3 in your image > then BrowserEnvironment would enumerate Object from Pharo 1.2 and > Object from Pharo 1.3. > >> Now if you do not like our changes we will fork. This is sad because we tried to make sure that >> RB could work on them but there is no problem we can fork also RB. Just let us know. > > I don't know why you get angry? I am just telling you how John Brant > and Don Roberts implemented the system. Just fundamentally changing > core classes of a framework without understanding the implications > doesn't help. If you don't believe me ask John or compare the code > with the one in VisualWorks where you could actually have different > system versions at the same time in the image. > > So again: If you want a view onto your namespace create a > BrowserEnvironmentWrapper subclass that holds a reference to your > namespace like PackageEnvironment with its package. That is how it is > ment to be done. > > Lukas > > -- > Lukas Renggli > www.lukas-renggli.ch > |
Re: Refactoring Browser Enhancements
|
|
In reply to this post by Lukas Renggli
Let us be more productive :)
A BrowserEnvironment has an environment input= a systemDictionary (may be we should call it origin) a selectedEnviroment = a composite based on the BrowserEnvironment hierarchy The precondition is that it does not make sense to mix apple and orange so selectedEnvironment inside a composite to be meaningfull should sahre the same origin. Now it does not mean that we cannot have different composite over the same origin (ie we have multiple queries over Smalltalk globals) or that we cannot have different and separate composite working on different origin (ie I have a selection on Pharo1.2 and one on Pharo1.3 or a remote one). On Mar 18, 2011, at 10:09 PM, Lukas Renggli wrote: > - BrowserEnvironment>>#copyEmpty copyEmpty ^ self class new on: SystemDictionary new; yourself. > - BrowserEnvironment>>#& & anEnvironment "If we or anEnvironment includes everything, then just include the other environment (optimization)" self isSystem ifTrue: [^anEnvironment]. anEnvironment isSystem ifTrue: [^self]. ^AndEnvironment onEnvironment: self and: anEnvironment does not change > - BrowserEnvironment>>#I | anEnvironment "If we or anEnvironment includes everything, then return it instead of creating an or that will include everything." self isSystem ifTrue: [^self]. anEnvironment isSystem ifTrue: [^anEnvironment]. ^ OrEnvironment onEnvironment: self or: anEnvironment > - BrowserEnvironment>>#not not self isSystem ifTrue: [^SelectorEnvironment new]. ^NotEnvironment onEnvironment: self > - BrowserEnvironment>>#isSystem isSystem ^true No problem (this could be renamed isOriginalInput but we keep it for compatibiity > - ClassEnvironment>>#classesDo: classesDo: aBlock classes do: [ :each | | class | class := Smalltalk globals at: each ifAbsent: [ nil ]. (class notNil and: [ environment includesClass: class ]) ifTrue: [ aBlock value: class ] ]. metaClasses do: [ :each | | class | class := Smalltalk globals at: each ifAbsent: [ nil ]. (class notNil and: [ environment includesClass: class class ]) ifTrue: [ aBlock value: class class ] ] We already fixed it. classesDo: aBlock classes do: [ :each | | class | class := self environment at: each ifAbsent: [ nil ]. (class notNil and: [ selectedEnvironment includesClass: class ]) ifTrue: [ aBlock value: class ] ]. metaClasses do: [ :each | | class | class := self environment at: each ifAbsent: [ nil ]. (class notNil and: [ selectedEnvironment includesClass: class class ]) ifTrue: [ aBlock value: class class ] ] > - SelectorEnvironment>>#classesDo: classesDo: aBlock classSelectors keysDo: [ :each | | class | class := Smalltalk globals at: each ifAbsent: [ nil ]. (class notNil and: [ environment includesClass: class ]) ifTrue: [ aBlock value: class ] ]. metaClassSelectors keysDo: [ :each | | class | class := Smalltalk globals at: each ifAbsent: [ nil ]. (class notNil and: [ environment includesClass: class class ]) ifTrue: [ aBlock value: class class ] ] again we already fixed it. classesDo: aBlock classes do: [ :each | | class | class := self environment at: each ifAbsent: [ nil ]. (class notNil and: [ selectedEnvironment includesClass: class ]) ifTrue: [ aBlock value: class ] ]. metaClasses do: [ :each | | class | class := self environment at: each ifAbsent: [ nil ]. (class notNil and: [ selectedEnvironment includesClass: class class ]) ifTrue: [ aBlock value: class class ] ] So does it make sense? I think so. We just have a common input for a given composite. Stef |
Re: Refactoring Browser Enhancements
|
|
In reply to this post by Benjamin Van Ryseghem (Pharo)
Hi benjamin
I published a new package (to revert the dependencies) in DirtyExperiments and a fixed the points raised by lukas. I added also some comments. I should fix testEnvironmentWrapper | printString wrapper | printString := BrowserEnvironment new referencesTo: #printString. wrapper := BrowserEnvironmentWrapper onEnvironment: printString. self assert: wrapper numberSelectors = printString numberSelectors. self assert: wrapper numberClasses = printString numberClasses. self assert: wrapper environment == printString Stef On Mar 18, 2011, at 6:39 PM, Benjamin wrote: > Hi lukas > > with Stef we refactored the Browser environment: > - now it can be parametrized by a system dictionary > - we also move selectionIn* to the AST-Core so that Environment can be loaded independently. > > Could you integrate the packages in rb? > There are > AST-Core > Refactoring-Critics > Refactoring-Environment > Refactoring-Test-Envrionment > > In DirtyExperiments on SqueakSource > > > Ben > > PS: All the test are green ;) > > > > > |
Re: Refactoring Browser Enhancements
|
|
fixed now and published in DirtyExperiments.
Stef On Mar 19, 2011, at 9:31 AM, Stéphane Ducasse wrote: > Hi benjamin > > I published a new package (to revert the dependencies) in DirtyExperiments > and a fixed the points raised by lukas. I added also some comments. > I should fix > > testEnvironmentWrapper > | printString wrapper | > printString := BrowserEnvironment new referencesTo: #printString. > wrapper := BrowserEnvironmentWrapper onEnvironment: printString. > self assert: wrapper numberSelectors = printString numberSelectors. > self assert: wrapper numberClasses = printString numberClasses. > self assert: wrapper environment == printString > > Stef > > > On Mar 18, 2011, at 6:39 PM, Benjamin wrote: > >> Hi lukas >> >> with Stef we refactored the Browser environment: >> - now it can be parametrized by a system dictionary >> - we also move selectionIn* to the AST-Core so that Environment can be loaded independently. >> >> Could you integrate the packages in rb? >> There are >> AST-Core >> Refactoring-Critics >> Refactoring-Environment >> Refactoring-Test-Envrionment >> >> In DirtyExperiments on SqueakSource >> >> >> Ben >> >> PS: All the test are green ;) >> >> >> >> >> > > |
Re: Refactoring Browser Enhancements
|
|
In reply to this post by Stéphane Ducasse
So lukas
- did you check what we did? - will you include in rb packages? I need to know. Stef On Mar 19, 2011, at 7:55 AM, Stéphane Ducasse wrote: > Let us be more productive :) > > A BrowserEnvironment > has an environment input= a systemDictionary (may be we should call it origin) > a selectedEnviroment = a composite based on the BrowserEnvironment hierarchy > > The precondition is that it does not make sense to mix apple and orange so > selectedEnvironment inside a composite to be meaningfull should sahre the same origin. > > Now it does not mean that we cannot have different composite over the same origin (ie we have > multiple queries over Smalltalk globals) or that we cannot have different and separate composite > working on different origin (ie I have a selection on Pharo1.2 and one on Pharo1.3 or a remote one). > > > On Mar 18, 2011, at 10:09 PM, Lukas Renggli wrote: > >> - BrowserEnvironment>>#copyEmpty > > copyEmpty > ^ self class new on: SystemDictionary new; yourself. > >> - BrowserEnvironment>>#& > > & anEnvironment > "If we or anEnvironment includes everything, then just include the other environment (optimization)" > > self isSystem ifTrue: [^anEnvironment]. > anEnvironment isSystem ifTrue: [^self]. > ^AndEnvironment onEnvironment: self and: anEnvironment > > does not change > > >> - BrowserEnvironment>>#I > > | anEnvironment > "If we or anEnvironment includes everything, then return it instead of creating > an or that will include everything." > > self isSystem ifTrue: [^self]. > anEnvironment isSystem ifTrue: [^anEnvironment]. > ^ OrEnvironment onEnvironment: self or: anEnvironment > >> - BrowserEnvironment>>#not > not > self isSystem ifTrue: [^SelectorEnvironment new]. > ^NotEnvironment onEnvironment: self > > > >> - BrowserEnvironment>>#isSystem > isSystem > ^true > > No problem (this could be renamed isOriginalInput but we keep it for compatibiity > >> - ClassEnvironment>>#classesDo: > > classesDo: aBlock > classes do: [ :each | > | class | > class := Smalltalk globals at: each ifAbsent: [ nil ]. > (class notNil and: [ environment includesClass: class ]) > ifTrue: [ aBlock value: class ] ]. > metaClasses do: [ :each | > | class | > class := Smalltalk globals at: each ifAbsent: [ nil ]. > (class notNil and: [ environment includesClass: class class ]) > ifTrue: [ aBlock value: class class ] ] > We already fixed it. > > classesDo: aBlock > classes do: [ :each | > | class | > class := self environment at: each ifAbsent: [ nil ]. > (class notNil and: [ selectedEnvironment includesClass: class ]) > ifTrue: [ aBlock value: class ] ]. > metaClasses do: [ :each | > | class | > class := self environment at: each ifAbsent: [ nil ]. > (class notNil and: [ selectedEnvironment includesClass: class class ]) > ifTrue: [ aBlock value: class class ] ] > >> - SelectorEnvironment>>#classesDo: > > classesDo: aBlock > classSelectors keysDo: [ :each | > | class | > class := Smalltalk globals at: each ifAbsent: [ nil ]. > (class notNil and: [ environment includesClass: class ]) > ifTrue: [ aBlock value: class ] ]. > metaClassSelectors keysDo: [ :each | > | class | > class := Smalltalk globals at: each ifAbsent: [ nil ]. > (class notNil and: [ environment includesClass: class class ]) > ifTrue: [ aBlock value: class class ] ] > > again we already fixed it. > > classesDo: aBlock > classes do: [ :each | > | class | > class := self environment at: each ifAbsent: [ nil ]. > (class notNil and: [ selectedEnvironment includesClass: class ]) > ifTrue: [ aBlock value: class ] ]. > metaClasses do: [ :each | > | class | > class := self environment at: each ifAbsent: [ nil ]. > (class notNil and: [ selectedEnvironment includesClass: class class ]) > ifTrue: [ aBlock value: class class ] ] > > > So does it make sense? > I think so. > We just have a common input for a given composite. > > Stef |
Re: Refactoring Browser Enhancements
|
|
Yes, I looked at it and I gave detailed feedback in my previous mails.
No, I won't include it for the reasons given in my previous mails. Your changes subtly break assumptions made by any user of BrowserEnvironment (in addition to unnecessarily changing the API), but I am not going to repeat that and how to solve it differently a fourth time :-( Lukas On 20 March 2011 09:43, Stéphane Ducasse <[hidden email]> wrote: > So lukas > - did you check what we did? > - will you include in rb packages? > I need to know. > > Stef > > On Mar 19, 2011, at 7:55 AM, Stéphane Ducasse wrote: > >> Let us be more productive :) >> >> A BrowserEnvironment >> has an environment input= a systemDictionary (may be we should call it origin) >> a selectedEnviroment = a composite based on the BrowserEnvironment hierarchy >> >> The precondition is that it does not make sense to mix apple and orange so >> selectedEnvironment inside a composite to be meaningfull should sahre the same origin. >> >> Now it does not mean that we cannot have different composite over the same origin (ie we have >> multiple queries over Smalltalk globals) or that we cannot have different and separate composite >> working on different origin (ie I have a selection on Pharo1.2 and one on Pharo1.3 or a remote one). >> >> >> On Mar 18, 2011, at 10:09 PM, Lukas Renggli wrote: >> >>> - BrowserEnvironment>>#copyEmpty >> >> copyEmpty >> ^ self class new on: SystemDictionary new; yourself. >> >>> - BrowserEnvironment>>#& >> >> & anEnvironment >> "If we or anEnvironment includes everything, then just include the other environment (optimization)" >> >> self isSystem ifTrue: [^anEnvironment]. >> anEnvironment isSystem ifTrue: [^self]. >> ^AndEnvironment onEnvironment: self and: anEnvironment >> >> does not change >> >> >>> - BrowserEnvironment>>#I >> >> | anEnvironment >> "If we or anEnvironment includes everything, then return it instead of creating >> an or that will include everything." >> >> self isSystem ifTrue: [^self]. >> anEnvironment isSystem ifTrue: [^anEnvironment]. >> ^ OrEnvironment onEnvironment: self or: anEnvironment >> >>> - BrowserEnvironment>>#not >> not >> self isSystem ifTrue: [^SelectorEnvironment new]. >> ^NotEnvironment onEnvironment: self >> >> >> >>> - BrowserEnvironment>>#isSystem >> isSystem >> ^true >> >> No problem (this could be renamed isOriginalInput but we keep it for compatibiity >> >>> - ClassEnvironment>>#classesDo: >> >> classesDo: aBlock >> classes do: [ :each | >> | class | >> class := Smalltalk globals at: each ifAbsent: [ nil ]. >> (class notNil and: [ environment includesClass: class ]) >> ifTrue: [ aBlock value: class ] ]. >> metaClasses do: [ :each | >> | class | >> class := Smalltalk globals at: each ifAbsent: [ nil ]. >> (class notNil and: [ environment includesClass: class class ]) >> ifTrue: [ aBlock value: class class ] ] >> We already fixed it. >> >> classesDo: aBlock >> classes do: [ :each | >> | class | >> class := self environment at: each ifAbsent: [ nil ]. >> (class notNil and: [ selectedEnvironment includesClass: class ]) >> ifTrue: [ aBlock value: class ] ]. >> metaClasses do: [ :each | >> | class | >> class := self environment at: each ifAbsent: [ nil ]. >> (class notNil and: [ selectedEnvironment includesClass: class class ]) >> ifTrue: [ aBlock value: class class ] ] >> >>> - SelectorEnvironment>>#classesDo: >> >> classesDo: aBlock >> classSelectors keysDo: [ :each | >> | class | >> class := Smalltalk globals at: each ifAbsent: [ nil ]. >> (class notNil and: [ environment includesClass: class ]) >> ifTrue: [ aBlock value: class ] ]. >> metaClassSelectors keysDo: [ :each | >> | class | >> class := Smalltalk globals at: each ifAbsent: [ nil ]. >> (class notNil and: [ environment includesClass: class class ]) >> ifTrue: [ aBlock value: class class ] ] >> >> again we already fixed it. >> >> classesDo: aBlock >> classes do: [ :each | >> | class | >> class := self environment at: each ifAbsent: [ nil ]. >> (class notNil and: [ selectedEnvironment includesClass: class ]) >> ifTrue: [ aBlock value: class ] ]. >> metaClasses do: [ :each | >> | class | >> class := self environment at: each ifAbsent: [ nil ]. >> (class notNil and: [ selectedEnvironment includesClass: class class ]) >> ifTrue: [ aBlock value: class class ] ] >> >> >> So does it make sense? >> I think so. >> We just have a common input for a given composite. >> >> Stef > > > -- Lukas Renggli www.lukas-renggli.ch |
Re: Refactoring Browser Enhancements
|
|
On Mar 20, 2011, at 9:59 AM, Lukas Renggli wrote: > Yes, I looked at it and I gave detailed feedback in my previous mails. > > No, I won't include it for the reasons given in my previous mails. > Your changes subtly break assumptions made by any user of > BrowserEnvironment (in addition to unnecessarily changing the API), > but I am not going to repeat that and how to solve it differently a > fourth time :-( Too bad that you are stubborn. I'm too. The changes we are proposing to not break the default use of the system. ***All the queries that a normal user will do will share the same default environment *** and only when we will need to browse/query another systemDictionary we will have to make sure that the environment points to the right systemDictionary and that the queries access this root (which they do). Show me how this is wrong. We need it. So if you do not want to help I cannot do too much and I will fork because all our efforts to make the system parameterized will not be blocked just because you do not want to take 5 min to listen carefully what we are saying. Stef > > Lukas > > On 20 March 2011 09:43, Stéphane Ducasse <[hidden email]> wrote: >> So lukas >> - did you check what we did? >> - will you include in rb packages? >> I need to know. >> >> Stef >> >> On Mar 19, 2011, at 7:55 AM, Stéphane Ducasse wrote: >> >>> Let us be more productive :) >>> >>> A BrowserEnvironment >>> has an environment input= a systemDictionary (may be we should call it origin) >>> a selectedEnviroment = a composite based on the BrowserEnvironment hierarchy >>> >>> The precondition is that it does not make sense to mix apple and orange so >>> selectedEnvironment inside a composite to be meaningfull should sahre the same origin. >>> >>> Now it does not mean that we cannot have different composite over the same origin (ie we have >>> multiple queries over Smalltalk globals) or that we cannot have different and separate composite >>> working on different origin (ie I have a selection on Pharo1.2 and one on Pharo1.3 or a remote one). >>> >>> >>> On Mar 18, 2011, at 10:09 PM, Lukas Renggli wrote: >>> >>>> - BrowserEnvironment>>#copyEmpty >>> >>> copyEmpty >>> ^ self class new on: SystemDictionary new; yourself. >>> >>>> - BrowserEnvironment>>#& >>> >>> & anEnvironment >>> "If we or anEnvironment includes everything, then just include the other environment (optimization)" >>> >>> self isSystem ifTrue: [^anEnvironment]. >>> anEnvironment isSystem ifTrue: [^self]. >>> ^AndEnvironment onEnvironment: self and: anEnvironment >>> >>> does not change >>> >>> >>>> - BrowserEnvironment>>#I >>> >>> | anEnvironment >>> "If we or anEnvironment includes everything, then return it instead of creating >>> an or that will include everything." >>> >>> self isSystem ifTrue: [^self]. >>> anEnvironment isSystem ifTrue: [^anEnvironment]. >>> ^ OrEnvironment onEnvironment: self or: anEnvironment >>> >>>> - BrowserEnvironment>>#not >>> not >>> self isSystem ifTrue: [^SelectorEnvironment new]. >>> ^NotEnvironment onEnvironment: self >>> >>> >>> >>>> - BrowserEnvironment>>#isSystem >>> isSystem >>> ^true >>> >>> No problem (this could be renamed isOriginalInput but we keep it for compatibiity >>> >>>> - ClassEnvironment>>#classesDo: >>> >>> classesDo: aBlock >>> classes do: [ :each | >>> | class | >>> class := Smalltalk globals at: each ifAbsent: [ nil ]. >>> (class notNil and: [ environment includesClass: class ]) >>> ifTrue: [ aBlock value: class ] ]. >>> metaClasses do: [ :each | >>> | class | >>> class := Smalltalk globals at: each ifAbsent: [ nil ]. >>> (class notNil and: [ environment includesClass: class class ]) >>> ifTrue: [ aBlock value: class class ] ] >>> We already fixed it. >>> >>> classesDo: aBlock >>> classes do: [ :each | >>> | class | >>> class := self environment at: each ifAbsent: [ nil ]. >>> (class notNil and: [ selectedEnvironment includesClass: class ]) >>> ifTrue: [ aBlock value: class ] ]. >>> metaClasses do: [ :each | >>> | class | >>> class := self environment at: each ifAbsent: [ nil ]. >>> (class notNil and: [ selectedEnvironment includesClass: class class ]) >>> ifTrue: [ aBlock value: class class ] ] >>> >>>> - SelectorEnvironment>>#classesDo: >>> >>> classesDo: aBlock >>> classSelectors keysDo: [ :each | >>> | class | >>> class := Smalltalk globals at: each ifAbsent: [ nil ]. >>> (class notNil and: [ environment includesClass: class ]) >>> ifTrue: [ aBlock value: class ] ]. >>> metaClassSelectors keysDo: [ :each | >>> | class | >>> class := Smalltalk globals at: each ifAbsent: [ nil ]. >>> (class notNil and: [ environment includesClass: class class ]) >>> ifTrue: [ aBlock value: class class ] ] >>> >>> again we already fixed it. >>> >>> classesDo: aBlock >>> classes do: [ :each | >>> | class | >>> class := self environment at: each ifAbsent: [ nil ]. >>> (class notNil and: [ selectedEnvironment includesClass: class ]) >>> ifTrue: [ aBlock value: class ] ]. >>> metaClasses do: [ :each | >>> | class | >>> class := self environment at: each ifAbsent: [ nil ]. >>> (class notNil and: [ selectedEnvironment includesClass: class class ]) >>> ifTrue: [ aBlock value: class class ] ] >>> >>> >>> So does it make sense? >>> I think so. >>> We just have a common input for a given composite. >>> >>> Stef >> >> >> > > > > -- > Lukas Renggli > www.lukas-renggli.ch > |
«
Return to Pharo Smalltalk Developers
|
1 view|%1 views
| Free forum by Nabble | Edit this page |