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 and understand.

+1

Dave

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@rowledge.org; http://www.rowledge.org/tim It said, "Insert disk #3," but only two will fit!

On 21-10-2017, at 12:05 PM, Kjell Godo squeaklist@gmail.com wrote:

On Fri, Oct 20, 2017 at 21:05 Ben Coman btc@openinworld.com wrote: On Sat, Oct 21, 2017 at 8:04 AM, tim Rowledge tim@rowledge.org wrote: 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 don't have access to either Squeak nor Pharo image to confirm, but in Pharo I vaguely remember that pragmas (a bit non-intuitively) are found via senders-of. Maybe Squeak is similar??

It is, and I found it accidentally via random flailing (I need to write a book on Programming by Random Flailing. It’s evidently what most of the commercial software world does) and whilst it is just fine as a quick and dirty approach it really isn’t what a helpful IDE should do. After all the typical related code is listing the selector in a way that has always meant a message being sent by this code and find it by listing implementors. We can do better.

in Pharo the Finder Window has a drop down menu for finding Pragmas

That sounds like a decent start. It’s like Traits and now Environments; good ideas screwed up by being incomplete and untooled.

tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Fractured Idiom:- MAZEL TON - Lots of luck

Thanks for taking a look at this so quickly Dave.

In fact you were *so* quick that you got the slightly broken changeset I replaced within a few minutes! That removed the definitely spurious#modalFileSelectorForPattern: method which was a left-over from the work I was originally doing for the save-as stuff. I also cleaned out a few no-longer used methods.

The comments about confusion have clearly served their purpose in alerting reviewers to stuff I didn’t resolve. It’s nice to know they got read :-) The follow up must be to actually do a better commenting job on the systemnavigation/toolset/standardtoolset/systembrowser/uimanager side.

I’m also interested in suggestions for updating the BrowserTest>>#testBuildMessageCategoryBrowserEditString method to do something meaningful with the partial-edit text; right now the browser opens and sort of sticks there because of the ‘do you want to cancel’ dialogue. And indeed, does the reinstated partial-edit stuff actually do what we want? It’s been quite a while since it worked and I had completely forgotten about it.

As a follow up on the untested MVC effects, I have confusing news. A fully updated image lets me open an MVC project and make browsers etc, although they look a bit odd with some parts coloured and some monochrome. I get quite a lot of uncleared gribbleys on the display when moving them. Also my first attempt to load my changes from within the MVC project (just in case) was stymied by a very broken FileList that completely messed up trying to navigate directories. Eventually I was able to open a file view on my changes. An attempt to file it in was problematic because the menu looks like this -

It might take a moment to spot that the ‘more…’ is not at the bottom and clicking on the ‘filein entire file’ crashes because the index in to the menu was 21 (I think - way greater than the number of elements anyway) ! Eventually I selected the entire text and used the ‘filein selection’, which loaded the code ok. Opening a browser or workspace seemed fine at this point, so that implies my changes didn’t break anything directly involved. Still trying to work out what might be wrong.

tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Science is imagination equipped with grappling hooks.

After tracing through the menu building (oh, my) the problem I see with an MVC file view window menu is that the services added in (I think) SimpleServiceEntry>>#addServiceFor:toMenu: add the label and selector and argument BUT there is an assumption about the list of arguments which is unmet. Thus we end up with a menu having a 21 entry list of labels/selectors and only 4 items of arguments. So the code at the end of CustomMenu>>#invokeOn:orSendTo: that tries to do `ifFalse: [recipient perform: aSelector withArguments: (self arguments at: anIndex)]]` … which must fail since arguments at: 21 cannot work.

tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Strange OpCodes: SLTMDL: Shift Left, Test Mask and Dim the Lights

I’ve added Tools-tpr.769 to the inbox, so as long ayou have an up to date image you ought have no problem loading the proposed Browser build changes to review them.

tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim A computer program does what you tell it to do, not what you want it to do.

I like your changes. Looking through them I found functionality I did not know existed, e.g. the ClassListBrowser to browse arbitrary list of classes and the Multi-window browsers preference.

Thanks for doing some cleaning.

Bernhard

Am 23.10.2017 um 00:54 schrieb tim Rowledge tim@rowledge.org:

I’ve added Tools-tpr.769 to the inbox, so as long ayou have an up to date image you ought have no problem loading the proposed Browser build changes to review them.

tim

tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim A computer program does what you tell it to do, not what you want it to do.

OK, having heard no complaints about the proposed changes to Browser assembly, I am moving it from inbox to trunk….

tim -- tim Rowledge; tim@rowledge.org; http://www.rowledge.org/tim Guests who kill talk show hosts--On the last Geraldo.