Bugzilla – Full Text Bug Listing
| Summary: | ModelService.cloneElement() should not clone widget and renderer fields | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Dean Roberts <dean.t.roberts> | ||||
| Component: | UI | Assignee: | Eric Moffatt <emoffatt> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | daniel_megert, emoffatt, pwebster, remy.suen | ||||
| Version: | 4.1 | ||||||
| Target Milestone: | 4.2 M2 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 339130 | ||||||
| Attachments: |
|
||||||
|
Description
Dean Roberts
The EMF default copier will not copy EStructureFeatures with isDerived set to false. I will submit a patch today (as soon as I figure out GIT) :-) Created attachment 200551 [details]
Add derived=true to all transient fields
The .ecore file adding derived=true tags to all transient fields in the model.
I believe the right work flow is to only include the .ecore file in the patch so that the committer can apply the patch and generate the model before commit.
Just a comment, when regenerating the workbench model, make sure you regenerate org.eclipse.e4.ui.model.workbench.edit as well OW Committed in >20110830. Applied the patch (+ regenerated code including 'edit') Reopening this bug as the changes have been reverted, see bug 356285. Per comment 2, we make the assumption that any transient EMF fields should also be derived. Is this assumption valid? While this assumption broke Eclipse and caused bug 356285, it is not to say that the assumption is invalid (but an indication that perhaps our menu rendering story needs work). Should we take the approach where most fields are transient and derived except a select few? Most fields are transient but only a few are derived? It turns out that the initial idea of 'no transient fields should be cloned' (i.e. 'transient' == 'derived') is naive (if not outright wrong). The 'clone' implementation just uses ECoreUtil's 'copy' mechanism (and I'd like to keep it hat way). The feature this work started out for is to allow the implementation of the Perspective's 'Save As...' and related functionality. In this case the clone most not contain rendering info (it will be rendered when it is opened). It turns out that parts of the command handling use EMF's copy mechanism as well, leading to today's broken build (sorry about that) so a new approach is needed. For now I'm going to make a revised version of the patch that restricts the use of 'derived' to specifically those fields that are used to contain instance-specific data; Contribution: 'object' Context: 'context' Dirtyable: 'dirty' UIElement: 'widget' & 'renderer' Placeholder: 'curSharedRef' Committed in >20110901. Make the changes as per the last comment. commit 128b649c05426407399735d4d6d3a3f64173a758 Verified with I20110705-1340 by exercising the save perspective code. Cut and paste error in previous comment. This was verified in I20110915-0200 |