Bug 112225 - [WorkbenchParts] Need consistent save lifecycle when multiple parts share the same model
Summary: [WorkbenchParts] Need consistent save lifecycle when multiple parts share the...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P3 enhancement (vote)
Target Milestone: 3.2 M6   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks: 112215
  Show dependency tree
 
Reported: 2005-10-11 11:10 EDT by Dusko CLA
Modified: 2016-06-11 07:01 EDT (History)
12 users (show)

See Also:


Attachments
Model explorer example (64.73 KB, application/zip)
2006-02-13 14:10 EST, Nick Edgar CLA
no flags Details
patch against org.eclipse.ui.workbench (40.71 KB, patch)
2006-03-27 15:22 EST, Boris Bokowski CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dusko CLA 2005-10-11 11:10:12 EDT
The interface was implemented in 3.1 as response to numerous requests to have 
non-editor based Save participants. That mostly meant views but in some cases 
it also meant non-view based participants.

First of all, the interface still forces the participants to be either views 
or editors; it does not provide for non-part participants.

However, the worse problem is that it provides for no synchronization 
mechanism between various participants. The platform just blindly invokes the 
new method and gives no context to the parts. That means if the resource is 
displayed in multiple parts (say an editor and some kind of navigator view) 
they cannot function properly – it is up to those parts to synchronize their 
behavior. The actions in which the synchronization would have to be done 
(Save, Close, etc.) do not provide for that either.

Here is an example:
1) There is some resource with complex content
2) The user opens editor for part of the content
3) The user opens editor for another part of the content
4) The user explores the whole content in a navigator like view
5) The user makes modification to the resource through the navigator view
6) At this point, the view is ‘dirty’ but so are both editors

The current definition of the ISaveablePart2 interface and its usage in the 
Platform is not sufficient to handle this case. Not to mention that the user 
might want to close both editors and the view but still be prompted one hour 
later when closing the application that there is a ‘dirty’ resource.
Comment 1 Paul Webster CLA 2005-10-12 08:39:26 EDT
(In reply to comment #0)
> The interface was implemented in 3.1 as response to numerous requests to have 
> non-editor based Save participants. That mostly meant views but in some cases 
> it also meant non-view based participants.

Do you have the bug numbers or documents describing the "feature" that added
ISaveablepart2?

> First of all, the interface still forces the participants to be either views 
> or editors; it does not provide for non-part participants.

True, it's still bound to IWorkbenchPart (editor or view) ... I was surprised to
see that it worked on views as well as editors.


PW
Comment 2 Dusko CLA 2005-10-12 09:58:02 EDT
I don't have the bug numbers or documents for the "feature" since I did not 
participate in those discussions. I found those bugs while I was searching for 
ways to save a view or (even better) a resource without part. Just do a search 
in Bugzilla with keywords like 'Save', 'View' or 'ISaveablePart' and there 
should be a few...

The new interface ISP2 simply does not go far enough. Theoretically, it does 
enable views to participate in the Save cycle but it does not provide enough 
in terms of synchronization mechanisms leading to problematic UI. Here is 
quote from its javadoc:

"Note that if a part implements this interface, it is excluded from the 
common "prompt to save" dialog, and instead opens its own dialog.  This may 
cause multiple prompts to the end user during a single user operation. 
Implementors should be aware that this may lead to a less than optimal user 
experience."

I'd say that is an understatement. It would be ideal if the save participant 
would not have to be a view/editor so that a single resource displayed in 
multiple parts (in our case a navigator view, main editor and a number of 
auxiliary editors) could be controlled from a single place. At the very least, 
there should be some way for those parts to communicate with each other during 
the save operation.

Also, bug 112222 additionally hampers the usability of the interface.
Comment 3 Paul Webster CLA 2005-10-18 10:33:35 EDT
> First of all, the interface still forces the participants to 
> be either views  or editors; it does not provide for non-part
> participants.

The workbench page controls the part lifecycle ... if it's not a part, it has no
lifecycle.

I'm changing this to an enhancment, since ISaveablePart2 is definitely
restricted to workbench parts.

PW
Comment 4 Dusko CLA 2005-11-07 15:39:00 EST
I disagree with this being changed to an enhancement. This is still a major 
issue for us that were supposedly solved in 3.1 by providing the 
ISaveablePart2 interface but this interface simply does not go far enough.

I can accept the part for non-part (view or editor) save participants being an 
enhancement but, as I already stated in the Bugzilla, that is the less 
important part of the problem.

I’ll quote the interface’s javadoc again: 

"Note that if a part implements this interface, it is excluded from the 
common "prompt to save" dialog, and instead opens its own dialog.  This may 
cause multiple prompts to the end user during a single user operation. 
Implementors should be aware that this may lead to a less than optimal user 
experience."

It has severe synchronization problems. Just look at the example I had in the 
initial submittal:
1) There is some resource with complex content
2) The user opens editor for part of the content
3) The user opens editor for another part of the content
4) The user explores the whole content in a navigator like view
5) The user makes modification to the resource through the navigator view
6) At this point, the view is ‘dirty’ but so are both editors

