Smalltalk › Squeak › Squeak - Dev

The Inbox: Tools-ct.924.mcz

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

3 messages Options

commits-2

The Inbox: Tools-ct.924.mcz

A new version of Tools was added to project The Inbox:
http://source.squeak.org/inbox/Tools-ct.924.mcz

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

Name: Tools-ct.924
Author: ct
Time: 7 December 2019, 5:03:02.396431 pm
UUID: 0db76606-63d7-4a4a-94fd-99fe4a088304
Ancestors: Tools-mt.922

Fix a bug ProcessBrowser >> #selectedClass in case of no context selection

=============== Diff against Tools-mt.922 ===============

Item was changed:
----- Method: ProcessBrowser>>selectedClass (in category 'accessing') -----
selectedClass
"Answer the class in which the currently selected context's method was
found."
^ selectedClass
+ ifNil: [
+ selectedContext ifNil: [^ nil].
+ selectedClass := selectedContext receiver
- ifNil: [selectedClass := selectedContext receiver
ifNil: [selectedSelector := selectedContext method selector.
selectedContext method methodClass]
ifNotNil: [selectedContext methodClass]]!

Christoph Thiede

Re: The Inbox: Tools-ct.924.mcz

What I find quite crazy, you can create a failure from this bug by opening a new ProcessBrowser and selecting "browse model class" from its morphic debug menu:

This is because the whole class lookup happens relative to the selection-dependent environment of the model instance.

Is this really what we want? Shouldn't these browse things maybe be implemented rather on models' class side?

And just another, a little bit related question: Are {SystemNavigation>>#browserClass[:], SystemBrowserClass>>#browserShowsPackagePane[:], BrowseTest.original[Hierarchy]BrowserClass} really in use anywhere, or should they be deprecated?

Best,

Christoph

Von: Squeak-dev <[hidden email]> im Auftrag von [hidden email] <[hidden email]>
Gesendet: Samstag, 7. Dezember 2019 17:03:06
An: [hidden email]
Betreff: [squeak-dev] The Inbox: Tools-ct.924.mcz

A new version of Tools was added to project The Inbox:
http://source.squeak.org/inbox/Tools-ct.924.mcz

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

Name: Tools-ct.924
Author: ct
Time: 7 December 2019, 5:03:02.396431 pm
UUID: 0db76606-63d7-4a4a-94fd-99fe4a088304
Ancestors: Tools-mt.922

Fix a bug ProcessBrowser >> #selectedClass in case of no context selection

=============== Diff against Tools-mt.922 ===============

Item was changed:
----- Method: ProcessBrowser>>selectedClass (in category 'accessing') -----
selectedClass
         "Answer the class in which the currently selected context's method was
         found."
         ^ selectedClass
+                ifNil: [
+                        selectedContext ifNil: [^ nil].
+                        selectedClass := selectedContext receiver
-                ifNil: [selectedClass := selectedContext receiver
                                 ifNil: [selectedSelector := selectedContext method selector.
                                            selectedContext method methodClass]
                                 ifNotNil: [selectedContext methodClass]]!

pastedImage.png (102K) Download Attachment

Carpe Squeak!

Jakob Reschke

The environment and systemNavigation of an object (was: The Inbox: Tools-ct.924.mcz)

Hi Christoph, hi all,

Am Sa., 7. Dez. 2019 um 17:21 Uhr schrieb Thiede, Christoph <[hidden email]>:

What I find quite crazy, you can create a failure from this bug by opening a new ProcessBrowser and selecting "browse model class" from its morphic debug menu:

This is because the whole class lookup happens relative to the selection-dependent environment of the model instance.

Is this really what we want? Shouldn't these browse things maybe be implemented rather on models' class side?

And just another, a little bit related question: Are {SystemNavigation>>#browserClass[:], SystemBrowserClass>>#browserShowsPackagePane[:], BrowseTest.original[Hierarchy]BrowserClass} really in use anywhere, or should they be deprecated?

Wow it's already three years ago that I did these environment changes... time flies.

What Christoph fixed is a bug in any case, so that's good for a start.

About the class lookup: I added the environment dispatching for systemNavigation so that tools that show code from another environment (browsers that show classes in a different environment, message sets displaying methods of classes in a different environment etc.) will find the correct class (i. e., look it up in the correct environment). Imagine you would browse the class of the top method on the stack of a process, then you would need this if the method class is in another environment. But for ProcessBrowser browseHierarchy, this is indeed not correct. Since browseHierarchy wants to browse the hierarchy of the receiver class and not the selectedClass, it should also use the environment and systemNavigation of the receiver class not of the selectedClass.

Just thinking loud in the following, so you all have a chance to hook on and make a suggestion.

Object>>environment is the receiver class environment, but Model>>environment is the environment of the selected class (if there is one). Object>>systemNavigation uses that #environment. Changing Object>>systemNavigation to use the environment of the own class (self class environment instead of self environment) would not help because "Smalltalk tools" do need the SystemNavigation for the environment of the selectedClass. Why not move the current implementation of systemNavigation down to CodeHolder: because Inspector and ObjectExplorer are also "Smalltalk tools" in this regard, and they don't inherit from CodeHolder. So without sprinkling overrides of #systemNavigation to multiple subclasses of Model, Model would still have to override systemNavigation as it works today. Since ProcessBrowser inherits from Model, we would end up with the same problem as it currently exists.

This leaves us to change browseHierarchy to use a different systemNavigation. For example, it could use self class systemNavigation, how about that? But it might break browsing the hierarchy of a class in a Browser in a different environment, I did not check.

There is an ambiguity between the #environment of a class and the #environment of a tool/model. Either one must pay more attention to asking the correct object for its environment, or we need to introduce a different selector (and modify lots of sender methods in the tools). On the first: if you want the environment of the class of an object, do ask the class, not the object itself. To uphold the law of Demeter, maybe we still need to introduce a different selector classEnvironment or behaviorEnvironment on Object. But since systemNavigation is somewhat coupled to the environment, we may then need classSystemNavigation as well, And there may be more couplings of this kind. So the effort might be the same as introducing a selector for the environment of the selected class, but it might (?) need less senders to be changed. The alternatives might be selectedClassEnvironment, selectedClassSystemNavigation and so on. Whatever we do, all tools must use the environment of the selected class if they do any navigation from the selection. This does not apply to "browse model class", which does not operate on the selection, but the tool itself.

Kind regards,

Jakob