Hi Alex,
unfortunately changes in TRCompositeShape created a regression, because now
TRRemoveCallbacks are executed multiple times.
the commit is Trachel-AlexandreBergel.266
Interestingly there is TRCompositeShapeTest>>testCallback01 which works,
but it doesn't use RTElement.
Here's a test case:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|v e executed|
v := RTView new.
e := RTElement new.
e + (RTBox new) + (RTEllipse new).
executed := 0.
e trachelShape addCallback: (TRRemoveCallback new block: [ :shape |
executed := executed + 1 ]).
v add: e.
TestAsserter new assert: executed equals: 0.
e remove.
TestAsserter new assert: executed equals: 1.
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Now since you have discussed with Jan that the new behavior makes sense in
some circumstances, it would be better to have both behaviors.
So I would propose to make a distinction for RemoveCallback - to not
triggered them from TRCompositeShape, because it is not reentrant.
Thus when executed on composite shape, only non-removing callbacks would be
executed.
Additionally it seems very random that CompositeShape uses it's first
subshape for callbacks; what is so special about the first one? Why can't
composite shape hold it's own callbacks, or have an explicit way to specify
which one to use (first, second, none, ..)?
Since Composite Shape is recurring problematic theme, perhaps there should
be some wider discussion to collect all the use cases?
What do you think?
Peter