The current infrastructure does not give enough to the client code to handle 
this. Say that the user closes one editor – the editor code does not now if 
the app is being closed or one of the close editor actions has been invoked 
(close, close all, close others). It does not have enough information to 
determine if it should prompt the user to save or not. Say that it was close 
all – the user will get prompted twice. That is bad. Not to mention that in 
this case the user most likely should not be prompted at all since there is a 
view still open that can save the same resource.

Another thing is that it really works just for some internal Eclipse code 
(like handling the close operations). It does not expose any APIs to the 
client code that might have to perform the save operations on those parts. 
Say, for refactoring you need to save all open parts. Well, there is no API 
that will take into account ISP2 properly. You can query dirty parts and 
invoke save but that is not the same. ISP2’s method to prompt on close is not 
applicable – the parts are not being closed.

The 3.2 work on logical/physical resources is solving a different problem and 
does nothing in this area. I do believe that this bug should be re-classified 
as a bug and not an enhancement and another Bugzilla opened for the 
enhancement portion.
Comment 5 Paul Webster CLA 2005-11-07 17:57:14 EST
See bug 76768 for the previous history of ISaveablePart2.

PW
Comment 6 Dusko CLA 2005-11-08 11:00:43 EST
The discussion in bug 76768 makes a lot of sense. If I had one editor or one 
view per resource then the level of support provided in ISP2 would be close to 
sufficient and my impression is that people interested in the 76768 had such 
configuration.

However, my problem is that I do have multiple editors and views displaying 
content from the same resource. That situation is different – ISP2 simply does 
not provide enough for this use case. It is missing synchronization 
capabilities; the views and editors cannot sync their behavior and they should 
or it leads to the UI problems (please see examples in my previous comments).

