[squeak-dev] The Trunk: Morphic-eem.1719.mcz

Marcel Taeumel marcel.taeumel at hpi.de
Mon May 3 08:27:31 UTC 2021

Hi Christoph, hi all.

Let me add some more details about my reasoning behind Morphic-mt.1761.

First of all, let me explain the "code smells" that I found in the changeset:

1. Relying on the overly specific entry point Browser class >> #browseMethodFull.

2. Adapting HierarchyBrowser probably because of the first smell, breaking existing behavior.

3. Implicitly adding (and thus "simulating") a dynamic variable via set-and-ensure.


I noticed that the role of a "requestor" is participating in the "Reuse windows" preference. I am familiar with this role in the sense that model-view communication (e.g. PluggableListMorph to Browser) uses it to some extent. So, instead of being focused on who should not see that preference (i.e. hacking global state), it makes sense to design its intentions more clearly. In my opinion, that's the better object-oriented design. It's more robust.

Now, a safe way to pass a requestor along is as message argument. See Morph >> #mouseLeave: and EventHandler >> #mouseLeave:fromMorph: as an example. However, this role might emerge only later in the design process. Then you have the problem of old code relying on the existing interface. What then? A dynamically scoped variable might help.

Yet, dynamic scope can be challenging to maintain. Especially when having the notion of an ever-running image (object memory), which is highly state-based, dynamic scope can be easily messed up. Any object could choose to reset (and rely on) the scope for its own purpose. That's dangerous. Faulty behavior might become difficult to trace and understand without the right tools. Even with the right tools, the underlying design might have become accidentally more complex than it needs to be.

In Morphic, we have the dynamically scoped ActiveWorld(Variable), ActiveHand(Variable), and ActiveEvent(Variable) to support scenarios where one of those basic objects is not available in a messages send. For examples, browse senders of #currentEvent or #currentWorld. Unfortunately, we have to take care that only well-defined places reset this scope. Such managing of global contracts can be quite effortful. ... The user interrupt (i.e. CMD+Dot) comes to mind. It must work; the system must stay responsive.

Let's treat the use of a dynamic variable as "forward scope." There is another way. A way that avoids the risks of not knowing what might happen within that scope through sends to come. Instead, we can take a look at thisContext, which allows reasoning about the current scope "backwards" and "at rest."

Is a query over thisContext always a solution to a missing argument in the message send? Surely not. We are lucky enough to benefit from some inherent properties of the Morphic design:

1. Single UI process
2. Little use of deferred UI messages; no use of other async UI request via promises etc.
3. Clear user model through morph composition; whole-parts hierarchy visible in pixels

To sum up my intentions behind Morphic-mt.1761, I looked for a way to implement the role of a "requestor" for the "Reuse windows" preference as concise (!) as possible -- at a single place to reduce the cost of future maintenance. After pondering the pros and cons of "global state", "dynamic forward scope", and "resting backward scope" in Morphic, I decided to apply meta programming to look into thisContext to figure out the current "requestor." Given that the solution is at a single place (!), I think that future adjustments will remain manageable.

If - at some point - Morphic's properties would change and hence not allow the current approach, I would keep looking for a way to clearly implement the role of a "requestor." At worst, the "reuse windows" preference might just not be supported under some circumstances. I would never risk hacking the entire system just for some edge case.

Yet, I stay confident. We will find a way. If it would mean to re-design #openInWorld: to always supply a requestor, we should do it, maybe by adding #openInWorld:from:. And clearly mark the missing requestor as "discouraged" because you would loose certain effects (or preferences). Well, this takes time and patience.

Until then, let's just keep this little piece of meta-programming to look up thisContext in SystemWindow >> #anyOpenWindowLikeMeIn:. Maybe add an extra flag to emphasize it a little bit more. To be able to quickly find it once we have a better solution for it.


TL;DR: For the Squeak system, dynamic scope is only a little bit better than relying on global state. At best, all required information are in the message arguments. The next better thing is receiver state (i.e. instVars). After that, it gets tricky. Remember that singletons (e.g. SystemWindow class >> #topWindow) are global state, too.

Happy squeaking! :-)

Am 01.05.2021 19:10:25 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
Hi Marcel,

thanks for your review, I will notify you if I identify any regressions. :-)

But your metaprogramming approach from Morphic-mt.1761 ... To be honest, I cannot really say that I would like it. Such magic can be very hard to debug, explore, extend, etc. IMHO, in Smalltalk, we can do better. :-) Instead, I believe an explicit method that to turn off #reuseWindows temporarily would be a better option. If #browseMethodFull is used too frequently, maybe we could wrap it into a new selector #browseMethodFullNew?

