Hi Marcel,<br>
<br>
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. :-)<br>
<br>
<b><font face="Bitmap DejaVu Sans" size="9">=============== Summary ===============</font></b><font face="Bitmap DejaVu Sans" size="9"><br>
<br>
Change Set: menuBuilderFailed<br>
Date: 14 January 2022<br>
Author: Christoph Thiede<br>
<br>
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.<br>
<br>
</font><b><font face="Bitmap DejaVu Sans" size="9">=============== Diff ===============</font></b><font face="Bitmap DejaVu Sans" size="9"><br>
<br>
</font><b><font face="Bitmap DejaVu Sans" size="9">Model>>buildMenu:withBuilders:shifted: {*Tools-pluggable menus} · ct 1/14/2022 16:54 (changed)</font></b><font face="Bitmap DejaVu Sans" size="9"><br>
buildMenu: aMenu withBuilders: builders shifted: aBoolean<br>
" We let every builder modify the menu. Skip erroneous builders silently.<br>
The builder should indicate whether to abort by returning nil."<br>
| menu |<br>
menu := aMenu.<br>
</font><s><font color="#0000FF"><font face="Bitmap DejaVu Sans" size="9">- builders do: [:builder |<br>
- menu := [self perform: builder method selector withEnoughArguments: { menu . aBoolean }]<br>
- ifError: [:msg | Transcript showln: 'Menu builder failed: ', msg. menu].<br>
- menu ifNil: [^ aMenu]].<br>
- ^ menu<br>
</font></font></s><font color="#FF0000"><font face="Bitmap DejaVu Sans" size="9">+ builders do: [:builder | | buildBlock |<br>
+ buildBlock := [menu := self perform: builder method selector withEnoughArguments: {menu . aBoolean}].<br>
+ buildBlock<br>
+ on: Error, Warning, Halt do: [:ex |<br>
+ menu add: ('<menu builder failed: {1}>' translated format: {ex}) action: buildBlock]].<br>
+ ^ menu<br>
<br>
</font></font><font face="Bitmap DejaVu Sans" size="9">Thank you for the discussion, and sorry for not better coordinating this issue. :-)</font><font color="#FF0000"><font face="Bitmap DejaVu Sans" size="9"><br>
</font></font><font face="Bitmap DejaVu Sans" size="9"><br>
Best,<br>
Christoph</font><br>
<br>
<font color="#808080">---<br>
</font><font color="#808080"><i>Sent from </i></font><font color="#808080"><i><a href="https://github.com/hpi-swa-lab/squeak-inbox-talk"><u><font color="#808080">Squeak Inbox Talk</font></u></a></i></font><br>
<br>
On 2022-01-14T16:50:51+01:00, marcel.taeumel@hpi.de wrote:<br>
<br>
> Hi Christoph --<br>
> <br>
> Done. See Tools-mt.1105.<br>
> <br>
> Best,<br>
> Marcel<br>
> Am 14.01.2022 16:20:46 schrieb Marcel Taeumel <marcel.taeumel at hpi.de>:<br>
> Hi Christoph --<br>
> <br>
> > This is an anti-pattern in my opinion and should be revised.<br>
> > Can we please remove that #ifError: from Model>>#buildMenu:withBuilders:shifted: again? :-)<br>
> <br>
> 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 =)<br>
> <br>
> > Or maybe a separate "release mode" preference.<br>
> <br>
> Seriously ... we do need to talk about "modes" at some point ... Stop it. xD<br>
> <br>
> Best,<br>
> Marcel<br>
> Am 14.01.2022 16:16:54 schrieb christoph.thiede at student.hpi.uni-potsdam.de <christoph.thiede at student.hpi.uni-potsdam.de>:<br>
> Hi Marcel,<br>
> <br>
> thank you for the clarification (and thank you Eliot for fixing this bug)!<br>
> <br>
> 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. :-)<br>
> <br>
> Can we please remove that #ifError: from Model>>#buildMenu:withBuilders:shifted: again? :-)<br>
> <br>
> Best,<br>
> Christoph<br>
> <br>
> ---<br>
> Sent from Squeak Inbox Talk [https://github.com/hpi-swa-lab/squeak-inbox-talk]<br>
> <br>
> On 2022-01-14T10:08:23+01:00, marcel.taeumel at hpi.de wrote:<br>
> <br>
> > Hi Christoph --<br>
> ><br>
> > You are showing the "more..." menu. The initial menu is truncated because of that bug in #isBreakOnEntry.<br>
> ><br>
> ><br>
> ><br>
> > After Eliot's patch, it looks as expected:<br>
> ><br>
> ><br>
> ><br>
> > Best,<br>
> > Marcel<br>
> > Am 13.01.2022 19:26:32 schrieb Thiede, Christoph <christoph.thiede at student.hpi.uni-potsdam.de>:<br>
> > I have a message trace here with no message selected, but the menu can be opened anyway without an error:<br>
> ><br>
> ><br>
> > <br>
> ><br>
> ><br>
> > Under which circumstances did this error occur? :-)<br>
> ><br>
> > Best,<br>
> > Christoph<br>
> > Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von commits at source.squeak.org <commits at source.squeak.org><br>
> > Gesendet: Donnerstag, 13. Januar 2022 19:21:13<br>
> > An: squeak-dev at lists.squeakfoundation.org; packages at lists.squeakfoundation.org<br>
> > Betreff: [squeak-dev] The Trunk: Tools-eem.1104.mcz<br>
> > <br>
> > Eliot Miranda uploaded a new version of Tools to project The Trunk:<br>
> > http://source.squeak.org/trunk/Tools-eem.1104.mcz [http://source.squeak.org/trunk/Tools-eem.1104.mcz]<br>
> ><br>
> > ==================== Summary ====================<br>
> ><br>
> > Name: Tools-eem.1104<br>
> > Author: eem<br>
> > Time: 13 January 2022, 10:21:10.513361 am<br>
> > UUID: 1d051d8d-473d-43ee-9628-d33992a5849f<br>
> > Ancestors: Tools-mt.1103<br>
> ><br>
> > Fix MNU in menu building for MessageTrace with no method selected<br>
> ><br>
> > =============== Diff against Tools-mt.1103 ===============<br>
> ><br>
> > Item was changed:<br>
> > ----- Method: CodeHolder>>isBreakOnEntry (in category 'breakpoints') -----<br>
> > isBreakOnEntry<br>
> > <br>
> > + ^self selectedClassOrMetaClass<br>
> > + ifNil: [false]<br>
> > + ifNotNil:<br>
> > + [:class|<br>
> > + (class<br>
> > + compiledMethodAt: self selectedMessageName<br>
> > + ifAbsent: nil)<br>
> > + ifNil: [false]<br>
> > + ifNotNil: [:method| method hasBreakpoint]]!<br>
> > - | selectedMethod |<br>
> > - selectedMethod := self selectedClassOrMetaClass<br>
> > - compiledMethodAt: self selectedMessageName<br>
> > - ifAbsent: [^ false].<br>
> > - ^ selectedMethod hasBreakpoint!<br>
> ><br>
> ><br>
> > -------------- next part --------------<br>
> > An HTML attachment was scrubbed...<br>
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0001.html><br>
> > -------------- next part --------------<br>
> > A non-text attachment was scrubbed...<br>
> > Name: image.png<br>
> > Type: image/png<br>
> > Size: 22635 bytes<br>
> > Desc: not available<br>
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0005.png><br>
> > -------------- next part --------------<br>
> > A non-text attachment was scrubbed...<br>
> > Name: pastedImage.png<br>
> > Type: image/png<br>
> > Size: 4074 bytes<br>
> > Desc: not available<br>
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0006.png><br>
> > -------------- next part --------------<br>
> > A non-text attachment was scrubbed...<br>
> > Name: pastedImage.png<br>
> > Type: image/png<br>
> > Size: 18275 bytes<br>
> > Desc: not available<br>
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0007.png><br>
> > -------------- next part --------------<br>
> > A non-text attachment was scrubbed...<br>
> > Name: pastedImage.png<br>
> > Type: image/png<br>
> > Size: 15326 bytes<br>
> > Desc: not available<br>
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0008.png><br>
> > -------------- next part --------------<br>
> > A non-text attachment was scrubbed...<br>
> > Name: image.png<br>
> > Type: image/png<br>
> > Size: 78379 bytes<br>
> > Desc: not available<br>
> > URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/a8d504cb/attachment-0009.png><br>
> ><br>
> ><br>
> -------------- next part --------------<br>
> An HTML attachment was scrubbed...<br>
> URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20220114/c96d0a4c/attachment.html><br>
> <br>
> <br>
["menuBuilderFailed.1.cs"]