The Inbox: System-bf.997.mcz

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

The Inbox: System-bf.997.mcz

commits-2
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.
  !


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-bf.997.mcz

timrowledge

> On 25-01-2018, at 7:56 AM, [hidden email] 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; [hidden email]; http://www.rowledge.org/tim
Klingon Code Warrior:- 10) "This code is a piece of crap!  You have no honor!"



Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-bf.997.mcz

Bert Freudenberg
On 25 January 2018 at 19:48, tim Rowledge <[hidden email]> wrote:

> On 25-01-2018, at 7:56 AM, [hidden email] 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 -


Reply | Threaded
Open this post in threaded view
|

Re: The Inbox: System-bf.997.mcz

David T. Lewis
On Thu, Jan 25, 2018 at 10:13:56PM +0100, Bert Freudenberg wrote:

> On 25 January 2018 at 19:48, tim Rowledge <[hidden email]> wrote:
>
> >
> > > On 25-01-2018, at 7:56 AM, [hidden email] 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


Reply | Threaded
Open this post in threaded view
|

Project>dispatchTo:... (was Re: The Inbox: System-bf.997.mcz)

timrowledge
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; [hidden email]; http://www.rowledge.org/tim
I majored in Art and Logic. Now I draw my own conclusions





Project-dispatch.cs (23K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Project>dispatchTo:... (was Re: The Inbox: System-bf.997.mcz)

David T. Lewis
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; [hidden email]; http://www.rowledge.org/tim
> I majored in Art and Logic. Now I draw my own conclusions
>
>

>


Reply | Threaded
Open this post in threaded view
|

Re: Project>dispatchTo:... (was Re: The Inbox: System-bf.997.mcz)

timrowledge


> On 30-01-2018, at 5:43 AM, David T. Lewis <[hidden email]> 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; [hidden email]; http://www.rowledge.org/tim
"Bother" said Pooh, and deleted his message base



Reply | Threaded
Open this post in threaded view
|

Re: Project>dispatchTo:... (was Re: The Inbox: System-bf.997.mcz)

timrowledge


> On 30-01-2018, at 9:56 AM, tim Rowledge <[hidden email]> 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, [hidden email] 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; [hidden email]; http://www.rowledge.org/tim
Useful random insult:- Understands English as well as any parrot.



Reply | Threaded
Open this post in threaded view
|

Re: Project>dispatchTo:... (was Re: The Inbox: System-bf.997.mcz)

David T. Lewis
On Thu, Feb 01, 2018 at 04:18:02PM -0800, tim Rowledge wrote:

>
>
> > On 30-01-2018, at 9:56 AM, tim Rowledge <[hidden email]> 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, [hidden email] 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