[squeak-dev] The Inbox: ToolBuilder-Kernel-nice.143.mcz

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Wed Apr 28 19:42:58 UTC 2021


For compatibility, you can implement this:

resumeEvaluating: aBlock
  (self class superclass canUnderstand: #resumeEvaluating:) ifTrue:
[^super resumeEvaluating: aBlock].
  "compatibility with squeak < 6.0"
  ^self resume: aBlock value

Le mer. 28 avr. 2021 à 21:24, Nicolas Cellier
<nicolas.cellier.aka.nice at gmail.com> a écrit :
>
> Hi Jakob,
> yes, an even more brainstorming example...
> what happens is that this ProgressInitiationException handler action:
>
>             e rearmHandlerDuring:
>                         [[e sendNotificationsTo: [:min :max :current |
> "silence"]]
>                             on: ProgressNotification do:
> [:notification | notification resume]]
>
> is unwound by the resumeEvaluating: inside sendNotificationsTo:
> thus, the ( on: ProgressNotification do: ) is no more on the stack, so
> no more active...
> The handler is not even rearmed because of the unwinding, but it is
> still active.
>
> I suggest that we simply equip the ProgressInitiationException with
> appropriate protocol, like:
>
> resumeSuppressingProgress
>     "Catch and suppress every ProgressInitiationException and
> ProgressNotification
>     during the evaluation of workBlock"
>
>     ^self resumeEvaluating:
>         [self rearmHandlerDuring:
>             [[workBlock value: [ :barVal | "do nothing"]]
>                 on: ProgressNotification do: [:e | e resume]]]
>
> If I use this resumeSuppressingProgress in the tests, they pass.
> It's unfortunate that you would have to change the client code, as it
> will complicate the configuration for ensuring compatibility with
> various versions of Squeak, but I think that it's the most reasonable
> solution. Beside, the new message is clearly expressing the
> intention...
>
> See ToolBuilder-Kernel-nice.145
>
> Le mer. 28 avr. 2021 à 19:35, Jakob Reschke <jakres+squeak at gmail.com> a écrit :
> >
> > Hi Nicolas,
> >
> > Sorry, tests are still failing.
> >
> > Please install the Git Browser/Squot, then you will have a
> > TestCaseSuppressingProgressDisplayTraitTest in your image. With
> > ToolBuilder-Kernel-nice.144 and Kernel-nice.1394, it contains two
> > failing tests. Those tests assert that the progress suppressing does
> > not affect test results. But the supposedly-passing or -failing tests
> > currently signal errors. Those errors are still the MNU asNumber as in
> > my previous message, so the handlers are still not on the stack as the
> > progress implementation expects it.
> >
> > You can also directly run the underlying tests with:
> > (TestCaseSuppressingProgressDisplayTraitTestSubject selector:
> > #testThatPasses) debug.
> > (TestCaseSuppressingProgressDisplayTraitTestSubject selector:
> > #testThatFails) debug.
> >
> > For practical purposes, I can see that my Squot tests do display some
> > progress (below the TestRunner's progress bar) even though that should
> > have been suppressed.
> >
> > Kind regards,
> > Jakob
> >
> > Am Di., 27. Apr. 2021 um 13:57 Uhr schrieb Nicolas Cellier
> > <nicolas.cellier.aka.nice at gmail.com>:
> > >
> > > 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
> > > > > > > +               ]]
> > > > > > > -               ]
> > > > > > > -       )
> > > > > > >   !
> > > > > > >
> > > > > > >
> > > > > >
> > >
> >


More information about the Squeak-dev mailing list