-1 :-)

I like the idea of improving the code here. However, I disagree with adding more magic into Morph >> #buildDebugMenu:. 

The concept of "debug action" doesn't seem to be a single, tangible concept that morphs have in common. For one morph, "action" could mean #step; for another, it could be #mouseDown:. Maybe both should be made available in a debug menu under certain conditions? Only the receiver can know for sure.

This is just my opinion. I know that, in a system like Squeak, it is sometimes challenging to decide between such meta programming (i.e., #respondsTo:) and good old subclassing/overrides. :-) In this case, I prefer the latter. It seems to be easier to debug and maintain.

Best,
Marcel

P.S.: In buttons and menus, #browseImplementorsOfActions seems unnecessary vague because we know the exact receiver of the action and can thus point to a specific implementation.

Am 01.10.2019 15:08:10 schrieb commits at source.squeak.org <commits at source.squeak.org>:
A new version of Morphic was added to project The Inbox:
http://source.squeak.org/inbox/Morphic-ct.1554.mcz

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

Name: Morphic-ct.1554
Author: ct
Time: 1 October 2019, 3:07:47.979339 pm
UUID: 39fdea5d-c5bf-f04f-9d4b-5cb0e64a1af0
Ancestors: Morphic-ul.1552

Refactor & extend "browse implementors" entry of Morphic debug menu

- Introduce common selector #browseImplementorsOfAction and deprecate PluggableButtonMorph>>#browseAllImplementorsOfActionSelector + MenuItemMorph>>#browseAllImplementorsOfRealSelector
- Use #respondsTo: check instead of overriding #buildDebugMenu: multiple times
- Using the same pattern, introduce #debugAction for PluggableButtonMorph and MenuItemMorph, and add an item for it into the debug menu

=============== Diff against Morphic-ul.1552 ===============

Item was changed:
+ ----- Method: MenuItemMorph>>browseAllImplementorsOfRealSelector (in category 'debug and other') -----
- ----- Method: MenuItemMorph>>browseAllImplementorsOfRealSelector (in category 'browse') -----
browseAllImplementorsOfRealSelector
+ self deprecated: 'Use #browseImplementorsOfAction'.
+ ^ self browseImplementorsOfAction!
- SystemNavigation default browseAllImplementorsOf: self realSelector localTo: target class!

Item was added:
+ ----- Method: MenuItemMorph>>browseImplementorsOfAction (in category 'debug and other') -----
+ browseImplementorsOfAction
+
+ SystemNavigation default browseAllImplementorsOf: self realSelector localTo: target class!

Item was removed:
- ----- Method: MenuItemMorph>>buildDebugMenu: (in category 'browse') -----
- buildDebugMenu: aHandMorph
- | aMenu |
- aMenu := super buildDebugMenu: aHandMorph.
- aMenu addLine.
- aMenu add: 'implementors of' translated target: self action: #browseAllImplementorsOfRealSelector.
- ^ aMenu!

Item was added:
+ ----- Method: MenuItemMorph>>debugAction (in category 'debug and other') -----
+ debugAction
+
+ ToolSet debugMethod: (self class lookupSelector: #invoke) forReceiver: self inContext: nil.!

Item was added:
+ ----- Method: MenuItemMorph>>invoke (in category 'debug and other') -----
+ invoke
+
+ ^ self invokeWithEvent: nil!

Item was changed:
+ ----- Method: MenuItemMorph>>realSelector (in category 'debug and other') -----
- ----- Method: MenuItemMorph>>realSelector (in category 'browse') -----
realSelector
selector == #perform:orSendTo: ifTrue: [^arguments first].
^selector!

Item was changed:
----- Method: Morph>>buildDebugMenu: (in category 'debug and other') -----
buildDebugMenu: aHand
"Answer a debugging menu for the receiver. The hand argument is seemingly historical and plays no role presently"

| aMenu aPlayer |
aMenu := MenuMorph new defaultTarget: self.
aMenu addStayUpItem.
(self hasProperty: #errorOnDraw) ifTrue:
[aMenu add: 'start drawing again' translated action: #resumeAfterDrawError.
aMenu addLine].
(self hasProperty: #errorOnStep) ifTrue:
[aMenu add: 'start stepping again' translated action: #resumeAfterStepError.
aMenu addLine].

aMenu add: 'inspect morph' translated action: #inspectInMorphic:.
aMenu add: 'inspect owner chain' translated action: #inspectOwnerChain.
Smalltalk isMorphic ifFalse:
[aMenu add: 'inspect morph (in MVC)' translated action: #inspect].

self isMorphicModel ifTrue:
[aMenu add: 'inspect model' translated target: self model action: #inspect;
add: 'explore model' translated target: self model action: #explore].
(aPlayer := self player) ifNotNil:
[aMenu add: 'inspect player' translated target: aPlayer action: #inspect].

aMenu add: 'explore morph' translated target: self selector: #exploreInMorphic:.

aMenu addLine.
aPlayer ifNotNil:
[ aMenu add: 'viewer for Player' translated target: self player action: #beViewed.
aMenu balloonTextForLastItem: 'Opens a viewer on my Player -- this is the same thing you get if you click on the cyan "View" halo handle' translated ].

aMenu add: 'viewer for Morph' translated target: self action: #viewMorphDirectly.
aMenu balloonTextForLastItem: 'Opens a Viewer on this Morph, rather than on its Player' translated.
aMenu addLine.

aPlayer ifNotNil:
[aPlayer class isUniClass ifTrue: [
aMenu add: 'browse player class' translated target: aPlayer selector: #haveFullProtocolBrowsedShowingSelector: argumentList: #(nil)]].
aMenu add: 'browse morph class' translated target: self selector: #browseHierarchy.
(self isMorphicModel)
ifTrue: [aMenu
add: 'browse model class'
target: self model
selector: #browseHierarchy].
aMenu addLine.

self addViewingItemsTo: aMenu.
aMenu
add: 'make own subclass' translated action: #subclassMorph;
add: 'save morph in file' translated action: #saveOnFile;
addLine;
add: 'call #tempCommand' translated action: #tempCommand;
add: 'define #tempCommand' translated action: #defineTempCommand;
addLine;

add: 'control-menu...' translated target: self selector: #invokeMetaMenu:;
add: 'edit balloon help' translated action: #editBalloonHelpText.
+
+ ((self respondsTo: #browseImplementorsOfAction) or: [self respondsTo: #debugAction]) ifTrue: [
+ aMenu addLine.
+ (self respondsTo: #browseImplementorsOfAction) ifTrue: [
+ aMenu add: 'implementors of' translated target: self action: #browseImplementorsOfAction].
+ (self respondsTo: #debugAction) ifTrue: [
+ aMenu add: 'debug action' translated target: self action: #debugAction]].
+
-
^ aMenu!

Item was changed:
+ ----- Method: PluggableButtonMorph>>browseAllImplementorsOfActionSelector (in category 'debug and other') -----
- ----- Method: PluggableButtonMorph>>browseAllImplementorsOfActionSelector (in category 'browse') -----
browseAllImplementorsOfActionSelector
+ self deprecated: 'Use #browseImplementorsOfAction'.
+ ^ self browseImplementorsOfAction!
- SystemNavigation default browseAllImplementorsOf: actionSelector localTo: model class!

Item was added:
+ ----- Method: PluggableButtonMorph>>browseImplementorsOfAction (in category 'debug and other') -----
+ browseImplementorsOfAction
+
+ SystemNavigation default browseAllImplementorsOf: actionSelector localTo: model class!

Item was removed:
- ----- Method: PluggableButtonMorph>>buildDebugMenu: (in category 'browse') -----
- buildDebugMenu: aHandMorph
- | aMenu |
- aMenu := super buildDebugMenu: aHandMorph.
- aMenu addLine.
- aMenu add: 'implementors of' translated target: self action: #browseAllImplementorsOfActionSelector.
- ^ aMenu!

Item was added:
+ ----- Method: PluggableButtonMorph>>debugAction (in category 'debug and other') -----
+ debugAction
+
+ ToolSet debugMethod: (self class lookupSelector: #performAction) forReceiver: self inContext: nil.!


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20191002/1c9ca3a4/attachment.html>