[squeak-dev] bad MessageTrace regression (was: The Trunk: Morphic-mt.1652.mcz)

christoph.thiede at student.hpi.uni-potsdam.de christoph.thiede at student.hpi.uni-potsdam.de
Mon Feb 28 17:58:49 UTC 2022


Hi Marcel, hi all,

replying to your critique in "The Inbox: Tools-ct.1136.mcz" in this thread:

I made another attempt to assemble all our requirements for spawning vs tracing in the MessageTrace. The following list of requirements appears to have been agreed by Chris, Jakob, and me:

- Menu and keyboard shortcut do the same thing
- Senders in the message list: add to trace
- Implementors in the message list: add to trace
- Senders in the code editor: new window
- Implementors in the code editor: add to trace
- Senders/Implementors Buttons: new window

(Hm, that feels a lot like a logic puzzle. :D)

I have rechecked and Tools-ct.1136 seems to satisfy each of these requirements. :-)

Regarding your critique:

> Let's keep menu click and button-bar click consistent. 

My argument for just *not* connecting them to the same behavior is this: The button bar is common to all tools (browser, debugger, ...) and users can expect that it will always open a new tool. On the other hand, the menu is a trace-specific feature, you are triggering it through the core view representing the trace (the message list), so it makes a lot of sense to me display the result of the menu operations right in the trace.

> We have #browseSendersOfMessages ... button bar + popup menu #browseSenders ... text/list keyboard shortcuts
> 
> It's confusing enough that there are two paths. What I would agree with is when we spawn a new window for the #browseSendersOfMessages, which you can easily distinguish via requestor being #modelMenu.

To be honest, that sounds more like an implementation-first than like a UX-first argument to me. :-) Maybe the mapping from the different interactions to the protocol does not match the tool in question very well. IMO (as Jakob mentioned earlier) a menu item and its shortcut should trigger the same code path. Ideally, the requestor in #browseAllCallsOn:requestor:/#browseAllImplementorsOf:requestor: should identify the widget where the event was received, not the kind of user interaction (mouse vs. keyboard).

tl;dr: I still like my proposal in Tools-ct.1136, and if you are expecting different semantics of the tool, please let's work together on revising the list of requirements from above. In this case I am convinced that UX should be valued higher than the aesthetics of the implementation. :-)

Best,
Christoph

---
Sent from Squeak Inbox Talk

On 2022-02-28T09:32:12+01:00, marcel.taeumel at hpi.de wrote:

> Oh, my. -1 :-)
> 
> Let's keep menu click and button-bar click consistent. 
> 
> We have
> #browseSendersOfMessages ... button bar + popup menu
> #browseSenders ... text/list keyboard shortcuts
> 
> It's confusing enough that there are two paths. What I would agree with is when we spawn a new window for the #browseSendersOfMessages, which you can easily distinguish via requestor being #modelMenu.
> 
> Best,
> Marcel
> Am 25.02.2022 16:51:30 schrieb commits at source.squeak.org <commits at source.squeak.org>:
> A new version of Tools was added to project The Inbox:
> http://source.squeak.org/inbox/Tools-ct.1136.mcz
> 
> ==================== Summary ====================
> 
> Name: Tools-ct.1136
> Author: ct
> Time: 25 February 2022, 4:51:14.742129 pm
> UUID: d3396d11-145e-294d-b51c-805c6b3b64ab
> Ancestors: Tools-mt.1135
> 
> Revises senders/implementors in message traces so that:
> 
> * using the buttons always spawns a new window
> * you can press shift to invert whether the new messages will be appended to the existing window or opened in a new window
> 
> See: http://lists.squeakfoundation.org/pipermail/squeak-dev/2022-February/218946.html
> 
> =============== Diff against Tools-mt.1135 ===============
> 
> Item was changed:
> ----- Method: MessageTrace>>browseAllCallsOn:requestor: (in category 'actions') -----
> browseAllCallsOn: selectorSymbol requestor: anObject
> "Overwritten to modify the trace if the request origins from a model-menu command such as the message-list menu (shortcut)."
> 
> + (((Preferences traceMessages and: [anObject ~= #button] and: [selectorSymbol = self selectedMessageName]) xor: Sensor shiftPressed)
> + and: [self hasUnacceptedEdits not])
> - (selectorSymbol = self selectedMessageName
> - and: [ Preferences traceMessages ] and: [ self hasUnacceptedEdits not ])
> ifTrue: [ self addParentMethodsSending: selectorSymbol ]
> ifFalse: [ super browseAllCallsOn: selectorSymbol requestor: anObject ].!
> 
> Item was changed:
> ----- Method: MessageTrace>>browseAllImplementorsOf:requestor: (in category 'actions') -----
> browseAllImplementorsOf: selectorSymbol requestor: anObject
> "Overwritten to modify the trace if the request origins from a model-menu command such as the message-list menu (shortcut)."
> 
> | selectorToBrowse |
> selectorToBrowse := self selection
> ifNil: [ selectorSymbol ]
> ifNotNil: [ self getImplementorNamed: selectorSymbol asSymbol "since we can get passed literals"].
> + (((Preferences traceMessages and: [anObject ~= #button]) xor: Sensor shiftPressed) and: [self hasUnacceptedEdits not])
> - (Preferences traceMessages and: [ self hasUnacceptedEdits not ])
> ifTrue: [ self addChildMethodsNamed: selectorToBrowse ]
> ifFalse: [ super browseAllImplementorsOf: selectorToBrowse requestor: anObject ].!
> 
> Item was added:
> + ----- Method: MessageTrace>>browseMessagesFromButton (in category 'controls') -----
> + browseMessagesFromButton
> +
> + self getSelectorAndSendQuery: #browseAllImplementorsOf:requestor: to: self with: #(button).!
> 
> Item was added:
> + ----- Method: MessageTrace>>browseSendersOfMessagesFromButton (in category 'controls') -----
> + browseSendersOfMessagesFromButton
> +
> + self getSelectorAndSendQuery: #browseAllCallsOn:requestor: to: self with: #(button).!
> 
> Item was added:
> + ----- Method: MessageTrace>>optionalButtonPairs (in category 'controls') -----
> + optionalButtonPairs
> +
> + ^ super optionalButtonPairs collect: [:spec |
> + spec second
> + caseOf:
> + {[#browseSendersOfMessages] -> [spec copy
> + at: 2 put: #browseSendersOfMessagesFromButton;
> + yourself].
> + [#browseMessages] -> [spec copy
> + at: 2 put: #browseMessagesFromButton;
> + yourself]}
> + otherwise: [spec]]!
> 
> 
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220228/9daa5119/attachment.html>
> 
> 

On 2022-02-28T18:32:52+01:00, christoph.thiede at student.hpi.uni-potsdam.de wrote:

> Hi Chris,
> 
> > > > Just to be clear, nothing is different than what it was before it got
> > > broken in 5.3.
> > >
> > > In Squeak 5.2 (and also 5.3), I could do this:
> > >
> > > 1. Type foo
> > > 2. Browse iMplementors
> > > 3. Select the first method
> > > 4. Press the "senders" button
> > > 5. And a second message trace on the senders of #foo opens.
> > >
> > 
> > "A second message trace" as in a second window?  That's not the behavior I
> > see.  That should only happen if traceMessages was not enabled, or if the
> > keyboard focus was somewhere besides the upper pane.
> 
> Please see the attached screencast which I recorded in a fresh 5.2 image. :-)
> 
> > So, you want to be able to only forward trace (e.g., add indended
> > implementors) but replace ability to back trace (add outdented senders)
> > with only starting yet a new trace?  If so, I'm puzzled why you would want
> > that.  Back tracing is useful but there'd be no way to do it if it always
> > opened a new window.
> 
> It just depends on the use case. In many situations, backtracing is helpful so I'm fine with reusing the existing window, but in other situations, my conceptual model is to start a new session. I want to explore two different routes through a program/through the system that should not intermix each other. In this case, I need a new and clear and empty window. :-)
> 
> > I'm not sure if you feel your change is an optimization of that use-case, or a different one..
> 
> It is a change to make sure that the old behavior of breaking out of the trace is still possible. :-)
> 
> PS: Yes, that scratch pad feature is indeed very nice. Looking forward to trying it out when attached to the mouse cursor. :-)
> 
> Best,
> Christoph
> 
> ---
> Sent from Squeak Inbox Talk
> 
> On 2022-02-26T18:13:10-06:00, asqueaker at gmail.com wrote:
> 
> > Hi,
> > 
> > sorry for the delay. Exams and things and other things, you know. :-)
> > >
> > 
> > :-)
> > 
> > 
> > > > Just to be clear, nothing is different than what it was before it got
> > > broken in 5.3.
> > >
> > > In Squeak 5.2 (and also 5.3), I could do this:
> > >
> > > 1. Type foo
> > > 2. Browse iMplementors
> > > 3. Select the first method
> > > 4. Press the "senders" button
> > > 5. And a second message trace on the senders of #foo opens.
> > >
> > 
> > "A second message trace" as in a second window?  That's not the behavior I
> > see.  That should only happen if traceMessages was not enabled, or if the
> > keyboard focus was somewhere besides the upper pane.
> > 
> > 
> > > This no longer works since the recent changes.
> > >
> > 
> > So, you want to be able to only forward trace (e.g., add indended
> > implementors) but replace ability to back trace (add outdented senders)
> > with only starting yet a new trace?  If so, I'm puzzled why you would want
> > that.  Back tracing is useful but there'd be no way to do it if it always
> > opened a new window.
> > 
> > 
> > > > Cmd+c+0+v+m. :)
> > >
> > > I know and sometimes do that. But you won't be able to convince me that
> > > this could be as fast as a single button. Also, the search bar/scratch pad
> > > do not provide support for stacking these conceptual breaks,
> > 
> > sometimes I am parking another doit script there ...
> > >
> > 
> > Actually, that's precisely why the "scratch pad" was hacked in -- it's
> > multi-line.  There's your "stack".  :)  Rudimentary, I know, but it works.
> > Undo also works.
> > 
> > Side note:  the scratch pad in the upper right is a temporary hack -- I
> > always wanted Cmd+0 to open a BalloonMorph at the hand with the cursor
> > ready to type, so the user's eyes don't always have to traverse back and
> > forth to the upper right and back..
> > 
> > > But, I don't care about (b). Traces are normally built via the upper and
> > > lower pane functions, not the middle button row. So, ONLY for the button
> > > row between the panes, it seems fine for it to unconditionally open a new
> > > window. If it's possible to twiggle it dynamically with a modifier key,
> > > that'd be preferable but, even if not, it seems reasonable to open a new
> > > window. But we need to maintain the legacy behaviors for the upper and
> > > lower panes, please.
> > >
> > > Alright, please see Tools-ct.1136. I still do not consider this a perfect
> > > and maximally intuitive solution, but apparently we are a few users of this
> > > tool only, so if we can find a heuristic that satisfies everyone, this
> > > might not be a big problem. :-)
> > >
> > 
> > The heuristic is for building Traces.  I'm not sure if you feel your change
> > is an optimization of that use-case, or a different one..
> > 
> > Best,
> >   Chris
> > -------------- next part --------------
> > An HTML attachment was scrubbed...
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220226/e58c5d33/attachment.html>
> > 
> > 
> ["messagetrace-5.2.gif"]
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220228/2a1a5997/attachment-0001.html>
> -------------- next part --------------
> A non-text attachment was scrubbed...
> Name: messagetrace-5.2.gif
> Type: image/gif
> Size: 297959 bytes
> Desc: not available
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220228/2a1a5997/attachment-0001.gif>
> 
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220228/8fd2b57f/attachment.html>


More information about the Squeak-dev mailing list