[squeak-dev] Changes to Browser building/opening
David T. Lewis
lewis at mail.msen.com
Sat Oct 21 02:18:46 UTC 2017
On Fri, Oct 20, 2017 at 09:29:06PM -0400, David T. Lewis wrote:
> Review notes:
> The affected packages are System, ToolBuilder, Morphic, Tools, ST80, and ToolBuilder-Morphic.
> Make sure your trunk image is up to date for those packages, then file in
> the change set that Tim posted.
> I have not really gone through the changes in Tools yet (which are the heart
> of the matter), but I will offer some nit-picky comments just to get the
> conversation started.
> Changes to package System affect a couple of method comments, all good.
> The change in package ToolBuilder is minor, no issues, but I didn't mind
> the old version that was more wordy but maybe more intention revealing? Fine
> either way though.
> Package Morphic has a comment added to browseClassFromIt that says "OK, so
> do we actually need to go via systemNavigation or is ToolSet ok". I don't
> know the answer ... good question for the squeak-dev list but I fear that
> the method comment may sit there for the next 20 years without an answer ;-)
> TextEditor>>browseIt is a good simplification, but the comment about "more
> confusion here" leaves me more confused, so maybe delete the comment and
> raise the question on squeak-dev?
> The change in package ToolBuilder-Morphic might be a mistake, as there are
> no implementors of #modalFileSelectorForPattern: (or is something missing
> from the change set?
> ST80 changes are a simplification of #browseClassFromIt (good) and a comment
> in #browseIt. The comment says "more confusion here" which does not leave
> me any less confused, could we discuss on the list instead?
> Most of the real changes are in Tools, about to start browsing them now :-)
The Tools changes look like a nice cleanup that makes the code easier to read
> On Fri, Oct 20, 2017 at 05:04:06PM -0700, tim Rowledge wrote:
> > I hurt my back a couple of weeks ago and so I found myself forced to sit on my office chair as the least painful place to be. With nothing urgent to do other than avoid boredom, I finally thought to myself that I would attack an annoyance that has been annoying me for years in Squeak.
> > Well as is so often the case, as I dug into what to do I got distracted. In this case I stumbled upon the assorted Browser related methods for building the browsers. So far as I can see waaaay back when we adopted the ToolBuilder idea a whole bunch of methods got hacked about to work with it and then just left to moulder. For example a number of messages that clearly used to actually build windows got turned into code that did no such thing and instead assembled a spec to pass to a toolbuilder. The comments usually didn???t even get updated. Oh, and the ability to spawn browsers with a partially edited bit of code seems to have gone away at that point. And the simple toolbuilder idiom that allows
> > ToolBuilder open: BlahClass
> > didn???t seem to work too well froBrowsers either. Part of that is because the Browser code can build a stack of different UIs instead of the simplistic approach of 'one class one UI???. We could have split Browser up a bit to make that work, and maybe we still should. It has appeal.
> > There was much related ???fun??? whilst digging into the area, where I discovered that we have SystemNavigation having some code that tries to build assorted browsers (with many comments containing broken examples, sigh), ToolSet and StandardToolSet gathering some Browser building and SystemBrowser doing nothing more complex than deciding between Browser and PackagePaneBrowser. None of this has much in the way of helpful commentary. Oh and let???s not forget UIManager which seems to get tangled in it as well. I found places where a browser was referred to both via ToolSet and Browser. And whilst having pragmas to mark menu related methods is a neat idea it would be a whole lot better if only you could sensibly find the damned things with implementors-of etc.
> > I made a small pile of changes to clean up at least some of this. I???ve made the building more explicit and enabled 'ToolBuilder open: Browser/HierarchyBrowser/ClassListBrowser' etc. I managed to make the spawning-with-partial-text thing work in all the cases I can find. I think I broke something in MVC projects but right now I can???t even get into an mvc project to look - and given that I didn???t (knowingly) touch changesorters I???m not sure it???s my fault.
> > The changes range across several MC packages so I think it is better to share the changes file for an initial look-see. It???s on the swiki at http://wiki.squeak.org/squeak/2736
> > tim
> > --
> > tim Rowledge; tim at rowledge.org; http://www.rowledge.org/tim
> > It said, "Insert disk #3," but only two will fit!
More information about the Squeak-dev