[squeak-dev] The Trunk: Tools-eem.1104.mcz

christoph.thiede at student.hpi.uni-potsdam.de christoph.thiede at student.hpi.uni-potsdam.de
Fri Jan 14 16:02:26 UTC 2022


Hi Marcel,

argh, a few minutes too late, but here is an alternative proposal that comes without the overhead of a new exception class and extra state that needs to be maintained. I would also call this approach more user-friendly because you do not see a single debugger without requesting it. :-)

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

Change Set:        menuBuilderFailed
Date:            14 January 2022
Author:            Christoph Thiede

This change set increases the discoverability of errors that have occured inside a menu builder. Instead of aborting the menu construction and logging the incident to the Transcript only, a failure item is added to the menu. The user can click on this item to debug the underlying error.

=============== Diff ===============

Model>>buildMenu:withBuilders:shifted: {*Tools-pluggable menus} · ct 1/14/2022 16:54 (changed)
buildMenu: aMenu withBuilders: builders shifted: aBoolean
    " We let every builder modify the menu. Skip erroneous builders silently.
    The builder should indicate whether to abort by returning nil."
    | menu |
    menu := aMenu.
-     builders do: [:builder |
-         menu := [self perform: builder method selector withEnoughArguments: { menu . aBoolean }]
-             ifError: [:msg | Transcript showln: 'Menu builder failed: ', msg. menu].
-         menu ifNil: [^ aMenu]].
-     ^ menu
+     builders do: [:builder | | buildBlock |
+         buildBlock := [menu := self perform: builder method selector withEnoughArguments: {menu . aBoolean}].
+         buildBlock
+             on: Error, Warning, Halt do: [:ex |
+                 menu add: ('<menu builder failed: {1}>' translated format: {ex}) action: buildBlock]].
+     ^ menu

Thank you for the discussion, and sorry for not better coordinating this issue. :-)

Best,
Christoph

---
Sent from Squeak Inbox Talk

On 2022-01-14T16:50:51+01:00, marcel.taeumel at hpi.de wrote:

