Community
Participate
Working Groups
Papyrus 1.0.0 (Luna) RC3 It is not uncommon for logical UML models to span multiple physical resources, either by reuse of model libraries, application of profiles, or simply organizing complex models into separate loosely coupled storage units. In such cases it is desirable to be able to edit related and interdependent models in separate editors, but the current strategy for reconciling editors with external changes severely limits the utility of such a workflow. In particular, when one editor modifies a model that is used by another editor and saves those changes, the other editor is closed and re-opened in order to fully flush its current state and re-load the updates from storage. A more robust solution than this is needed to preserve the user's editing context as reflected in, amongst possibly others to be discovered: * organization of editors: order of tabs in one or more "editor areas" in the current perspective * active editor and diagram tab * active selection in the editor and/or ancillary views * undo/redo command history Some of these may be easier to maintain than others, some may be more important than others. This bugzilla will serve as an umbrella for this feature work, for tracking/reporting purposes, and will reference other bugzillae representing individual problem reports or sub-tasks.
Tentatively scheduled for SR1. We shall see from risk analysis whether it needs to be sent to Mars [1], instead. [1] Oo, I enjoyed writing that.
I have pushed an initial prototype of in-place editor re-load to a new branch committers/cdamus/editor_reload with a brief demonstration video: http://youtu.be/FqYzGAq6JmQ The approach in this prototype seems to show promise. The basic idea is to do everything that closing and re-opening an editor does, except without actually closing the editor but instead only "hollowing it out" and re-building it. So, the service registry and the sash container are disposed and then re-created. Because dependent views such as Model Explorer, Outline, and Properties still maintain pages for the reloaded editor, they need to be notified that references to the service registry, editing domain, etc. that they are holding on to need to be re-acquired. This is accomplished by a new listener interface that lets views dispose of dependent state before the editor reloads and then re-initialize their dependent state after it has finished reloading. Follow-up work will include capturing various context state before re-load and restoring it after: * in the editor, selected diagram tab, and selection within that tab * in dependent views, current selection and, for trees, which nodes are expanded * which workbench view is current and contributes the selection to the page A skeleton of the infrastructure required for all of this is already in place: IEditorReloadListeners capture state in the "about-to-reload" callback and restore it in the "reloaded" callback. However, I think I'll change the API signatures to return a state-capture token that the editor then passes in to the listener in the reloaded callback because it's possible that reloaded event never actually happens (if something goes wrong, the editor just closes and doesn't complete the reload) and I don't want listeners to have to maintain state that they would then not know when to clean up.
I have pushed a few new commits to the branch, up to 089faad, to implement restoration of certain state in dependent views after an editor has re-loaded: * for Model Explorer: * which nodes are expanded * which nodes are selected * for each page of the Outline view (different pages for each diagram in the editor): * which presentation is active (tree outline or overview thumbnail) * in the tree outline, which nodes are selected In all cases of tree node state (expansion and selection), nodes are identified by the URIs of the model elements that they represent. In the case that an element no longer exists after re-load (as could happen because that may be the change we're detecting in persistent storage, as saved by some other editor), then the node is simply ignored and its expansion/selection state is (obviously) not restored.
A few more commits on the branch (up to 6bb864a) improve the restoration of selection state, by adding the selection of edit parts in each open diagram. Edit parts to select are remembered by the URIs of the notation view elements that are their models. This actually remove the need to handle selection in the outline's tree viewer, which is a good thing because it wasn't done right anyways. :-) A brief video demonstrates: http://youtu.be/larpcyKFn94
A couple more small updates: Commit e80eeba adds the restoration of palette state for nested diagram editors in re-opened editors, delegating to the PaletteViewer's memento capability: * active tool * pinnable stack tool selection * drawer expansion state * drawer scroll position Commit b3bb863 fixes a problem in which after re-opening an editor it could no longer open new nested editor pages when creating new diagrams/tables in the Model Explorer.
Commit 02aafe5 adds support for restoring the drawer expansion, stack selection, and active tool states in the palette when the Palette View is in use. This incidentally fixes a problem in the Palette View, in which the PapyrusPaletteSynchronizer loses the state of the palette for the active nested diagram when switching between Papyrus Editors (the synchronizer causes a superfluous PalettePage adapter to be created for the editor as a whole).
Commit 10d7e46 adds support for restoring column and row selections in table editors. Sanity testing reload scenarios editors on CDO models that reference resources stored in the workspace checks out clean. The editor re-load works just the same in CDO. Of course, when all cross-referenced models are in the repository (or deployed in plug-ins, of course), the user experience is much smoother: there is no reloading of editors at all. The CDO transactions in other open editors get invalidation events that update them seamlessly.
Commit 2e9d737 adds JUnit tests covering various aspects of editor re-loading and preservation of the user's editing context.
I've updated the Gerrit review with changes. Patch set 5 iterates on the previous patch set, adding: * deferral of editor re-load to when the editor is next activated. If the dependent editor is dirty and the user elects to save it, it is saved immediately so that the user gets immediate feed-back that it is saved (by means of the asterisk decoration being removed). If the user opts not to save the dependent editor, then it is not saved but re-loading is deferred nonetheless. The option to ignore a dirty editor altogether works as before * updates to the JUnit tests to account for deferral of the editor re-load, including a new test verifying that re-load is deferred * fixed a problem in the restoration of the palette states of diagrams when the Palette view is active (PapyrusPaletteSynchronizer doing stuff in the wrong sequence when at initialization and disposal)
The Gerrit review https://git.eclipse.org/r/29933 is merged. A follow-up commit 7b9fa63b694baf79bb2d9d6f5d7d2c5c24dde46d adds capture and restore of which referenced library models in the editor were made explicitly writable, so that the following scenario works as expected: * open a model editor M in which M imports a library L * open L in its own editor * in M's editor, make L writable * modify and save L in its own editor * switch back to editor M, triggering re-load. Now L should still be writable in M's editor Ronan, over to you for verification. I know you'll do a thorough job and will find missed use cases and/or other bugs. :-)
Great work Christian! It looks so much better already. Would you like bugs as a reply here or new artifacts? They will only be minor I think. First up... Bug #1 a) Open model Z and Y. b) Now alter the name of Class Y to say Ys. c) Make the change by selecting the modelY.di and altering the name in the porject explorer. d) Now select modelZ.di and alter the name of property "paramater" to say "paramaters" e) Select save all. You will notice the properties view has lost context. It should be focusing on the property but it is in fact focusing on the owning class of the property, in this case Z.
Note: For Bug 1 It is the highlighted element on the diagram that decides the focus after clciking on "saving all". Even if I am editing using just the Model Explorer.
Issue #2: When following the procedure for Issue #1 and selecting "save all" (I called it bug #1 before but that was my bad as it creates a link to a real bug #1) I get a window saying "Resourced changed". It warns of unsaved changes being lost. I clicked "save all" so this means I have saved/will save all. To me this popup is redundant and somewhat confusing.
Issue #3: a) Resize the Model Explorer when modelZ is open so that you get a scrollbar. b) Scroll to the bottom and select some element down there. For example Package Y in the loaded ModelY c) Open the modelY editor and change the class name of "Y" to "Y1" d) Save modelY e) Switch to modelZ but do nothing. The scroll bar state has been maintained f) Now swicth back to modelY and change the class name of "Y1" to "Y12" g) Save modelY h) Now swicth back to modelY and note the scroll bar position was *not* maintained and the highlighted element cannot be seen. A manual scroll is needed to regain context.
(In reply to Ronan B from comment #11) > Great work Christian! It looks so much better already. Would you like bugs > as a reply here or new artifacts? They will only be minor I think. First > up... Hah! I knew you would make quick work of this one. Thanks! Replies/reopens here will be fine if the problems are incorrect behaviour of the solution that is implemented. For example, editor state/context that is not correctly restored, exceptions observed while an editor is re-loaded, and editor that isn't re-loaded that should have been. Function that has regressed because of the introduction of this new feature wants new bugzilla entries. E.g., views/menus/toolbars that stop working on an editor after it was re-loaded.
(In reply to Ronan B from comment #12) > Note: For Bug 1 It is the highlighted element on the diagram that decides > the focus after clciking on "saving all". Even if I am editing using just > the Model Explorer. Indeed, this looks like an ordering problem: the diagram state is restored after the Model Explorer state. (In reply to Ronan B from comment #13) > Issue #2: > When following the procedure for Issue #1 and selecting "save all" (I called > it bug #1 before but that was my bad as it creates a link to a real bug #1) Yeah. Refer to it as comment 11. :-) (frankly, I was surprised to find that Bug 1 was a real bug report and not just some test record) > I get a window saying "Resourced changed". It warns of unsaved changes being > lost. I clicked "save all" so this means I have saved/will save all. To me > this popup is redundant and somewhat confusing. Yep. (In reply to Ronan B from comment #14) > Issue #3: Whoa, slow down, buddy! ;-) > h) Now swicth back to modelY and note the scroll bar position was *not* > maintained and the highlighted element cannot be seen. A manual scroll is > needed to regain context. Yep.
Issue #4: a) Select the loaded model "UML Primitive Types" in modelZ b) Switch to modelY and change the name of Class "Y" to "Y1" c) Save modelY d) Switch to modelZ and note the element "UML Primitive Types" is *not* highlighted anymore. I assume the reloading of dependant models causes this disruption.
Looks like editing the Papyrus resources with the XML editor causes some major trouble: - Create an empty Papyrus model - Change the name of the root element with the XML editor - Save - Go back to the Papyrus editor - Everything's crashed (No more diagrams, ModelExplorer, Outline) !ENTRY org.eclipse.e4.ui.workbench 4 0 2014-07-28 14:01:51.423 !MESSAGE Error setting focus to : org.eclipse.e4.ui.model.application.ui.basic.impl.PartImpl model.profile.di !STACK 0 org.eclipse.core.runtime.AssertionFailedException: null argument: at org.eclipse.core.runtime.Assert.isNotNull(Assert.java:85) at org.eclipse.core.runtime.Assert.isNotNull(Assert.java:73) at org.eclipse.ui.part.PageSite.<init>(PageSite.java:91) at org.eclipse.papyrus.infra.core.contentoutline.NestedEditorDelegatedOutlinePage.initPage(NestedEditorDelegatedOutlinePage.java:428) at org.eclipse.papyrus.infra.core.contentoutline.NestedEditorDelegatedOutlinePage.doCreatePage(NestedEditorDelegatedOutlinePage.java:477) at org.eclipse.papyrus.infra.core.contentoutline.NestedEditorDelegatedOutlinePage.createPage(NestedEditorDelegatedOutlinePage.java:388) at org.eclipse.papyrus.infra.core.contentoutline.NestedEditorDelegatedOutlinePage.getOutlinePageRec(NestedEditorDelegatedOutlinePage.java:593) at org.eclipse.papyrus.infra.core.contentoutline.NestedEditorDelegatedOutlinePage.pageActivated(NestedEditorDelegatedOutlinePage.java:316) at org.eclipse.papyrus.infra.core.sasheditor.internal.SashContainerEventsProvider.firePageActivatedEvent(SashContainerEventsProvider.java:98) at org.eclipse.papyrus.infra.core.sasheditor.internal.SashWindowsContainer.setActivePage(SashWindowsContainer.java:379) [...] !ENTRY org.eclipse.papyrus.infra.core.sasheditor 4 0 2014-07-28 14:01:51.502 !MESSAGE Unexpected Error !STACK 0 org.eclipse.swt.SWTException: Widget is disposed at org.eclipse.swt.SWT.error(SWT.java:4441) at org.eclipse.swt.SWT.error(SWT.java:4356) at org.eclipse.swt.SWT.error(SWT.java:4327) at org.eclipse.swt.widgets.Widget.error(Widget.java:476) at org.eclipse.swt.widgets.Widget.checkWidget(Widget.java:348) at org.eclipse.swt.widgets.Composite.setFocus(Composite.java:1031) at org.eclipse.swt.widgets.Composite.setFocus(Composite.java:1039) at org.eclipse.papyrus.uml.diagram.profile.UmlProfileDiagramForMultiEditor.setFocus(UmlProfileDiagramForMultiEditor.java:159) at org.eclipse.papyrus.infra.core.sasheditor.internal.EditorPart.setFocus(EditorPart.java:652) at org.eclipse.papyrus.infra.core.sasheditor.internal.SashWindowsContainer.setFocus(SashWindowsContainer.java:600) at org.eclipse.papyrus.infra.core.sasheditor.internal.SashWindowsContainer.setFocus(SashWindowsContainer.java:588) at org.eclipse.papyrus.infra.core.sasheditor.editor.AbstractMultiPageSashEditor.setFocus(AbstractMultiPageSashEditor.java:275) at org.eclipse.papyrus.infra.core.editor.CoreMultiDiagramEditor.setFocus(CoreMultiDiagramEditor.java:888) at org.eclipse.ui.internal.e4.compatibility.CompatibilityPart.delegateSetFocus(CompatibilityPart.java:190) [...] If I reproduce the same steps with a standard text editor, it works fine. Might be related to the XML Editor's outline view
I have found another problem. Say I have editor A on model A that loads a resource from model B in editor B. 1) Change something in model B. Leave B unsaved. 2) Switch to editor A and change something in model A. 3) Now save All with Command+Shift+S. 4) Editor A does not re-load and so does not pick up the changes in B. The reason for this is that the ResourceUpdateService asynchronously resets the "isSaving" state. Consequently, when A's update service sees B has changed, it does nothing because it thinks the change is from A saving the B resource, itself, since "isSaving" is still true despite that editor A has already finished saving (it won't be reset until the UI event queue gets around to it later). This asynchronous reset was introduced as a (partial?) fix for bug 411574, but this in-place re-loading is a more complete fix, so I think I can just revert the isSaving reset to be synchronous.
(In reply to comment #18) > Looks like editing the Papyrus resources with the XML editor causes some major > trouble: Is this new since in-place re-loading, though? Seems like something that should have been a problem before. I'll check it out.
An additional use case which isn't supported, using Profile + Model: - Create a Profile, define it - Create a Model, apply the profile - Change the profile, redefine it - Come back to the model: the Profile resource is reloaded, but the "Repair profile" dialog doesn't pop up (Neither "Repair" nor "Reapply")
(In reply to Camille Letavernier from comment #21) > An additional use case which isn't supported, using Profile + Model: > > - Create a Profile, define it > - Create a Model, apply the profile > - Change the profile, redefine it > - Come back to the model: the Profile resource is reloaded, but the "Repair > profile" dialog doesn't pop up (Neither "Repair" nor "Reapply") That's odd. The editor creates a new ServiceRegistry, which between snippets on a new model-set and new lifecycle listeners should kick the ProfilesReapplyService just as a new editor would. I'll look into it.
> Is this new since in-place re-loading, though? Seems like something that should have been a problem before. I'll check it out. I regularly use the XML editor with Papyrus resources for debugging, and have not seen this issue until now
Issue #5: a) Open modelY and modelZ editors in the tiled mode/side by side mode i.e. like you have it in your video at http://www.youtube.com/watch?v=larpcyKFn94&feature=youtu.be 3:37 b) Change the name of Class "Y" to "Y1" c) Save modelY You will notice that the modelZ editor has not shown any updates. I must click on the modelZ editor for the changes to be seen. This is not the same behaviour as I see in your video.
> Issue #5: This is actually intended (The change has been made on purpose, a few days after the video was published). Reloading an editor takes time, and needs to be done mostly in the UI thread. Reloading immediately would freeze Eclipse for a few seconds, or tens of seconds for each medium-sized model. So, the reload only occurs when you switch back to the editor (So that it happens less often, and only when you actually need it) This is mainly due to Bug 394779: [Performances] Do not open all nested editors at once
(In reply to comment #23) > > Is this new since in-place re-loading, though? Seems like something that > should have been a problem before. I'll check it out. > > I regularly use the XML editor with Papyrus resources for debugging, and have > not seen this issue until now Yeah, it was simply a problem of prematurely discarding the ContentOutlineRegistry. The editor's outline page needs to be handled the same as property pages: the same page needs to be retained for the lifetime of the editor, not discarded at re-load. I have a fix in hand.
(In reply to Camille Letavernier from comment #25) > > Issue #5: > > This is actually intended (The change has been made on purpose, a few days > after the video was published). Reloading an editor takes time, and needs to > be done mostly in the UI thread. Reloading immediately would freeze Eclipse > for a few seconds, or tens of seconds for each medium-sized model. So, the > reload only occurs when you switch back to the editor (So that it happens > less often, and only when you actually need it) > > This is mainly due to Bug 394779: [Performances] Do not open all nested > editors at once It might be intended be I don't think it is a good solution. If the editor is already open, and the particular element in question has had a dependency modified, which it is the case here, it should be updated on screen. I'd agree with you if the editor was not visible but in this case it is visible.
I've pushed some fixes for the problems reported, so far. Other issues I have looked at and provide comments. comment 11: Fixed in commit 873ee04cc69a1ae657368106793651aaa01a1cd5. I implemented a custom property page for the Papyrus editor that restores the input of the Properties view using its “Show In” context. comment 13: Eclipse provides editors with no means to determine whether they are being saved by invocation of the "Save All" action (as opposed to simply "Save" or "Save As..."). The only way to prevent this confusing dialog (I agree that it is superfluous) is to defer not only the re-loading of an editor but also the prompting to save it until that editor is next activated. I think that's the best thing to do, because showing this dialog really is wrong. Other opinions? comment 14: The selection is restored with a “select reveal” , which scrolls the viewer to show the selected elements. This is working for me. The viewer isn’t scrolled to exactly the same position as before, but the selected element(s) is/are visible. So, I'm not able to reproduce a problem of the element selected in the Model Explorer not being visible. Is there any step I might be missing? comment 17: Fixed in commit 43f60feb94fc733617ef0f62093cf7e52f5fd259. I have added re-load context remembering loaded resources to forcibly re-load them before restoring the Model Explorer selection, because otherwise proxy resolution hasn't yet loaded some referenced resources. Incidentally fixes problem of losing context of models that were imported in the editor but not yet used by referencing any of their contained elements. comment 18: Fixed in commit d5e9a0bf66b5340605f820c5a197ae88b072715f. The editor’s outline page was disposed on wrong editor lifecycle event. Now it is only disposed when the editor is actually closed (not when it's re-loaded). comment 19: Fixed in commit 381f25bc20e70e9d5072e616be071104fcf4e64b. Save All didn’t cause the active editor to re-load because its ResourceUpdateService thought it was still saving (because of asynchronous reset of saving condition). The "isSaving" condition is now re-set synchronously. I see no evidence that workspace resource notifications are being sent on a different thread than the thread that initiates saving of the editor, which is the (suspected?) race condition that this asynchronous update worked around, previously. comment 24: This is how many other (most?) editors in Eclipse work: they refresh themselves, if necessary, on activation. Text editors are different because multiple text editors on the same file share an in-memory "text buffer" that they all edit together. The analogue in EMF applications is sharing a single resource set amongst editors; we're not going there (yet).
Oh, and I've also added new JUnit tests to cover some of the problems observed today and addresed in comment 28: commit ff92db7ebae27632a177d12aa2bf60ef20290de1.
(In reply to Christian W. Damus from comment #28) > I've pushed some fixes for the problems reported, so far. Other issues I > have looked at and provide comments. > > comment 11: Fixed in commit 873ee04cc69a1ae657368106793651aaa01a1cd5. I > implemented a custom property page for the Papyrus editor that restores the > input of the Properties view using its “Show In” context. I don't see any change to the bahaviour. Still open I think. > > comment 13: Eclipse provides editors with no means to determine whether they > are being saved by invocation of the "Save All" action (as opposed to simply > "Save" or "Save As..."). The only way to prevent this confusing dialog (I > agree that it is superfluous) is to defer not only the re-loading of an > editor but also the prompting to save it until that editor is next > activated. I think that's the best thing to do, because showing this dialog > really is wrong. Other opinions? > > comment 14: The selection is restored with a “select reveal” , which scrolls > the viewer to show the selected elements. This is working for me. The > viewer isn’t scrolled to exactly the same position as before, but the > selected element(s) is/are visible. So, I'm not able to reproduce a problem > of the element selected in the Model Explorer not being visible. Is there > any step I might be missing? Still not working. The key here is not to touch anything in step (e). It is the repetition of changing something in Y and switching back to modelZ twice that shows the bug. If you "touch" modelZ in step (e) you won't see the bug. > > comment 17: Fixed in commit 43f60feb94fc733617ef0f62093cf7e52f5fd259. I > have added re-load context remembering loaded resources to forcibly re-load > them before restoring the Model Explorer selection, because otherwise proxy > resolution hasn't yet loaded some referenced resources. Incidentally fixes > problem of losing context of models that were imported in the editor but not > yet used by referencing any of their contained elements. Confirmed fixed :) > > comment 18: Fixed in commit d5e9a0bf66b5340605f820c5a197ae88b072715f. The > editor’s outline page was disposed on wrong editor lifecycle event. Now it > is only disposed when the editor is actually closed (not when it's > re-loaded). > > comment 19: Fixed in commit 381f25bc20e70e9d5072e616be071104fcf4e64b. Save > All didn’t cause the active editor to re-load because its > ResourceUpdateService thought it was still saving (because of asynchronous > reset of saving condition). The "isSaving" condition is now re-set > synchronously. I see no evidence that workspace resource notifications are > being sent on a different thread than the thread that initiates saving of > the editor, which is the (suspected?) race condition that this asynchronous > update worked around, previously. > > comment 24: This is how many other (most?) editors in Eclipse work: they > refresh themselves, if necessary, on activation. Text editors are different > because multiple text editors on the same file share an in-memory "text > buffer" that they all edit together. The analogue in EMF applications is > sharing a single resource set amongst editors; we're not going there (yet). If I use "another well known UML tool", which i guess is sharing a simgle resource, I can see the editors update instantly. It even updates before I actually save. The latter isn't needed I think but an update of editors that are affected upon save is necessary. This kind of thing confuses users and the way it is now is not user friendly.
(In reply to Ronan B from comment #30) > (In reply to Christian W. Damus from comment #28) > > I've pushed some fixes for the problems reported, so far. Other issues I > > have looked at and provide comments. > > > > comment 11: Fixed in commit 873ee04cc69a1ae657368106793651aaa01a1cd5. I > > implemented a custom property page for the Papyrus editor that restores the > > input of the Properties view using its “Show In” context. > > I don't see any change to the bahaviour. Still open I think. Hmm, thanks, Ronan. Perhaps there is some other sequence of steps required to reproduce this than what I did, or else it depends on the order of editor tabs in the workbench window? (Save All progresses from left to right) Would you mind providing more detailed steps to reproduce? See what I'm seeing, here: http://youtu.be/7KIbIeaQvvA > > comment 14: The selection is restored with a “select reveal” , which scrolls > > the viewer to show the selected elements. This is working for me. The > > viewer isn’t scrolled to exactly the same position as before, but the > > selected element(s) is/are visible. So, I'm not able to reproduce a problem > > of the element selected in the Model Explorer not being visible. Is there > > any step I might be missing? > > Still not working. The key here is not to touch anything in step (e). It is > the repetition of changing something in Y and switching back to modelZ twice > that shows the bug. If you "touch" modelZ in step (e) you won't see the bug. I don't understand. At step (e) I can't just do nothing because the editor will re-load itself. I think you may have to spell out the steps to reproduce in more detail for me (sorry). I can repeatedly edit and save a referenced model Y and the referencing model Z's Model Explorer selection is always visible. Again, maybe it's dependent on the order of editors in the layout or some other factor I am missing? See what I'm seeing, here: http://youtu.be/-2eXvLhtDWo > If I use "another well known UML tool", which i guess is sharing a simgle > resource, I can see the editors update instantly. It even updates before I > actually save. The latter isn't needed I think but an update of editors that > are affected upon save is necessary. This kind of thing confuses users and > the way it is now is not user friendly. There is a tool that I know that exhibits the behaviour you describe, and it reflects changes in other editors immediately (before saving) because it uses a single resource set shared by all editors. Any given model element is loaded exactly once. That option isn't feasible for SR1 ...
(In reply to Christian W. Damus from comment #31) > (In reply to Ronan B from comment #30) > > (In reply to Christian W. Damus from comment #28) > > > I've pushed some fixes for the problems reported, so far. Other issues I > > > have looked at and provide comments. > > > > > > comment 11: Fixed in commit 873ee04cc69a1ae657368106793651aaa01a1cd5. I > > > implemented a custom property page for the Papyrus editor that restores the > > > input of the Properties view using its “Show In” context. > > > > I don't see any change to the bahaviour. Still open I think. > > Hmm, thanks, Ronan. Perhaps there is some other sequence of steps required > to reproduce this than what I did, or else it depends on the order of editor > tabs in the workbench window? (Save All progresses from left to right) > Would you mind providing more detailed steps to reproduce? See what I'm > seeing, here: > > http://youtu.be/7KIbIeaQvvA Hmm let me see. After chaging the attribute name in step (d) I do not click anywhere i.e. I do *not* reselect anything in the Model Explorer. I save all directly via the icon or shortcut. The key difference is that my second editor doesn't show as dirty yet because I have just changed the property name without changing context. This means I get the "Resources changed" dialog. After this dialog the properties view does not get the context correct. > > > > > comment 14: The selection is restored with a “select reveal” , which scrolls > > > the viewer to show the selected elements. This is working for me. The > > > viewer isn’t scrolled to exactly the same position as before, but the > > > selected element(s) is/are visible. So, I'm not able to reproduce a problem > > > of the element selected in the Model Explorer not being visible. Is there > > > any step I might be missing? > > > > Still not working. The key here is not to touch anything in step (e). It is > > the repetition of changing something in Y and switching back to modelZ twice > > that shows the bug. If you "touch" modelZ in step (e) you won't see the bug. > > I don't understand. At step (e) I can't just do nothing because the editor > will re-load itself. I think you may have to spell out the steps to > reproduce in more detail for me (sorry). I can repeatedly edit and save a > referenced model Y and the referencing model Z's Model Explorer selection is > always visible. Again, maybe it's dependent on the order of editors in the > layout or some other factor I am missing? See what I'm seeing, here: > > http://youtu.be/-2eXvLhtDWo > Step (h) says "Now swicth back to modelY and note the scroll bar position was *not* maintained and the highlighted element cannot be seen. A manual scroll is needed to regain context." it should read "Now swicth back to model*Z* and note the scroll bar position was *not* maintained and the highlighted element cannot be seen. A manual scroll is needed to regain context." Regardless what you are doing in your test-case is correct :) For me the scroll is maintained but the Model Explorer is not updated correctly to show the scrolling i.e. it shows the top part of the model even though the scroll bar is moved down. I must click on the scroll bar, without moving it, and then suddenly the Model Explorer fixes itself. This could be a linux thing? > > > If I use "another well known UML tool", which i guess is sharing a simgle > > resource, I can see the editors update instantly. It even updates before I > > actually save. The latter isn't needed I think but an update of editors that > > are affected upon save is necessary. This kind of thing confuses users and > > the way it is now is not user friendly. > > There is a tool that I know that exhibits the behaviour you describe, and it > reflects changes in other editors immediately (before saving) because it > uses a single resource set shared by all editors. Any given model element > is loaded exactly once. That option isn't feasible for SR1 ... But it worked in your first video didn't it?
(In reply to comment #32) > > Hmm, thanks, Ronan. Perhaps there is some other sequence of steps required > > to reproduce this than what I did, or else it depends on the order of editor > > tabs in the workbench window? (Save All progresses from left to right) > > Would you mind providing more detailed steps to reproduce? See what I'm > > seeing, here: > > > > http://youtu.be/7KIbIeaQvvA > > Hmm let me see. After chaging the attribute name in step (d) I do not click > anywhere i.e. I do *not* reselect anything in the Model Explorer. I save all > directly via the icon or shortcut. The key difference is that my second editor > doesn't show as dirty yet because I have just changed the property name without > changing context. This means I get the "Resources changed" dialog. After this > dialog the properties view does not get the context correct. Eureka! I can now repro. In this scenario, the second model isn't actually modified because the Properties view hasn't applied its edit. That's the bit I wasn't grokking, before. So, what actually happens is that the Save All doesn't have to save the second model, because it's not dirty, but Save All does save the first model. That causes the Properties view to lose focus, which consequently applies its edit to the second model, which now is dirty, so saving the first model has to prompt to ask what to do with the second model that's now dirty. So, now that whole save rigmarole ends up putting focus on the editor, not the Model Explorer, so that the editor's selection overrides the Model Explorer's in the Properties view. Ugh. > For me the scroll is maintained but the Model Explorer is not updated correctly > to show the scrolling i.e. it shows the top part of the model even though the > scroll bar is moved down. I must click on the scroll bar, without moving it, and > then suddenly the Model Explorer fixes itself. This could be a linux thing? You're saying that the scroll bar is showing the thumb positioned at the correct place but the viewport's content is not actually scrolled to that position? That does look like a SWT/GTK bug (I think the GTK3 implementation of SWT is new in Luna?). If you have access to a Windows system to try to reproduce this, I'd greatly appreciate it. Otherwise, as I can't repro on Mac, there's not much that I can do about it. > > > If I use "another well known UML tool", which i guess is sharing a simgle > > > resource, I can see the editors update instantly. It even updates before I > > > actually save. The latter isn't needed I think but an update of editors that > > > are affected upon save is necessary. This kind of thing confuses users and > > > the way it is now is not user friendly. > > > > There is a tool that I know that exhibits the behaviour you describe, and it > > reflects changes in other editors immediately (before saving) because it > > uses a single resource set shared by all editors. Any given model element > > is loaded exactly once. That option isn't feasible for SR1 ... > > But it worked in your first video didn't it? Only by accident of a bug: interim version in the video immediately re-loaded all editors that saw a change in one of their resources, even if they weren't actually visible. The cost of reloading all of the nested diagram editors can really add up, so we implemented the reload-on-activation policy. Note that we haven't even touched on the case of changes made outside of the Eclipse context, such as when on the git command-line I do a pull that updates some resource used by one of my open editors ...
(In reply to comment #33) > > Eureka! I can now repro. > > In this scenario, the second model isn't actually modified because the > Properties view hasn't applied its edit. That's the bit I wasn't grokking, > before. > > So, what actually happens is that the Save All doesn't have to save the second > model, because it's not dirty, but Save All does save the first model. That > causes the Properties view to lose focus, which consequently applies its edit to > the second model, which now is dirty, so saving the first model has to prompt to > ask what to do with the second model that's now dirty. So, now that whole save > rigmarole ends up putting focus on the editor, not the Model Explorer, so that > the editor's selection overrides the Model Explorer's in the Properties view. > Ugh. I have a change in code review that fixes this problem: https://git.eclipse.org/r/30701 You can see it in action, here (2.5 mins video): http://youtu.be/xbIdLN4WKy8 This incidentally also fixes the problem raised in comment 13 about Save All popping up an inappropriate prompt to deal with unsaved changes because the prompting to re-load a dirty editor is now also deferred until the next activation of that editor. Not having gotten any comments disagreeing with that approach to fix comment 13, and seeing that it also fixes the comment 11 problem, I'll be more than happy to merge it. I do need the code review, though, for the changes in various interfaces and redistribution of responsibilities amongst various classes.
(In reply to Christian W. Damus from comment #34) > > I have a change in code review that fixes this problem: > > https://git.eclipse.org/r/30701 > > You can see it in action, here (2.5 mins video): > > http://youtu.be/xbIdLN4WKy8 > This looks good to me!
Issue #6: a) Install the modied models attached b) Set up your editors as in the attached Bug437217-Issue6-Screenshot.png c) Click on the CD1 tab in the modelZ.di editor d) Swicth to the modelY and change the class Y to Y2 e) Swicth back to modelZ and save all You will notice that the last diagram tab in the right most editor has suddenly got focus, even though it was the left most tab that had focus before I clicked on save.
Created attachment 245509 [details] Test Models for Issue 6
Created attachment 245510 [details] Screenshot for Issue 6
(In reply to Christian W. Damus from comment #33) > You're saying that the scroll bar is showing the thumb positioned at the > correct place but the viewport's content is not actually scrolled to that > position? That does look like a SWT/GTK bug (I think the GTK3 > implementation of SWT is new in Luna?). If you have access to a Windows > system to try to reproduce this, I'd greatly appreciate it. Otherwise, as I > can't repro on Mac, there's not much that I can do about it. > Ok I can confirm this is a Linux issue. On Windows this "sort of" works. The selection is remembered and the scroll bar is scrolled to the selection but the scroll bar is not reset to the actual former position. Is it possible to save the state of both? > > Only by accident of a bug: interim version in the video immediately > re-loaded all editors that saw a change in one of their resources, even if > they weren't actually visible. The cost of reloading all of the nested > diagram editors can really add up, so we implemented the > reload-on-activation policy. Ah okay now I see why you took this out, it could be really expensive. That said unless a solution is found a clear caveat must be stated in the documentation. There will be complaints about this i'm sure :) > > Note that we haven't even touched on the case of changes made outside of the > Eclipse context, such as when on the git command-line I do a pull that > updates some resource used by one of my open editors ... I did some tests with SVN updates/sync from within eclipse and all worked as expected. But indeed I did not try doing this out of the Eclipse context.
(In reply to comment #36) > Issue #6: > a) Install the modied models attached > b) Set up your editors as in the attached Bug437217-Issue6-Screenshot.png > c) Click on the CD1 tab in the modelZ.di editor > d) Swicth to the modelY and change the class Y to Y2 > e) Swicth back to modelZ and save all > > You will notice that the last diagram tab in the right most editor has suddenly > got focus, even though it was the left most tab that had focus before I clicked > on save. Yep. We're not accounting correctly for sash divisions within the editors. Easily fixed: commit df3f7725ccdc33d3f586f1071616c404ca0b75b9, including a JUnit regression test. That is to say, assuming that I understand correctly that * by "leftmost tab had focus" you meant that in the right sash-pane of the modelZ editor the CD2 diagram was visible * by "right-most editor has ... focus" you mean that in the right sash-pane of the modelZ editor the CDX diagram is now visible What was working correctly in this scenario (as in other simpler cases) is that the actual focus (the selected diagram page) in modelZ after re-load was still CD1, the one in the left sash-pane, as indicated by the fact that the Z package was selected in the Properties view, not CopyOf_Z_1 (CD2) nor ModelZ (CDX).
(In reply to comment #39) > > Ok I can confirm this is a Linux issue. On Windows this "sort of" works. The > selection is remembered and the scroll bar is scrolled to the selection but the > scroll bar is not reset to the actual former position. Is it possible to save > the state of both? I had considered that, but the problem is that the changes in the resource that changed on disk (triggering the re-load) can significantly alter the structure of the Model Explorer tree. The selected object could end up in a very different place in the tree than where it previously was, so if we just scroll the viewport to where it previously was, then the selected object may not be visible. The important factors for continuity of the user's work are that the same objects be selected (if they still exist!) and be visible. The purpose (as I see it) isn't to make it appear as though nothing at all happened to the editor.
(In reply to Christian W. Damus from comment #41) > (In reply to comment #39) > > > > Ok I can confirm this is a Linux issue. On Windows this "sort of" works. The > > selection is remembered and the scroll bar is scrolled to the selection but the > > scroll bar is not reset to the actual former position. Is it possible to save > > the state of both? > > I had considered that, but the problem is that the changes in the resource > that changed on disk (triggering the re-load) can significantly alter the > structure of the Model Explorer tree. That is true and I agree. > The selected object could end up in a > very different place in the tree than where it previously was, so if we just > scroll the viewport to where it previously was, then the selected object may > not be visible. > > The important factors for continuity of the user's work are that the same > objects be selected (if they still exist!) and be visible. The purpose (as > I see it) isn't to make it appear as though nothing at all happened to the > editor. Agreed but you just highlighted a new issue :) Issue #7: a) Hightlight the read-only and loaded Class Y using the Model Explorer in modelZ b) Change to modelY and delete the Class Y and then save c) Reopen modelZ and you will get an exception as below 14-07-30 15:51:52.299 XtextBuilder [INFO] Build TestModels in 1 ms !ENTRY org.eclipse.papyrus.infra.core 4 0 2014-07-30 15:51:58.647 !MESSAGE Uncaught exception in editor reload listener. !STACK 0 java.util.MissingResourceException: The string resource 'platform:/resource/TestModels/modelY.di' could not be located at org.eclipse.papyrus.infra.services.resourceloading.impl.ProxyManager.getEObjectFromStrategy(ProxyManager.java:167) at org.eclipse.papyrus.infra.services.resourceloading.OnDemandLoadingModelSet.getEObject(OnDemandLoadingModelSet.java:77) at org.eclipse.papyrus.cdo.core.resource.CDOAwareModelSet.getEObject(CDOAwareModelSet.java:91) at org.eclipse.papyrus.infra.core.editor.reload.IInternalEMFSelectionContext$Default.resolveToken(IInternalEMFSelectionContext.java:56) at org.eclipse.papyrus.infra.core.editor.reload.EMFTreeViewerContext.resolveToken(EMFTreeViewerContext.java:50) at org.eclipse.papyrus.infra.core.editor.reload.EMFTreeViewerContext.resolveToken(EMFTreeViewerContext.java:1) at org.eclipse.papyrus.infra.core.editor.reload.SelectionContext.resolve(SelectionContext.java:68) at org.eclipse.papyrus.infra.core.editor.reload.SelectionContext.restore(SelectionContext.java:48) at org.eclipse.papyrus.infra.core.editor.reload.TreeViewerContext.restore(TreeViewerContext.java:52) at org.eclipse.papyrus.views.modelexplorer.ModelExplorerView$4.editorReloaded(ModelExplorerView.java:240) at org.eclipse.papyrus.infra.core.editor.reload.EditorReloadEvent.dispatchEditorReloaded(EditorReloadEvent.java:153) at org.eclipse.papyrus.infra.core.editor.CoreMultiDiagramEditor.doReload(CoreMultiDiagramEditor.java:920) at org.eclipse.papyrus.infra.core.editor.CoreMultiDiagramEditor.setFocus(CoreMultiDiagramEditor.java:892) at org.eclipse.ui.internal.e4.compatibility.CompatibilityPart.delegateSetFocus(CompatibilityPart.java:190) at sun.reflect.GeneratedMethodAccessor42.invoke(Unknown Source) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:55) at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:247) at org.eclipse.e4.core.internal.di.InjectorImpl.invokeUsingClass(InjectorImpl.java:253) at org.eclipse.e4.core.internal.di.InjectorImpl.invoke(InjectorImpl.java:225) at org.eclipse.e4.core.contexts.ContextInjectionFactory.invoke(ContextInjectionFactory.java:107) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.focusGui(PartRenderingEngine.java:795) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:679) at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:613) at org.eclipse.e4.ui.internal.workbench.swt.AbstractPartRenderer.activate(AbstractPartRenderer.java:106) at org.eclipse.e4.ui.workbench.renderers.swt.StackRenderer$10.widgetSelected(StackRenderer.java:1039) at org.eclipse.swt.widgets.TypedListener.handleEvent(TypedListener.java:248) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4486) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1388) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1412) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1397) at org.eclipse.swt.widgets.Widget.notifyListeners(Widget.java:1182) at org.eclipse.swt.custom.CTabFolder.setSelection(CTabFolder.java:3110) at org.eclipse.swt.custom.CTabFolder.onMouse(CTabFolder.java:1794) at org.eclipse.swt.custom.CTabFolder$1.handleEvent(CTabFolder.java:283) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:84) at org.eclipse.swt.widgets.Display.sendEvent(Display.java:4486) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:1388) at org.eclipse.swt.widgets.Display.runDeferredEvents(Display.java:3831) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:3441) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine$9.run(PartRenderingEngine.java:1151) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.e4.ui.internal.workbench.swt.PartRenderingEngine.run(PartRenderingEngine.java:1032) at org.eclipse.e4.ui.internal.workbench.E4Workbench.createAndRunUI(E4Workbench.java:148) at org.eclipse.ui.internal.Workbench$5.run(Workbench.java:636) at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:332) at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:579) at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150) at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:135) at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:196) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:134) at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:104) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:382) at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:236) at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.lang.reflect.Method.invoke(Method.java:606) at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:648) at org.eclipse.equinox.launcher.Main.basicRun(Main.java:603) at org.eclipse.equinox.launcher.Main.run(Main.java:1465) at org.eclipse.equinox.launcher.Main.main(Main.java:1438)
(In reply to Christian W. Damus from comment #40) > (In reply to comment #36) > > Issue #6: > > a) Install the modied models attached > > b) Set up your editors as in the attached Bug437217-Issue6-Screenshot.png > > c) Click on the CD1 tab in the modelZ.di editor > > d) Swicth to the modelY and change the class Y to Y2 > > e) Swicth back to modelZ and save all > > > > You will notice that the last diagram tab in the right most editor has suddenly > > got focus, even though it was the left most tab that had focus before I clicked > > on save. > > Yep. We're not accounting correctly for sash divisions within the editors. > Easily fixed: commit df3f7725ccdc33d3f586f1071616c404ca0b75b9, including a > JUnit regression test. > > That is to say, assuming that I understand correctly that > > * by "leftmost tab had focus" you meant that in the right sash-pane of the > modelZ editor the CD2 diagram was visible > * by "right-most editor has ... focus" you mean that in the right sash-pane > of the modelZ editor the CDX diagram is now visible > > What was working correctly in this scenario (as in other simpler cases) is > that the actual focus (the selected diagram page) in modelZ after re-load > was still CD1, the one in the left sash-pane, as indicated by the fact that > the Z package was selected in the Properties view, not CopyOf_Z_1 (CD2) nor > ModelZ (CDX). Yep roger that, that is what I meant. I'll say sash from now on :)
(In reply to comment #42) > > Issue #7: > a) Hightlight the read-only and loaded Class Y using the Model Explorer in > modelZ > b) Change to modelY and delete the Class Y and then save > c) Reopen modelZ and you will get an exception as below > > 14-07-30 15:51:52.299 XtextBuilder [INFO] Build TestModels in 1 ms > > !ENTRY org.eclipse.papyrus.infra.core 4 0 2014-07-30 15:51:58.647 > !MESSAGE Uncaught exception in editor reload listener. > !STACK 0 > java.util.MissingResourceException: The string resource > 'platform:/resource/TestModels/modelY.di' could not be located > at > org.eclipse.papyrus.infra.services.resourceloading.impl.ProxyManager.getEObjectFromStrategy(ProxyManager.java:167) Wow. What on Earth is a MissingResourceException doing here? Bug 440783 raised for that. Not only is this an inappropriate exception type to use, but it makes it appear that the entire resource was missing, not just an object in the resource. In any case, it's ridiculous to throw an exception on an unresolved proxy, which is a perfectly normal condition in EMF, so I've fixed that in commit b5a3c1d18812b8698b84c284be845ed6df0e80b5. Now you just get as much of the Model Explorer state restored as is pertains to objects that still exist.
(In reply to comment #34) > > https://git.eclipse.org/r/30701 This Gerrit patch set is now merged, fixing the problem in comment 11 and other Save All scenarios (such as popping up a prompt-to-save dialog for editors that should already be saving). It also makes handling of deleted resources consistent with the handling of changed resources. So, all outstanding follow-up problems are now fixed except for the SWT/GTK3 scrolling problem on Linux and the refreshing of visible-but-not-active editors, which perhaps still needs follow-up discussion or documentation, which should be tracked separately. Please raise any further issues as new bugzillae. Thanks!
(In reply to Christian W. Damus from comment #45) > (In reply to comment #34) > > So, all outstanding follow-up problems are now fixed except for the SWT/GTK3 > scrolling problem on Linux and the refreshing of visible-but-not-active > editors, which perhaps still needs follow-up discussion or documentation, > which should be tracked separately. FYI, i'm running Ubuntu 12.04LTS with GTK 3
Just testing this on Papyrus SR1 and I see Comment #12 wasn't fixed.
Created attachment 248039 [details] Screen capture of property sheet context in Save All scenario (In reply to comment #47) > Just testing this on Papyrus SR1 and I see Comment #12 wasn't fixed. What precisely are the steps to reproduce the problem from comment 12? I'm trying to reproduce the issue, but I'm not seeing any problem. See the attached which demostrates what I am observing.
Using the test model attached from before running on Windows 7 today: 1) Open the diagrams of model Z and Y. 2) Click on the background of the diagram in Model Y i.e. the package context The properties view is now showing for the Package Y 3) Click on the Class Y89 in the Model Explorer The properties view is now showing for the Class Y89 4) Click on the diagram Z1.di 5) Click on the diagram modelY.di The focus is on Package Y but it should be on Class Y89. Papyrus has forgotten if I had either the Diagram/Model Explorer context selected. Note: I did not perform any save operation. It seems this is broken regardless of save or not.
(In reply to comment #49) Ah, OK. Now I understand. This works as designed (by Eclipse Platform). The thing is that at step 5 you are activating the modelY.di editor. The Properties view is a slave to whichever workbench part is the active part, so it shows the current selection in the part that is now active: the diagram editor. At step 3 you made the Model Explorer the active workbench part, which is why the property sheet then showed that part's selection. This is all unrelated to the in-place reloading of editors and is the way that it is supposed to work.
Ok that is fair. I was concerned we had missed a use-case or a regression had occured.
