|
Description
Maxime Porhel
The Sample Reflective Ecore Model Editor does not propose the lacking types. We might want to use EMF's own mechanism for this: http://ed-merks.blogspot.com/2008/01/creating-children-you-didnt-know.html Our own code to support this predates this feature in EMF, but now even the most ancient EMF version we support provides this. Maybe we could remove most/all of the DialectUIServices.provide* methods. Steps to reproduce on Sequence diagram # Create two Sequence diagrams with 2 lifelines, 1 execution on each and a message from the execution to the other lifeline. # Launch the arrange all on both diagrams # Close one of the diagram # Move one of the message before its execution (reconnect) # Reopen the second diagram # Try to move m1 up (a few pixels) -> If the bug is present, m1 does not move but the execution and the other events below are shifted down. Wrong comment (now added on Bug 432936) (In reply to Maxime Porhel from comment #3) > Steps to reproduce on Sequence diagram > # Create two Sequence diagrams with 2 lifelines, 1 execution on each and a > message from the execution to the other lifeline. > # Launch the arrange all on both diagrams > # Close one of the diagram > # Move one of the message before its execution (reconnect) > # Reopen the second diagram > # Try to move m1 up (a few pixels) > > -> If the bug is present, m1 does not move but the execution and the other > events below are shifted down. It looks like enabling the Child Creation Extender as mentioned in Ed's blog fixes the issue, but we have a lot of customized (@not-generated) code in the impacted classes, so it is not clear which ones would be impacted but are ignored by the regen. Also, the number of possible elements in the VSM make it difficult to be sure we have fixed all the cases. I'll see if we can write some automated tool to help. Created attachment 242317 [details]
NewChildDescriptorTest.java
Attached is an automated test that outputs a sorted list of all the "newChildDescriptors" available for all Sirius types available in a VSM editor. It is implemented as a JUnit test to be run as a "JUnit Plug-in Test" to be sure it is executed in the correct runtime context (with all the various extension points and registries correctly setup).
Created attachment 242318 [details]
The result of the test program when run on Sirius 1.0.0M6
Created attachment 242319 [details]
The result of the test program when run on the current master
Comparing the attached result when run on master to the one when run on M6 shows that we lost (among others) diagram.description.tool.CreateEdgeView, diagram.description.tool.CreateView and diagram.description.tool.Navigation which were mentioned in the issue description. There are many other differences, which probably require some deeper investigation.
Created attachment 242320 [details]
Draft of a patch that enables EMF's native child extender mechanism
Created attachment 242321 [details]
The result of the test program when run on master with the patch applied
With the patch applied, the test program produces the exact same output as what we had with Sirius 1.0.0M6, before the effective split of the diagram metamodel out of the core plug-ins.
Created attachment 242476 [details]
NewChildDescriptorTest.java
New version of the test program which avoids duplicate entries in the report.
Created attachment 242477 [details]
The result of the test program when run on Sirius 1.0.0M6
Updated version of the test result for M6 without the duplications.
Created attachment 242478 [details]
The result of the test program when run on the current master
Updated version of the result of the test program on current master (without the child extender patch), without the duplications.
Comparing the two reports, the elements we have lost (i.e. that can no longer be created using the VSM editor's menus) are: Inside viewpoint.description.DecorationDescriptionsSet: - diagram.description.MappingBasedDecoration Inside viewpoint.description.Environment: - diagram.description.tool.BehaviorTool - diagram.description.tool.ContainerCreationDescription - diagram.description.tool.ContainerDropDescription - diagram.description.tool.DeleteElementDescription - diagram.description.tool.DiagramCreationDescription - diagram.description.tool.DiagramNavigationDescription - diagram.description.tool.DirectEditLabel - diagram.description.tool.DoubleClickDescription - diagram.description.tool.EdgeCreationDescription - diagram.description.tool.NodeCreationDescription - diagram.description.tool.ReconnectEdgeDescription - diagram.description.tool.RequestDescription - diagram.description.tool.ToolGroup Inside most ContainerModelOperation and also Case/SwitchChild: - diagram.description.tool.CreateEdgeView - diagram.description.tool.CreateView - diagram.description.tool.Navigation The environement is never actually visible to the user inside the VSM editor. For the other cases, I confirm that we lost MappingBasedDecoration, CreateEdgeView, CreateView and Navigation. It looks like the test program's results are reliable, so for now I'll assume there are no other things lost. With only these 4 elements to handle, enabling the child extender mechanism seems overkill, especially this close to M7. I'll see if I can recover them "manually". org.eclipse.sirius.ui.business.api.dialect.DialectUIServices already provides some extension mechanism. In fact it provides 5 different ones, each narrowly targeted towards extending the provider of a specific kind of element. None of these correspond to the context we need for the elements we lost. We may need to enable EMF's mechanism, otherwise it means either adding 1 or 2 new ad hoc extensions methods (which are API), or create a new one which is more generic, but would basically reproduce what EMF does. There is a bug in the new version of the test program: I had to comment out the reference to org.eclipse.sirius.diagram.description.DescriptionPackage.eINSTANCE when trying to run the test under M6 (where the package did not exist), and forgot to uncomment it when running the test under M6 and master. Created attachment 242493 [details]
NewChildDescriptorTest.java
New version of the test program with the diagram package re-enabled.
Created attachment 242494 [details]
The result of the test program when run on Sirius 1.0.0M6
New version of the M6 log produced by the fixed test.
Created attachment 242495 [details]
The result of the test program when run on the current master
New version of the current master (without the patch) log produced by the fixed test.
Created attachment 242496 [details]
The result of the test program when run on master with the patch applied
New version of the log with the patch applied and the fixed test program.
Created attachment 242497 [details]
The result of the test program when run on the current master
New version of the log for master (no patch) with the fixed test program.
Fixed by 4dc0c302144f9edf86f06a5ccfbe0236cedc78f5. Note that there are other regressions in the VSM editor caused by the diagram split: menu items order and grouping, property sections ordering and customization. They are less critical and will be treated after M7. Available in Sirius 1.0.0M7 (see https://wiki.eclipse.org/Sirius/1.0.0M7 & http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/tag/?id=v1.0.0M7). |