The other problem is that there is no way to unify the prompt/save behavior on 
other operations than close operations (please see comment #4). The closest 
API that I found that would solve this problem is the SaveableHelper class but 
it is internal in org.eclipse.ui and cannot be used in the client code.

Please take a look at bug 84406. This is a continuation of the same problem 
that Anthony was interested in on our side (I added Anthony to the CC list for 
this). However, at the time our app was on Eclipse 3.0 and we could only test 
it in artificial environments and ‘green thread’ scenarios. After integrating 
with 3.1 and actually using the implemented stuff (including ISP2) here are 
the major issues that prevent us from really using it:
1 – Lack of communication/sync mechanism between the parts participating in 
the save.
2 – Inability of the part to tell up-front why the save is being invoked (for 
ISP2’s promptOnClose): close view/editor, close all, close others, close app. 
Or for the isDirty() call – if we know that it is invoked in context of action 
(Save, Save All) enablement or context of prompting on close, that would help 
too.
3 – No public utility to unify view/editor hunting for saving (SaveableHelper 
is internal).
Comment 7 Paul Webster CLA 2005-11-08 11:52:22 EST
(In reply to comment #4)
> 
> The 3.2 work on logical/physical resources is solving a different problem and 
> does nothing in this area. I do believe that this bug should be re-classified 
> as a bug and not an enhancement and another Bugzilla opened for the 
> enhancement portion.

This is definitely an enhancement.  This request falls outside of the current
Editor and View lifecycle, and ISaveablePart2 does match the requirements of bug
76768 it was designed to solve.  There's definitely room for improvement in
ISaveablePart2, like the ability to "batch" custom save dialogs so user can
won't have multiple dialogs surfacing on a close all, one after the other.

As for your parts' need to communicate, that's what your model is for.  An
example of this is the text editor.  As each editor is opened, it generates a
document for that input:

documentProvider = getDocumentProvider();
document = documentProvider.getDocument(editorInput);

If 2 different editors are opened on the same IEditorInput they share a
synchronized model.  Changes in one editor update in the other editor, and it's
the model's responsibility to synchronize the parts.  If you close a text editor
it asks you to save, and if you do the model is updated so when the other editor
closes it doesn't ask you to save.

Now I was talking to Nick, and he mentioned that you're already doing this (in
spades).  He also mentioned Anthony had a "document on 'saving' use cases"?
related to saving views in eclipse that the UI team had looked at in 3.1. 
Anthony, if you still have that available, I'd like to take a look at it.

PW
Comment 8 Anthony Hunter CLA 2005-11-08 15:10:45 EST
I cannot post the document here as it contains proprietary information, I will 
email it to IBM parties.

Comment 9 Dusko CLA 2005-11-09 10:11:40 EST
We do not have a problem with model going out of sync because of multiple 
parts. That's never been the issue. The only problem that we have is 
controlling the life cycle of parts themselves. Even with that our model still 
stays 'in sync' and everything is fine - semantically. But the end user's 
experience is far from optimal because we cannot tweak the part life cycle to 
really work well in case where multiple parts (views and editors) display 
different perspectives on the model or even parts of the model. As for the 
text editor example, that is way too simplistic to cover our use cases and I 
do not think that it applies to this topic as an analogy.

Please take a look at the example: I have 3 parts (a view and 2 editors) and 
my model is 'dirty'. Each part has save capability. Additionally, there are a 
couple of other, regular, 'dirty' editors unrelated to our model and parts. 
Let’s think about following use cases:
1) The user closes one of our parts. We do not want to prompt since our model 
is represented with other two parts that will stay open.
2) The user closes all editors. We do not want to prompt since our model is 
represented with the view that will stay open.
3) The user closes application. We do want to prompt, once and as part of the 
Eclipse's dialog notifying the user about the non-saved resources together 
with other resources.

Currently, we cannot get that behavior no matter what we do. When the part is 
closed, Eclipse first invokes isDirty() then isSaveOnCloseNeeded() and then 
finally promptToSaveOnClose(). The part cannot determine during any of those 
calls if other parts participate in this procedure. That is, even though our 
part is aware of the two other parts representing the same model, it has no 
information if they will be closed as well or not. At the time when that 
information becomes available it is already too late to change behavior of the 
3 above mentioned calls. The best we could do was to prompt when any of our 
parts is closed and limit that to one prompt, regardless of how many parts are 
being closed. And, of course, that part has to invoke its own prompt - it 
cannot participate in the common dialog which would be the preferred way. Not 
to mention that the code limiting it to one prompt is really a hack and not 
something supported through Eclipse API. I say hack because it requires 
knowledge of the exact algorithm Eclipse applies when closing parts. That 
algorithm is not documented in javadoc (and it shouldn’t be) but that also 
means that it can easily be changed by Eclipse which would break our code.
Comment 10 Nick Edgar CLA 2005-11-28 17:15:12 EST
I met with Dusko today and have a better understanding of the problem.
Comment 9 is a good summary, but it helped to see the issues in action in the real product.

Basically they want the prompts to save on close (or shutdown) to only prompt when the last view or editor on the model is going to be closed, and the prompt should be in terms of the name of the model (and/or its underlying primary file, TBD), not the name of the editor.

They also want similar behavior for other actions that trigger saves, like refactoring.