Looking at the current solution, I would even prefer a (kind of global) variable that temporarily holds a Model/Browser instance that does not want to reused - for instance:
----- Method: SystemWindow>>anyOpenWindowLikeMeIn: (in category 'open/close') -----
anyOpenWindowLikeMeIn: aPasteUpMorph
| requestor |
self class reuseWindows ifFalse: [ ^ Array empty ].

requestor := Model currentRequestorNotToBeReused.

Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Taeumel, Marcel
Gesendet: Montag, 26. April 2021 10:03:32
An: squeak-dev
Betreff: Re: [squeak-dev] The Trunk: Morphic-eem.1719.mcz
Hi Christoph,

I somewhat integrated your proposed changes into Trunk. :-)

Browser >> #representsSameBrowseeAs:
SearchBar >> #smartSearch:in:
SystemWindow >> #anyOpenWindowLikeMeIn:

Note that a tool's buttons can typically be used to duplicate the tool if desired. Thus, it is not an option to just remove the "hierarchy" button, for example, if it interferes with the "re-use windows" preference -- which you proposed. Also, by just hacking into #browseMethodFull for Browser, you omit all the other possible paths that might interfere with that preference. Well, preference settings might have other side effects, yet, if one would want to do that (e.g., in tests), you must put it into the ensure context:

^ [systemWindow reuseWindows: false. super browseMethodFull]
ensure: [systemWindow reuseWindows: previous]

Anyway -- instead -- I added the constraint that the requesting window should not be considered as a re-use candidate. With our current tool architecture, I had to resort to meta programming (i.e. context checking). I hope that you won't notice any performance glitches.

(Took be about 1.5 hours.)

Am 25.04.2021 22:36:47 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
Hi all,

please find the attached changeset which fixes the regression of the smart search bar not honoring the #reuseWindows preference. Also, I added a proper implementation of #reuseWindows for browsers. Please review and/or merge! :-)


PS: What is packages at lists.squeakfoundation.org and why is it being cc'ed in this conversation?
Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Chris Muller <asqueaker at gmail.com>
Gesendet: Freitag, 5. Februar 2021 22:19:13
An: The general-purpose Squeak developers list
Cc: packages at lists.squeakfoundation.org
Betreff: Re: [squeak-dev] The Trunk: Morphic-eem.1719.mcz
Hi Christoph!

I thought this feature seemed reminiscent of Reuse Windows as well.  The method to hook in each Model subclass (as needed) is #representsSameBrowseeAs:.  Looking at that, you can see that simply making your code pane temporarily dirty, an additional window will be spawned.  I mention that because Reuse Windows is fantastic and I hate to see your experience with it ruined over something so trivial.  :)

You do also have the green duplicate halo.  People are happy to use "non-standard" UI features in other IDE's, but there seems to be an aversion to people using halos in Squeak.  I could be wrong about that, but I find the duplicate halo useful quite often.

 - Chris

On Thu, Feb 4, 2021 at 7:14 PM Thiede, Christoph <Christoph.Thiede at student.hpi.uni-potsdam.de [mailto:Christoph.Thiede at student.hpi.uni-potsdam.de]> wrote:

Hi Eliot,

could you please honor the "SystemWindow reuseWindows" here? I have turned that preference off in my image because I actually use to accept a class name multiple times in the search bar in order to open multiple windows - for instance, to view different protocols of the same class side-by-side. It would be great if this would work soon again ... :-)

Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org [mailto:squeak-dev-bounces at lists.squeakfoundation.org]> im Auftrag von commits at source.squeak.org [mailto:commits at source.squeak.org] <commits at source.squeak.org [mailto:commits at source.squeak.org]>
Gesendet: Donnerstag, 4. Februar 2021 03:38:15
An: squeak-dev at lists.squeakfoundation.org [mailto:squeak-dev at lists.squeakfoundation.org]; packages at lists.squeakfoundation.org [mailto:packages at lists.squeakfoundation.org]
Betreff: [squeak-dev] The Trunk: Morphic-eem.1719.mcz
Eliot Miranda uploaded a new version of Morphic to project The Trunk:
http://source.squeak.org/trunk/Morphic-eem.1719.mcz [http://source.squeak.org/trunk/Morphic-eem.1719.mcz]

==================== Summary ====================

Name: Morphic-eem.1719
Author: eem
Time: 3 February 2021, 6:38:11.11355 pm
UUID: ffb981b1-7c53-4fbe-b6f4-4c8f27c79f5a
Ancestors: Morphic-mt.1718

