Hi Jakob,
So far, you're my greatest brainstorming provider!
See my next solution in ToolBuilder-Kernel-nice.144.

I don't feel like I can omit the comments, which is a smell.
But that's the price for maintaining backward compatibility with
contradictory rules/features.


Le lun. 26 avr. 2021 à 23:29, Nicolas Cellier
<nicolas.cellier.aka.nice at gmail.com> a écrit :
>
> Le lun. 26 avr. 2021 à 23:17, Nicolas Cellier
> <nicolas.cellier.aka.nice at gmail.com> a écrit :
> >
> > Ah yes, tight coupling is not ideal...
> > It's pretty long to debug.
> > I see a (ProgressNotification signal: '' extra: (oldLabel :=
> > newLabel)) sent from inner do:displayProgress:
> > (do:displayProgress:every:)
> > This is indeed caught by outer displaySequentialProgress: which
> > interprets the messageText '' asNumber and fails...
> >
> > It appears like the enclosing on: ProgressInitiationException do: ...
> > sendNotificationsTo: ... is catching both initiations instead of only
> > the first one...
> > This is because the resumeEvaluating: rearmed the handler before
> > evaluating the block.
> > It does so while unwinding the ensure: [self reactivateHandler] in
> > handleSignal:...
> >
> > So, in normal case, the 2nd ProgressInitiationException is not caught...
> > It thus performs its default action, which is opening the progress
> > bar, and catching ProgressNotification to feed the progress bar...
> > In normal case, this is this second (on: ProgressNotification do:)
> > which is on top of stack of handlers...
> >
> > Somehow, my proposed solution is equivalent to evaluating this in
> > older image, which equally fails...
> >
> > ['Foobar' displaySequentialProgress:
> >     [#(a b c) do: [:each | (Delay forSeconds: 1) wait]
> > displayingProgress: [:each | 'item ', each]]]
> >         on: ProgressInitiationException do:
> >             [:e | e rearmHandlerDuring: [ e sendNotificationsTo: [:min
> > :max :val | "nothing"]]]
> >
> > If I change implementation with ugly:
> >
> > sendNotificationsTo: aNewBlock
> >     self resumeEvaluating:
> >         [self privHandlerContext deactivateHandler.
> >         [workBlock value: [ :barVal |
> >             aNewBlock value: minVal value: maxVal value: barVal]]
> >                 ensure: [self privHandlerContext reactivateHandler]]
> >
> > then this snippet pass...
> >
>
> sent too soon, sorry...
> The snippet you gave pass with horrific hack proposed above.
>
> But if we deactivateHandler inside sendNotificationsTo:, then we're
> not able to rearm anymore:
>
> on: ProgressInitiationException do:
>             [:e | e rearmHandlerDuring: [ e sendNotificationsTo: [:min
> :max :val | "nothing"]]]
>
> will now fail to properly rearm... Horrible.
>
> sendNotificationsTo: is really tightly coupled to the fact that inner
> handlers remain active...
>
> >
> > Le lun. 26 avr. 2021 à 21:44, Jakob Reschke <jakres+squeak at gmail.com> a écrit :
> > >
> > > Hi Nicolas,
> > >
> > > Unfortunately, there are yet other things not working now:
> > >
> > > ['Foobar' displaySequentialProgress:
> > >     [#(a b c) do: [:each | (Delay forSeconds: 1) wait]
> > > displayingProgress: [:each | 'item ', each]]]
> > >         on: ProgressInitiationException do:
> > >             [:e | e sendNotificationsTo: [:min :max :val | "nothing"]]
> > >
> > > will end with a MNU because the handler in displaySequentialProgress:
> > > does not support the ProgressNotification signalled from the loop.
> > > I did not debug it until the end, but I guess the implicit handler in
> > > MorphicUIManager>>#displayProgress:at:from:to:during: (emplaced by the
> > > loop) previously got the notification, but is now terminated away (by
> > > Context>>resumeEvaluating: in sendNotificationsTo:) before the
> > > workBlock is evaluated. So the loop progress notifications do not get
> > > there anymore, but to the handler in displaySequentialProgress:. If it
> > > is not terminated away, then I suppose the order of those two handlers
> > > on the stack is now swapped...
> > >
> > > Something like the above I use to test the progress suppressing during
> > > test cases, which was previously broken, but now works. Glad that I
> > > wrote these tests for the testing facility itself...
> > >
> > > It is quite messy how the progress stuff is coupled to the
> > > implementation details of exception handling.
> > >
> > > Kind regards,
> > > Jakob
> > >
> > > Am Mo., 26. Apr. 2021 um 02:20 Uhr schrieb <commits at source.squeak.org>:
> > > >
> > > > Nicolas Cellier uploaded a new version of ToolBuilder-Kernel to project The Inbox:
> > > > http://source.squeak.org/inbox/ToolBuilder-Kernel-nice.143.mcz
> > > >
> > > > ==================== Summary ====================
> > > >
> > > > Name: ToolBuilder-Kernel-nice.143
> > > > Author: nice
> > > > Time: 26 April 2021, 2:20:20.490026 am
> > > > UUID: fc579454-b15f-4d7e-bd7b-adfc3a1ad863
> > > > Ancestors: ToolBuilder-Kernel-nice.141
> > > >
> > > > Evaluate the new work block upon #sendNotificationsTo: in the context that sent the ProgressInitiationException signal
> > > >
> > > > =============== Diff against ToolBuilder-Kernel-nice.141 ===============
> > > >
> > > > Item was changed:
> > > >   ----- Method: ProgressInitiationException>>sendNotificationsTo: (in category 'initialize-release') -----
> > > >   sendNotificationsTo: aNewBlock
> > > > +       self resumeEvaluating: [workBlock value: [ :barVal |
> > > > -
> > > > -       self reactivateHandlers; resumeUnchecked: (
> > > > -               workBlock value: [ :barVal |
> > > >                         aNewBlock value: minVal value: maxVal value: barVal
> > > > +               ]]
> > > > -               ]
> > > > -       )
> > > >   !
> > > >
> > > >
> > >