Ctrl+S should save the whole model when triggered from any view or editor showing the model, including supporting views like the Properties view and navigation viwes like the Common Navigator.

For the Common Navigator, although it shows the structure of the open model, it does not support Ctrl+S because the view does not implement ISaveablePart.
However, we don't want the view to always act as if it's saveable, e.g. if none of the shown content extensions have an unsaved state.
The expected behaviour of Ctrl+S and dirty flags in the Common Navigator are:
- view tab shows '*' if any content extension has unsaved changes
- top-level element showing the model has '*' suffix if it is open and has unsaved changes
- File > Save, or Ctrl+S, saves only the model(s) containing the selected element(s).  The '*' suffix on the model(s) will be cleared, however the '*' on the view tab may remain if there are remaining models with unsaved changes.

For the Properties view, Ctrl+S should work the same as for the source of the properties.  E.g. if showing the properties for a model element selected in the Common Navigator, it should save that model.

I believe the desired behaviour above is common for many use cases, not just for their product.
Comment 11 Nick Edgar CLA 2005-12-06 11:05:45 EST
Here is a high level summary of the problem, a first cut at guidelines we'd like to support, and a summary of the known issues in the workbench that prevent us from implementing these guidelines.  Comments are welcome.

Applications often allow different aspects of the same model to be shown and edited in different workbench parts (editors and/or views).
Users expect intuitive and consistent handling of the 'Save' command (File>Save, or Ctrl+S) across all parts where the model is shown.
This includes appropriate handling for supporting views like the Properties view, and generic navigation views like the Common Navigator.

The following are some guidelines that should be implemented in order to respect user expectations:
* When there are unsaved changes to the model, this is indicated by a dirty flag (*) in the part tab representing the model.
* If a part can show the unsaved state of multiple models, then items representing models with unsaved changes show a dirty flag, in addition to the part tab's dirty flag.  The part tab shows the dirty flag if any contained models are dirty.  The 'Save' command affects only the selected model(s), not all models in the part.
* The 'Save' command is enabled if and only if the model(s) represented by the active part and its selection have any unsaved changes.
* The 'Save' command saves the model containing the focus control (and no other models), and clears the dirty flag.
* If the model supports nested models, only the innermost model(s) represented by the selection or focus control should be saved.
* The 'Save All' command saves all models with unsaved changes, regardless of focus, but limited to models shown in the current window.
* If multiple parts are showing the same model, then closing one part does not require a save and the user need not be prompted, though the application may choose to prompt in this case.
* If multiple parts must be saved (e.g. due to Exit, Close All, or before a refactoring) then the prompt-to-save dialog should show only a single item for each affected model, not separate items for each part showing the affected models.

Special handling is required for views showing the model, especially for general purpose views like the Common Navigator and for supporting views such as the Properties view.

In the case of the Common Navigator, the content extension for the model may show the inner structure of the model being edited (not just a single element representing the model as a whole).  The Common Navigator must handle the 'Save' command, as per the guidelines above, if it contains any content that may have unsaved changes, even if there are currently no unsaved changes.  Otherwise, 'Save' would remain enabled after running if the active editor had unsaved changes, and a subsequent 'Save' would save the editor's contents.  However, if the Common Navigator does not contain content that may have unsaved changes, 'Save' should work as before and be handled by the active editor.

The Properties view case is similar.  It must handle the 'Save' command if it is showing properties for a model that may have unsaved changes, even if there are currently no unsaved changes.

The Outline view case is similar to the Properties view case, but this is currently less problematic because both the Outline view and the 'Save' command track the active editor by default.

The following known issues in the workbench prevent applications from implementing the above guidelines:
# Although views can participate in the save lifecycle by implementing ISaveablePart, this is an all-or-nothing implementation choice, which is insufficient for the Common Navigator and Properties view cases.
# View tabs do not show the dirty flag.
# The prompt to save on close or shutdown is not model-aware.  If multiple editors are open on the same model with unsaved changes, the prompt-to-save dialog lists each editor separately.  It should instead list the model at most once.  The prompt should also support the case where closing the affected parts does not require a save of their model, e.g. if the model is still visible some other open editor or view.
# The workbench support for prompting to save is not exposed as API, resulting in inconsistent treatment from downstream plug-ins that need to ensure that all models that may be affected by a pending operation are saved first (e.g. JDT's refactoring support).  There is non-trivial logic here that downstream plug-ins should not be expected to copy.