Make SearchBar>>#smartSearch:in: search existing browsers for a class name being searched for, bringing the first such browser to the front and selecting the class.  This allows one to find classes in browsers either when one has very many, or when one is using multi-window browsers containing many many classes.

=============== Diff against Morphic-mt.1718 ===============

Item was added:
+ ----- Method: Browser>>displayClass: (in category '*Morphic-Menus-DockingBar-accessing') -----
+ displayClass: aClass
+        "Assuming the receiver has answered true to isDisplayingClass:, come to the front and select the given class."
+        | index |
+        index := self multiWindowIndexForClassName: aClass.
+        index  ~= 0 ifTrue:
+                [multiWindowState selectWindowIndex: index].
+        self selectClass: aClass!

Item was added:
+ ----- Method: Browser>>isDisplayingClass: (in category '*Morphic-Menus-DockingBar-accessing') -----
+ isDisplayingClass: aClass
+        | className |
+        className := aClass name.
+        (self multiWindowIndexForClassName: className) ~= 0 ifTrue: [^true].
+        ^selectedClassName = className!

Item was added:
+ ----- Method: Browser>>multiWindowIndexForClassName: (in category '*Morphic-Menus-DockingBar-accessing') -----
+ multiWindowIndexForClassName: className
+        "Answer the index of a browser displaying className in multiWindowState, if any.
+         Otherwise answer zero."
+        multiWindowState ifNil: [^0].
+        multiWindowState models withIndexDo:
+                [:browser :index|
+                browser selectedClassName = className ifTrue: [^index]].
+        ^0!

Item was changed:
  ----- Method: SearchBar>>smartSearch:in: (in category 'searching') -----
  smartSearch: text in: morph
         "Take the user input and perform an appropriate search"
         | input newContents |
         self removeResultsWidget.
         input := text asString ifEmpty:[^self].
         self class useSmartSearch ifFalse: [^ ToolSet default browseMessageNames: input].
+        (Symbol findInterned: input) ifNotNil:
+                [:symbol| input := symbol].
         "If it is a global or a full class name, browse that class."
+        (Smalltalk bindingOf: input) ifNotNil:
+                [:assoc| | class |
+                class := (assoc value isBehavior ifTrue:[assoc value] ifFalse:[assoc value class]) theNonMetaClass.
+                Project current world submorphs do:
+                        [:windowMorph|
+                         (windowMorph isSystemWindow
+                          and: [(windowMorph model isKindOf: Browser)
+                          and: [windowMorph model isDisplayingClass: class]]) ifTrue:
+                                [windowMorph beKeyWindow.
+                                 ^windowMorph model displayClass: class]].
+                ^ToolSet browse: class selector: nil].
-        (Smalltalk bindingOf: input) ifNotNil:[:assoc| | global |
-                global := assoc value.
-                ^ToolSet browse: (global isBehavior ifTrue:[global] ifFalse:[global class]) selector: nil].
         "If it is a symbol and there are implementors of it, browse those implementors."
         Symbol hasInterned: input ifTrue: [:selector |
                 (SystemNavigation new allImplementorsOf: selector) ifNotEmpty:[:list|
                         ^SystemNavigation new
                                 browseMessageList: list
                                 name: 'Implementors of ' , input]].
         "If it starts uppercase, browse classes if any. Otherwise, just search for messages."
+        input first isUppercase ifTrue:
+                [(UIManager default classFromPattern: input withCaption: '')
+                        ifNotNil:[:aClass| ^ToolSet browse: aClass selector: nil].
+                newContents := input, ' -- not found.'.
+                self searchTerm: newContents.
+                self selection: (input size+1 to: newContents size).
+                self currentHand newKeyboardFocus: morph textMorph.
+                ^ self].
+        "Default to browse message names..."
+        ToolSet default browseMessageNames: input!
-        input first isUppercase
-                ifTrue: [
-                        (UIManager default classFromPattern: input withCaption: '')
-                                ifNotNil:[:aClass| ^ToolSet browse: aClass selector: nil]
-                                ifNil: [
-                                        newContents := input, ' -- not found.'.
-                                        self searchTerm: newContents.
-                                        self selection: (input size+1 to: newContents size).
-                                        self currentHand newKeyboardFocus: morph textMorph.
-                                        ^ self]]
-                ifFalse: [
-                        ToolSet default browseMessageNames: input].!

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210503/4865eb06/attachment.html>

More information about the Squeak-dev mailing list