[dali-dev] recent EMFBinding/Selection modifications

[Markus Kuppe <mkuppe@xxxxxxxxxxx>]

Hi all,

todays CVS head shows a few changes to the selection and binding
projects which I don't understand or see the need for. %)

Whats the use cases that makes these changes necessary (btw. I'm missing
a corresponding enhancement bug in Bugzilla)?

The points I like to address/discuss are:

1. Indirect selection was renamed to parent selection. This fits
probably the current design of the Dali project, but a selection hasn't
necessarily a parent association.
Picture an EMF model that has no EMF associations between the EObjects.
With such a model it isn't possible to use the EMF reflection mechanism.
Therefor the original implementation delegates this to
ISelectionResolver, which is supposed to be implemented by the "client"
code (in this case the Dali project
(CompilationUnitSelectionTracker.getEObjectsAt(EObject))). In no way it
should be resolved by the ISelectionNotification. This class doesn't
know and shouldn't know about the EMF model associations.

I admit, the name "indirect", in retrospective, is in fact misleading
though. A closure of selections might be better.

2. In regards to my first point, "EClass selectableParentEClass" should
be removed from EMFSWTBinding(IEMFBindingAction anAction, EClass
anEClass, EClass selectableParentEClass, Control aControl, int anEventType).
However this led to an interesting discussion between Christian and
myself this morning...
ISelectionDispatcher.getCurrentSelection(EClass) should be modified to
return the selection (selectee and closure of selectees) instead of the
selectee alone.
As already explained, EMFBinding isn't always able to navigate in the
EMF graph so we use the resolver mechanism again.

3. Let aside the first two arguments, the change to
PersistentOutlineSelectionListener
"aNotification.getSelectionForType(eClass, eClass)"
looks like a hack. Why not add a second method to ISelectionNotification
that takes one eClass as a parameter. That way we would avoid the if
statement in getSelectoinForType() too.

To sum things up, I suggest to revert back to yesterdays version and to 
modify the return type ISelectionDispatcher.getCurrentSelection(EClass).

--
Regards,
Markus Kuppe

Versant GmbH, European Headquarters
Wiesenkamp 22b, 22359 Hamburg, Germany
+49(40)60990-215, http://www.versant.com