Even with the current workbench support, unexpected and unwanted behaviour can result if parts do not properly participate in the save lifeycle:
# If the active view shows the unsaved state of a model, but does not implement ISaveablePart, then 'Save' is still targeted to the active editor, not the view.  Because the view is showing unsaved state, the user expects 'Save' to save the model being edited, but the actual result is that the active editor is saved.  The editor may be completely unrelated to the model, and may contain changes that the user did not want to save.  In the worst case, this can result in data loss due to the save overwriting the stored contents earlier than desired.

The following are some open issues regarding save lifecycle:
* How should 'Save All' work when parts containing unsaved changes are not visible (e.g. in other perspective)?  We currently believe that Save All should save all models with unsaved changes in the current window, even if the part(s) showing the model are not visible in the current perspective.
* How should Close work when other parts showing the same model as the part to be closed are not visible?  In the past, text editors did not prompt to save on close if the editor was open in another window.  This was contrary to user expectations.  Since 3.1, multiple editors can be open on the same model in the same window (e.g. using Window > New Editor).  Text editors still prompt in this case, but perhaps this is not desired.  Perhaps the scope for checking whether a save on close is needed should be limited to the current window.  This would be consistent with the behavior of 'Save All'.  But this still may be confusing, e.g. if a view in the same window shows the same model but is not currently visible.  This case only arises for views, since editors are shared across all perspectives in the same window.
Comment 12 Dusko CLA 2005-12-13 09:52:14 EST
Comment 10 and Comment 11 represent a sound design and would definitely help us to solve our problems. I find paragraphs on Common Navigator and Properties View especially useful. There is a separate bug 112215 ([CommonNavigator] Support the ISaveablePart2 interface) on Common Navigator but I believe it is really good that we have the whole effort driven from a single place. That is, from this entry, since here we are dealing with part lifecycle issues in general.
Comment 13 Nick Edgar CLA 2006-01-05 11:20:17 EST
Dani points out that the suggestions above for dirty indications ('*') is a bit too strong, at least for the current JDT scenarios.

Here are some excerpts from a chat I had with him:

Daniel Megert - it says that parts showing dirty model should show the '*'. Should this be done for Outline view as well? Package Explorer?
nick_edgar - Well, my starting position is yes.  But for the Outline it doesn't seem to add much value.
nick_edgar - Does the PE show unsaved changes?
Daniel Megert - Yes.
Daniel Megert - it sits on the working copy model
Daniel Megert - This would mean that you have a '*' almost all the time in the tab
Daniel Megert - Currently you cannot tell whether a file is dirty in the Package Explorer. We've once written an experimental decorator that marks the open CUs bold.
nick_edgar - Was that helpful?
Daniel Megert - Not really since the foucs for dirty files is on the editor (list)
Daniel Megert - But save is available in the PE
Daniel Megert - Other views have the same issue, e.g TypeHierarchy, Types view etc. 
Daniel Megert - The question is whether the guideline should tell to use the '*' for view parts
nick_edgar - These proto-guidelines were derived from what I saw going on in (unnamed product).  It's like the Navigator in that it shows projects/folders/files, but also elaborates the contents of model files.  When one is open and has changes it shows a '*' in the label.  It seems like an omission for the view not to have a '*' in this case.
Daniel Megert - I just wanted to mention that most JDT views don't follow that guideline
nick_edgar - It's slightly different than JDT in that they allow all editors to be closed without saving the model.  So you need an indication that there is unsaved state in the view.
Daniel Megert - I agree on that one.
nick_edgar - So maybe the heuristic should be "if changes can get lost when the view is closed, it should show a '*'"?
Daniel Megert - Yes, that sounds better and also fits the Outline view which can be closed without losing data.
nick_edgar - It's also not an exact mapping to what happens with editors.  I can have multiple editors open on the same file.  It's only when closing the last one that changes could get lost, but we want '*' on all of them.
nick_edgar - I think the distinction is whether the view is a primary editing view, or is just a supporting view.
Daniel Megert - mmh - yes, that's true.
Daniel Megert - Note: I'm not against introducing the (*) on views - just wanted to make sure we don't post guidelines that the SDK isn't following and putting pressure on us to adopt these
nick_edgar - Right.  In their world they have to explicitly open the model before editors can be opened.  But in any case, I don't think it's unreasonable to allow showing and editing the model in views.  I know in the past the workbench has tried to enforce the convention where editors do editing (open/edit/save/close lifecycle) and views just allow navigation (Navigator, PE) and show structure etc (Outline, Properties). But as we've seen with JDT and other apps, this is a bit artificial.  The user just wants to edit their model in the most appropriate way.  Sometimes it's via an editor, sometimes via a view.  I'd like to provide a consistent user experience for this.  But I agree the first cut is a bit too strong on the use of '*' in views.
Comment 14 Nick Edgar CLA 2006-01-11 11:28:11 EST
Dusko, could you please elaborate on whether the point "it does not provide for non-part participants" is still important for you?  Given the proposal above, I would expect a prompt to save on close when you close the last part showing the model.  The proposal would not handle the scenario where "the user 
might want to close both editors and the view but still be prompted one hour 
later when closing the application that there is a ‘dirty’ resource."
Comment 15 Nick Edgar CLA 2006-01-11 11:54:01 EST
See also bug 71701, which requests the ability for plug-ins to veto shutdown.
It seems like most cases reported there would be addressed by either the current support for views implementing ISaveablePart (modulo the other limitations expressed here).
Comment 16 Dusko CLA 2006-01-11 12:16:13 EST
I added my comments to the bug 71701. Resolving that bug would be ideal because we do have use cases where that behavior is desired. An example is cross-referenced resources when parts for one resource get closed and not the other.

However, that is not as big priority since we can deal with this scenario in a bit less graceful way: if the user closes parts for resource A and resource B requires A then we would have to inform the user that we are going to close B parts as well.
Comment 17 Nick Edgar CLA 2006-01-11 17:13:13 EST
Dusko, could you please clarify the desired behaviour for when a view showing multiple models is closed?  E.g. if I have models A, B and C open, with no editors open, and I shut down, or close the model explorer view.  What would you expect the prompt to look like?
Comment 18 Nick Edgar CLA 2006-01-25 14:48:06 EST
Dusko, could you also clarify the expected behaviour of Save All?  Should it save all open models if the Model Explorer is showing?
Comment 19 Dusko CLA 2006-01-25 15:37:52 EST
Comment #17:
I would expect prompt for all 3 open models.

Comment #18:
It should save all open models.
Comment 20 Nick Edgar CLA 2006-01-26 16:06:47 EST
Filed bug 125386 for the Properties view's handling of Save.
Comment 21 Nick Edgar CLA 2006-01-26 16:22:02 EST
Views now show their dirty state in the tab (see bug 84407).
Comment 22 Nick Edgar CLA 2006-01-31 09:52:09 EST
I've released the following support for this problem to HEAD in the generic workbench layer (org.eclipse.ui.workbench).  

- Added interface ISaveableModel.  This represents an editable (and saveable) subset of the domain model, having presentation properties (text, image, tooltip) and doSave and isDirty lifecycle methods.

- Added interface ISaveableModelSource.  If a part implements ISaveablePart and ISaveableModelSource, then it can provide the ISaveableModels managed by the part via getModels().  It can also provide a more restricted list of "active" models (see the note on the Save action below) via getActiveModels().

- The Save action now checks for ISaveableModelSource on the active part, and enables if any active documents are dirty (in addition to the old behaviour).  A view showing multiple models (such as a model explorer view) may only want Save to target the currently active documents (i.e. models represented by, or containing, the selected elements) rather than all models managed by the view.  Therefore, the Save action tracks getActiveModels() rather than getModels().  Parts managing a single model should return the same for getModels() and getActiveModels().