> Hi Christoph --
> 
> Done. See Tools-mt.1105.
> 
> Best,
> Marcel
> Am 14.01.2022 16:20:46 schrieb Marcel Taeumel <marcel.taeumel at hpi.de>:
> Hi Christoph --
> 
> > This is an anti-pattern in my opinion and should be revised.
> > Can we please remove that #ifError: from Model>>#buildMenu:withBuilders:shifted: again? :-)
> 
> Not every user is a developer. Having menus that do not throw errors is a good thing. Hmm... a specialized Warning about the broken menu seems nice =)
> 
> > Or maybe a separate "release mode" preference.
> 
> Seriously ... we do need to talk about "modes" at some point ... Stop it. xD
> 
> Best,
> Marcel
> Am 14.01.2022 16:16:54 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:
> Hi Marcel,
> 
> thank you for the clarification (and thank you Eliot for fixing this bug)!
> 
> So the menu building mechanism is actually ignoring errors from single menu builders? This is an anti-pattern in my opinion and should be revised. Silently and unconditionally dropping errors instead of reporting them to the user/developer (and writing them to the Transcript does *not* count as reporting IMHO because I only open the Transcript when I *expect* to see particular messages in it) looks like a bad practice to me that makes debugging and testing (both manually and automated) unnecessarily hard and non-interactive. It's not the Smalltalk way of facilitating short feedback cycles and direct manipulation. If an error occurs, I want to know about it and fix it immediately (which usually only takes a few seconds or minutes). If you really see some situations where fixing the error is so expensive (I would be curious to hear them), I would prefer talking about an opt-in mechanism similar to Warning class >> #suppressed:. Or maybe a separate "release mode" preference. But please let's keep our primary focus in the main image on developers that are curious to notice and report/resolve every bug. :-)
> 
> Can we please remove that #ifError: from Model>>#buildMenu:withBuilders:shifted: again? :-)
> 
> Best,
> Christoph
> 
> ---
> Sent from Squeak Inbox Talk [https://github.com/hpi-swa-lab/squeak-inbox-talk]
> 
> On 2022-01-14T10:08:23+01:00, marcel.taeumel at hpi.de wrote:
> 
> > Hi Christoph --
> >
> > You are showing the "more..." menu. The initial menu is truncated because of that bug in #isBreakOnEntry.
> >
> >
> >
> > After Eliot's patch, it looks as expected:
> >
> >
> >
> > Best,
> > Marcel
> > Am 13.01.2022 19:26:32 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:
> > I have a message trace here with no message selected, but the menu can be opened anyway without an error:
> >
> >
> >  
> >
> >
> > Under which circumstances did this error occur? :-)
> >
> > Best,
> > Christoph
> > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von commits at source.squeak.org <commits at source.squeak.org>
> > Gesendet: Donnerstag, 13. Januar 2022 19:21:13
> > An: squeak-dev at lists.squeakfoundation.org; packages at lists.squeakfoundation.org
> > Betreff: [squeak-dev] The Trunk: Tools-eem.1104.mcz
> >  
> > Eliot Miranda uploaded a new version of Tools to project The Trunk:
> > http://source.squeak.org/trunk/Tools-eem.1104.mcz [http://source.squeak.org/trunk/Tools-eem.1104.mcz]
> >
> > ==================== Summary ====================
> >
> > Name: Tools-eem.1104
> > Author: eem
> > Time: 13 January 2022, 10:21:10.513361 am
> > UUID: 1d051d8d-473d-43ee-9628-d33992a5849f
> > Ancestors: Tools-mt.1103
> >
> > Fix MNU in menu building for MessageTrace with no method selected
> >
> > =============== Diff against Tools-mt.1103 ===============
> >
> > Item was changed:
> >   ----- Method: CodeHolder>>isBreakOnEntry (in category 'breakpoints') -----
> >   isBreakOnEntry
> >  
> > +        ^self selectedClassOrMetaClass
> > +                ifNil: [false]
> > +                ifNotNil:
> > +                        [:class|
> > +                         (class
> > +                                        compiledMethodAt: self selectedMessageName
> > +                                        ifAbsent: nil)
> > +                                ifNil: [false]
> > +                                ifNotNil: [:method| method hasBreakpoint]]!
> > -        | selectedMethod |
> > -        selectedMethod := self selectedClassOrMetaClass
> > -                compiledMethodAt: self selectedMessageName
> > -                ifAbsent: [^ false].
> > -        ^ selectedMethod hasBreakpoint!
> >
> >
> > -------------- next part --------------
> > An HTML attachment was scrubbed...
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0001.html>
> > -------------- next part --------------
> > A non-text attachment was scrubbed...
> > Name: image.png
> > Type: image/png
> > Size: 22635 bytes
> > Desc: not available
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0005.png>
> > -------------- next part --------------
> > A non-text attachment was scrubbed...
> > Name: pastedImage.png
> > Type: image/png
> > Size: 4074 bytes
> > Desc: not available
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0006.png>
> > -------------- next part --------------
> > A non-text attachment was scrubbed...
> > Name: pastedImage.png
> > Type: image/png
> > Size: 18275 bytes
> > Desc: not available
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0007.png>
> > -------------- next part --------------
> > A non-text attachment was scrubbed...
> > Name: pastedImage.png
> > Type: image/png
> > Size: 15326 bytes
> > Desc: not available
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0008.png>
> > -------------- next part --------------
> > A non-text attachment was scrubbed...
> > Name: image.png
> > Type: image/png
> > Size: 78379 bytes
> > Desc: not available
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0009.png>
> >
> >
> -------------- next part --------------
> An HTML attachment was scrubbed...
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/c96d0a4c/attachment.html>
> 
> 
["menuBuilderFailed.1.cs"]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a1e0fb67/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: menuBuilderFailed.1.cs
Type: application/octet-stream
Size: 1072 bytes
Desc: not available
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a1e0fb67/attachment.obj>


More information about the Squeak-dev mailing list