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