- The Close and Close All actions now check for any dirty documents (using getModels(), not getActiveModels()).  If multiple parts sharing the same model(s) are closed, then the user is only prompted once for the model rather than for each part.  If the model will remain visible in any parts in the same page that are not being closed, then the user is not prompted to save changes.  Prompts to save when closing the window and shutting down work similarly.

- The view tab now shows the dirty state indicator (*) when the view implements ISaveablePart and isDirty() returns true (bug 84407).  The dirty state indicator deliberately does not track the dirty state of models.  It's up to the part to properly reflect its own dirty state, taking into account any models it presents.  Normally a part would return true for isDirty() if any models it presents are dirty.

- There is no new lifecycle for triggering updates of the dirty state indicator and the enablement of Save actions.  Parts still need to firePropertyChange(ISaveablePart.PROP_DIRTY) if the dirty state changes on the part itself or on any models it presents.

I have an EMF-based example showing how this works, which I will post after a bit of cleanup.  Note however that the ISaveableModel support is not tied to EMF or the workspace model (IResource).
Comment 23 Nick Edgar CLA 2006-02-13 14:10:54 EST
Created attachment 34601 [details]
Model explorer example

The attached zip file contains 4 projects defining a simple "model explorer" example.  It requires EMF.

The projects are:
org.eclipse.ui.examples.modelexplorer
- EMF model and code generated from it

org.eclipse.ui.examples.modelexplorer.edit
- EMF edit code generated from the model

org.eclipse.ui.examples.modelexplorer.editor
- EMF editor code generated from the model

org.eclipse.ui.examples.modelexplorer.ui
- Model Explorer view

To install:
- install EMF to your host eclipse if you don't have it already (see http://eclipse.org/emf)
- save attachment to a .zip file (do not extract)
- import the projects into your workspace using File > Import > General > Existing Projects into Workspace > Select archive file
- launch a target Eclipse

To illustrate the saveable model support, try:
- create a simple project via File > New > Project... > General > Project
- create a model explorer file in the project via File > New > Other... > Example EMF Model Creation Wizards > Modeler Model
- this opens an editor on the model, or you can double-click on the file in the Navigator or Package Explorer view to open an editor
- right click in the editor and choose Show Properties View (required for most EMF editors)
- make some changes to the model, e.g. select the Model element and choose New Child > Diagram from the context menu
- try closing the editor
- you get the prompt to save on close
  - Cancel this
- open another instance of the editor by choosing New Editor on the editor tab
- try closing the second editor
- it closes without a prompt because the model is open in another editor
- show the Model Explorer view via Window > Show View > Other... > Model Explorer (Example)
  - drag the view by its tab to the left pane to give it more vertical space
- in the Model Explorer view, 
  - expand the project and the model file
  - it shows the structure of the open model
  - when a dirty model or one of its sub-elements is selected, the File > Save action is enabled
- without saving changes, try closing the remaining model editor
  - it does not prompt to save, because the model remains visible in the Model Explorer view
- close the Model Explorer view
  - it prompts to save the dirty model(s)
Comment 24 Nick Edgar CLA 2006-02-13 14:11:40 EST
There are a few update bugs in the current support.  Moving to M6.
Comment 25 Dusko CLA 2006-02-17 11:38:15 EST
The new implementation looks really good. I have only usability issue left.

Consider following example. There is a resource/model opened in 2 editors and a navigational view. The editors are bound to the resource but the view is not - it really represents multiple resources (the Common Navigator view). If I close one editor I definitely do not want to get prompted - I still have other editor opened on the same resource. However, if I close both editors, the things get trickier. There is still a view that shows the open resource/model (Common Navigator) and is capable of saving it. On the other hand, that view is integral part of the perspective and shows other resources as well so it (most likely) won't get closed anytime soon. The only time we are going to prompt the user is when the application is closed, which might be a bit too late.

