[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
|