Bert Freudenberg uploaded a new version of System to project The Inbox: http://source.squeak.org/inbox/System-bf.997.mcz
==================== Summary ====================
Name: System-bf.997 Author: bf Time: 25 January 2018, 4:56:35.380251 pm UUID: 604954ca-0389-43c0-a5c7-4ddc8e613cc9 Ancestors: System-dtl.996
Simplify project dispatch selector detection. No hard-coded list. Almost as good.
=============== Diff against System-dtl.996 ===============
Item was removed: - ----- Method: Project class>>baseSelectors (in category 'dispatching') ----- - baseSelectors - "The list of known base selectors that may be dispatched to project specific - implementations. For example, #OpenLabel:in: will be dispatched to #mvcOpenLabel:in: - for its MVC specific implementation. Add new base selectors here if additional methods - are added as targets of the dispatchTo:addPrefixAndSend:withArguments: mechanism." - - ^ { - #StartUpLeftFlush . - #StartUpWithCaption:icon:at:allowKeyboard: . - #OpenLabel:in: . - #Open: . - #Open . - #OpenOn:context:label:contents:fullView: . - #ResumeProcess: . - #OpenContext:label:contents: - } - !
Item was removed: - ----- Method: Project class>>dispatchSelectors (in category 'dispatching') ----- - dispatchSelectors - "All known targets of the dispatch mechanism" - - "Project dispatchSelectors" - - | selectors dispatchPrefixes | - selectors := OrderedCollection new. - dispatchPrefixes := Set withAll: (Project allSubclasses collect: [ :e | e basicNew selectorPrefixForDispatch ]). - self baseSelectors do: [ :base | - dispatchPrefixes do: [ :prefix | - selectors add: (prefix, base) asSymbol ] ]. - ^ selectors - - !
Item was changed: ----- Method: Project class>>isDispatchSelector: (in category 'dispatching') ----- isDispatchSelector: aSelector "In support of package modularity, some method selectors are generated based on project type and dispatched to the appropriate implementation for that project. For methods with these selectors, let dispatchTo:addPrefixAndSend:withArguments: be found as a sender." + | dispatchPrefixes prefix otherSelector | + dispatchPrefixes := Project allSubclasses collect: + [:cls | cls basicNew selectorPrefixForDispatch]. + "If it doesn't start with a recognized prefix, it's not dispatchable" + prefix := dispatchPrefixes detect: [:each | aSelector beginsWith: each] + ifNone: [^false]. + "If a similar symbol exists for all other prefixes, it's likely dispatchable" + ^(dispatchPrefixes copyWithout: prefix) + allSatisfy: [:otherPrefix | + otherSelector := otherPrefix, (aSelector allButFirst: prefix size). + Symbol hasInterned: otherSelector ifTrue: [:s]] - - ^ self dispatchSelectors includes: aSelector. !
On 25-01-2018, at 7:56 AM, commits@source.squeak.org wrote:
Simplify project dispatch selector detection. No hard-coded list. Almost as good.
Maybe I missed something about why this slightly odd abstraction was needed, but what is inadequate about simply having implementations of #startUpWithCaption:icon:at:allowKeyboard: in each project related class? How are these cases so different to the usages of UIManager et al. that we need yet another mechanism?
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Klingon Code Warrior:- 10) "This code is a piece of crap! You have no honor!"
On 25 January 2018 at 19:48, tim Rowledge tim@rowledge.org wrote:
On 25-01-2018, at 7:56 AM, commits@source.squeak.org wrote:
Simplify project dispatch selector detection. No hard-coded list. Almost
as good.
Maybe I missed something about why this slightly odd abstraction was needed, but what is inadequate about simply having implementations of #startUpWithCaption:icon:at:allowKeyboard: in each project related class? How are these cases so different to the usages of UIManager et al. that we need yet another mechanism?
This is code from before we had Project subclasses.
- Bert -
On Thu, Jan 25, 2018 at 10:13:56PM +0100, Bert Freudenberg wrote:
On 25 January 2018 at 19:48, tim Rowledge tim@rowledge.org wrote:
On 25-01-2018, at 7:56 AM, commits@source.squeak.org wrote:
Simplify project dispatch selector detection. No hard-coded list. Almost
as good.
Maybe I missed something about why this slightly odd abstraction was needed, but what is inadequate about simply having implementations of #startUpWithCaption:icon:at:allowKeyboard: in each project related class? How are these cases so different to the usages of UIManager et al. that we need yet another mechanism?
???This is code from before we had Project subclasses.
Actually, it is more recent (circa 2010) and you can blame me for it.
This was work that I did in early 2010 on package modularity to make MVC be a fully unloadable/reloadable package.
The dispatch mechanism was added here:
Name: System-dtl.241 Author: dtl Time: 30 January 2010, 10:00:40.376 pm UUID: 4d4d420e-4618-4fe6-ab0f-c63a28bb622f Ancestors: System-dtl.240
Add Project>>dispatchTo:addPrefixAndSend:withArguments: Allow classes with MVC and Morphic dependencies to dispatch through Project current to invoke appropriate methods. Prevents accumulation of unrelated implementations in Project.
And the end result was this:
Name: System-dtl.255 Author: dtl Time: 15 February 2010, 10:37:15.416 pm UUID: b70348f2-a094-4adc-80fa-696552b171f0 Ancestors: System-dtl.254
Update SystemDictionary>>discardMVC to perform complete MVC removal. MVC may be reinstalled by loading packages ST80 and ToolBuilder-MVC.
I am very pleased with the reloadable MVC capability, and I would like to do the same for Morphic. I am not so happy about my dispatching hack, so if someone can propose a better alternative, please do so. I am /sure/ there must be a better way to do this :-)
Dave
I spent some (too much) time looking at how to remove the need for the Project>dispatchTo:addPrefixAndSend:withArguments: method that I find rather irritating.
I was able to find ways to remove it, though in several cases this was by an only slightly less ugly double-dispatch - though at least it doesn’t work by sleight of hand. tl;dr - see the bottom of this email ya lazy scunner.
Following is my notes during the work - =============== PopUpMenu>startUpLeftFlush I don’t see any way that the original scenario for having this exist could still exist. Not to mention that MorphicProject already has its own #jumpToProject, making the entire dance pointless. Making MVCProject>>#jumpToProject just directly use PopUpMenu>>#mvcStartUpLeftFlush is a simple and direct improvement.
PopUpMenu>>#startUpWithCaption:icon:at:allowKeyboard: NB: This mixes in the entire strangeness of MVCMenuMorph, which is used oddly. Divert via Project current uiManager instead of #dispatch…
This really demonstrates what a terrible mess we have for menus. A complete replacement would be nice.
SyntaxError class>>#open: This probably ought to be dealt with via a suitable ToolBuilder assembling the correct bits.
PluggableFileList open stuff We get two different versions depending upon whether we use a form of `PluggableFileList getFolderDialog openLabel: ‘foo’ ` {which is explicitly used in MVCProject>>#findAFolderForProject:label:} or `PluggableFileList new open` PluggableFileList is only really used in - MVCProject>>#findAFolderForProject:label: which is used as part of Project loading and saving. The MorphicProject version explicitly uses FileList2 instead, so no need for Project based dispatching. In an MVCProject the ToolBuilder version fails because of a fatal problem in #buildPluggablePanel: - the children stuff is known faulty and needs fixing before we can do more. The not-ToolBuilder version is not used here. We should change to a file dialogue instead anyway. - Several StandardFileMenu methods when non-Morphic (which would break other kinds of project of course) so no need for project dispatching. As a temporary ‘fix’ I made a dispatch via UIManager.
MailComposition - doesn't actually need a dispatch via Project since it is a ToolBuilder UI. Fixed the #open method
FancyMailComposition Not a ToolBuilder thing yet. Weirdly two versions of morphic UI; a relatively clean one (FancyMailComposition new open) and a lurid purple one (FancyMailComposition new openInMorphic). Is the lurid one intended to go with the violent blue file list for EToys? There are no senders of openInMorphic in the alpha image. Is there any good reason to have two extremely similar classes in the system? Does one of these actually do anything especially useful?
Debugger>>#resumeProcess: is only sent from within the Debugger and Debugger>>#morphicResumeProcess: & Debugger>>#mvcResumeProcess: look tantalisingly similar. Maybe someone with recent experience of debugger hacking might look to merge them cleanly.
Debugger class>>#context: was easy enough to simplify
… and removes need for different Debugger class>>#mvcOpenContext:label:contents: and Debugger class>>#morphicOpenContext:label:contents:
Debugger class>>#openOn:context:label:contents:fullView: again, the two versions are really similar but not quite identical and could almost certainly be merged to allow removal of the project dispatch. ================
Summary - Some work on a couple of Debugger methods could clean up some important code and reduce the confusion always engendered by splitting similar paths. - FancyMailComposition may be something to remove and replace with the plain MailComposition. And purple? Really? - PluggableFileList (and superclasses). Eurgh. So much complication and confusion. Best to replace it with a clean new class and decide exactly what we want. - SyntaxError; convert to ToolBuilder. - PopUpMenu and friends; aaaaaaaaaaaaaarrrrrgh! So much hideousness in so many places doing so many nasty things so many times over.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim I majored in Art and Logic. Now I draw my own conclusions
Thanks for these notes Tim.
There might be a few good "Sunday Squeaker" projects in this list if anyone is looking for one :-)
Dave
On Mon, Jan 29, 2018 at 09:11:56PM -0800, tim Rowledge wrote:
I spent some (too much) time looking at how to remove the need for the Project>dispatchTo:addPrefixAndSend:withArguments: method that I find rather irritating.
I was able to find ways to remove it, though in several cases this was by an only slightly less ugly double-dispatch - though at least it doesn???t work by sleight of hand. tl;dr - see the bottom of this email ya lazy scunner.
Following is my notes during the work -
PopUpMenu>startUpLeftFlush I don???t see any way that the original scenario for having this exist could still exist. Not to mention that MorphicProject already has its own #jumpToProject, making the entire dance pointless. Making MVCProject>>#jumpToProject just directly use PopUpMenu>>#mvcStartUpLeftFlush is a simple and direct improvement.
PopUpMenu>>#startUpWithCaption:icon:at:allowKeyboard: NB: This mixes in the entire strangeness of MVCMenuMorph, which is used oddly. Divert via Project current uiManager instead of #dispatch???
This really demonstrates what a terrible mess we have for menus. A complete replacement would be nice.
SyntaxError class>>#open: This probably ought to be dealt with via a suitable ToolBuilder assembling the correct bits.
PluggableFileList open stuff We get two different versions depending upon whether we use a form of `PluggableFileList getFolderDialog openLabel: ???foo??? ` {which is explicitly used in MVCProject>>#findAFolderForProject:label:} or `PluggableFileList new open` PluggableFileList is only really used in
- MVCProject>>#findAFolderForProject:label: which is used as part of Project loading and saving. The MorphicProject version explicitly uses FileList2 instead, so no need for Project based dispatching. In an MVCProject the ToolBuilder version fails because of a fatal problem in #buildPluggablePanel: - the children stuff is known faulty and needs fixing before we can do more. The not-ToolBuilder version is not used here. We should change to a file dialogue instead anyway.
- Several StandardFileMenu methods when non-Morphic (which would break other kinds of project of course) so no need for project dispatching. As a temporary ???fix??? I made a dispatch via UIManager.
MailComposition
- doesn't actually need a dispatch via Project since it is a ToolBuilder UI. Fixed the #open method
FancyMailComposition Not a ToolBuilder thing yet. Weirdly two versions of morphic UI; a relatively clean one (FancyMailComposition new open) and a lurid purple one (FancyMailComposition new openInMorphic). Is the lurid one intended to go with the violent blue file list for EToys? There are no senders of openInMorphic in the alpha image. Is there any good reason to have two extremely similar classes in the system? Does one of these actually do anything especially useful?
Debugger>>#resumeProcess: is only sent from within the Debugger and Debugger>>#morphicResumeProcess: & Debugger>>#mvcResumeProcess: look tantalisingly similar. Maybe someone with recent experience of debugger hacking might look to merge them cleanly.
Debugger class>>#context: was easy enough to simplify
??? and removes need for different Debugger class>>#mvcOpenContext:label:contents: and Debugger class>>#morphicOpenContext:label:contents:
Debugger class>>#openOn:context:label:contents:fullView: again, the two versions are really similar but not quite identical and could almost certainly be merged to allow removal of the project dispatch. ================
Summary
- Some work on a couple of Debugger methods could clean up some important code and reduce the confusion always engendered by splitting similar paths.
- FancyMailComposition may be something to remove and replace with the plain MailComposition. And purple? Really?
- PluggableFileList (and superclasses). Eurgh. So much complication and confusion. Best to replace it with a clean new class and decide exactly what we want.
- SyntaxError; convert to ToolBuilder.
- PopUpMenu and friends; aaaaaaaaaaaaaarrrrrgh! So much hideousness in so many places doing so many nasty things so many times over.
tim
tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim I majored in Art and Logic. Now I draw my own conclusions
On 30-01-2018, at 5:43 AM, David T. Lewis lewis@mail.msen.com wrote:
Thanks for these notes Tim.
There might be a few good "Sunday Squeaker" projects in this list if anyone is looking for one :-)
Exactly. If there’s no major fault found with the changes I’ll properly push them out in a few days.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim "Bother" said Pooh, and deleted his message base
On 30-01-2018, at 9:56 AM, tim Rowledge tim@rowledge.org wrote:
Exactly. If there’s no major fault found with the changes I’ll properly push them out in a few days.
On 01-02-2018, at 4:13 PM, commits@source.squeak.org wrote:
tim Rowledge uploaded a new version of System to project The Trunk: http://source.squeak.org/trunk/System-tpr.999.mcz
A warning - this stuff messes with the debugger amongst other things and so might break stuff. It all works in my systems but y’never know for other people.
tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Useful random insult:- Understands English as well as any parrot.
On Thu, Feb 01, 2018 at 04:18:02PM -0800, tim Rowledge wrote:
On 30-01-2018, at 9:56 AM, tim Rowledge tim@rowledge.org wrote:
Exactly. If there???s no major fault found with the changes I???ll properly push them out in a few days.
On 01-02-2018, at 4:13 PM, commits@source.squeak.org wrote:
tim Rowledge uploaded a new version of System to project The Trunk: http://source.squeak.org/trunk/System-tpr.999.mcz
A warning - this stuff messes with the debugger amongst other things and so might break stuff. It all works in my systems but y???never know for other people.
Oh very good, thanks for getting this under way. No obvious problems that I can see so far. I also have SqueakShellProject in my image, and I don't see any issues there either.
Dave
squeak-dev@lists.squeakfoundation.org
-
Bert Freudenberg -
commits＠source.squeak.org -
David T. Lewis -
tim Rowledge