Ideally, the save framework would determine that one view capable of saving the resource is left around but before concluding that it does not need to prompt, it would consult the view. That way we could have a preference and the end user would be able to dictate if they want to be prompted when the last editor for the resource is closed even though Project Explorer is still around.
Comment 26 Boris Bokowski CLA 2006-03-27 15:22:46 EST
Created attachment 37018 [details]
patch against org.eclipse.ui.workbench
Comment 27 Boris Bokowski CLA 2006-03-27 15:33:50 EST
This patch adds API for maintaining a list of open models in the workbench.

It changes the contract (but not the signatures) of ISaveableModelSource - implementers must notify an ISaveableModelManager whenever the result of getModels() changes during the lifetime of the saveable model source.

I have talked to Michael Valenta about this, he will update the implementations of ISaveableModelSource in the SDK accordingly.

CC'ing McQ for approval of the API change.
Comment 28 Mike Wilson CLA 2006-03-27 16:34:42 EST
We do want to get this support in, but I trust we are all aware that it is *extremely* late in the cycle to be making these kinds of changes, and act accordingly.

+1. ok to proceed.
Comment 29 John Arthorne CLA 2006-03-29 17:25:38 EST
(for reference, new interfaces IModelLifecycleListener, ISaveableModelManager, and class ModelLifecyleEvent have been released)
Comment 30 Boris Bokowski CLA 2006-03-29 18:57:52 EST
There were concerns that the proposed API looks like a general model sharing mechanism that could be used by clients. This was never intended. To address the concerns, we decided to make the following changes:

. Rename the classes and interfaces so that "model" does not appear in their names, and make it clear in the Javadoc that this is not a mechanism for sharing models. Rather, it is a way to make it known to the workbench when different parts operate on the same underlying model.

old	                  new

ISaveableModel	          ISaveable
ISaveableModelSource      ISaveablesSource
IModelLifecycleListener	  ISaveableLifecycleListener
ModelLifecycleEvent	  SaveableLifecycleEvent

. Do not allow clients to get hold of instances of ISaveable from other parts. This consists of two parts:
  o Remove ISaveableModelManager from the public API. This means that clients have no way of getting the global list of ISaveable objects.
  o Write spec for ISaveablesSource.getSaveables() to say that this method is not intended to be called by clients.

Other than the changes described above, the current proposed API will remain public API, and the common navigator will make use of the API (i.e. implement ISaveablesSource).

Note that we are not trying to solve the (model layer) problem of sharing models between different plug-ins or components.  Clients who have a notion of shared models can use the API to make the UI layer aware of sharing across multiple workbench parts.

If clients have shared models, but do not use the new API, the result will be what you see with text editors today (see bug #131068): users will be prompted for saving, but when they click "No" their changes will still be there.
Comment 31 Boris Bokowski CLA 2006-03-30 01:29:28 EST
I released the changed API for I20060330-0010.
Comment 32 Boris Bokowski CLA 2006-03-30 08:53:20 EST
Released the removal of ISaveableModel and ISaveableModelSource.
Comment 33 John Arthorne CLA 2006-03-30 09:56:50 EST
Boris, is ISaveablesSource a typo (extra s)?
Comment 34 Michael Valenta CLA 2006-03-30 10:17:41 EST
The API changes in workbench have lead to the need to change API in Team. Here is a summary of what changed:

- ISavealeCompareModel interface removed since ISaveable was changed to a class.
- SaveableCompareModel class renamed to SaveableComparison which now extends Saveable.
- Method parameter types changed and methods renamed on several classes to reflect the removal of the term "model" from the workbench API. The changed API classes are:

    ISynchronizationCompareInput
    MergeActionHandler
    SynchronizationOperation
    ModelParticipantAction
    ModelSynchronizeParticipant
    
Comment 35 John Arthorne CLA 2006-03-30 10:24:01 EST
Please ignore comment #33 - ISaveablesSource is not a typo.
Comment 36 Boris Bokowski CLA 2006-04-11 17:13:30 EDT
This was released for 3.2 M6.
Comment 37 Boris Bokowski CLA 2006-06-02 15:38:42 EDT
Verified that this is in I20060602-0010.