Community
Participate
Working Groups
Build ID: 3.4 Steps To Reproduce: When a user closes an editor from Eclipse using the menu or by clicking the "x" on the tab, the workbench checks whether the editor is "dirty" and gives it a chance to take a pre-close save action (doPromptToSaveOnClose). If the editor isn't dirty, Eclipse closes the window in the UI and then notifies the EditorPart object that it's been closed. We need a third option. The programming model for our RCP application lets you have event code in a workbench part that executes on what we refer to as "Queryclose", and the code has the option to abort the close operation. This code is executed whenever a request is made to close the part, whether or not the client considers the contents "dirty." We could fix this problem by flagging document and view windows that contain Querysave code as dirty when they are opened -- even if opened in read mode -- but this has consequences in the UI -- the "save" toolbar button is activated and the tab gets a "*" on it -- and this is not appropriate for windows that are opened in read mode or that have no saveable changes -- they aren't really dirty. We would like to propose a change to the Eclipse part API that would let the EditorPart or ViewPart have a say in whether it is to be closed regardless of the dirty state. More information:
This sounds like something Erics modeling work might want to capture...
Matthew, can you give me a scenario? I'm having trouble visualizing exactly what you're asking for...is it just to prevent a User from 'accidentally' closing an important view? How many views do you see having this attribute? Under which conditions would you (the User) expect to be prompted: 1) Closing the view itself...surely 'yes' 2) Closing the perspective containing the view? 3) Closing the app? 4) Closing a Detached Window containing the view? 5) Closing a WorkbenchWindow (but not the session)? The answers will indicate how close to the current savable life-cycle we are. My initial guess is that this is indeed something different. I'm all in favor of a 'veto-able' close; it's just a question of defining its interaction boundaries. Kevin, what are your thoughts on this?
Matt, Paul just pointed out that you're talking editors while my response is talking Views...while not intentional it's still appropriate because we now have 'savable' views as well.
Re. comment #3 Right, whatever we do should work for both views and editors. I tried to be clear about that in the description. Re. comment #2 >> Matthew, can you give me a scenario? is it just to prevent a User from 'accidentally' closing an important view? Yes. Some parts may want to prompt the user with a "Are You Sure?" message. >> Under which conditions would you (the User) expect to be prompted: This is not really what we're looking for, it's more like >> Under which conditions would you (the WORKBENCH PART) expect to be given the CHANCE TO VETO THE CLOSE: 1) Closing the view itself...surely 'yes' 2) Closing the perspective containing the view? 'yes' 3) Closing the app? 'yes' 4) Closing a Detached Window containing the view? 'yes' 5) Closing a WorkbenchWindow (but not the session)? 'yes'
Matt, can you provide the scenario which prompted this? On the one hand, I generally believe RCP apps should be able to do whatever they want. However, depending on how we do the API it could be usable by views/editors that aren't in RCP apps. This then worries me: Right now we have a specific lifecycle for editors, and we have an affordance to support that lifecycle (the * dirty indicator). The user can see they have unsaved changes, closing in that state prompts user associates the feedback * with the prompting and the underlying state, knows that saving before close avoids the prompt, etc. What you've asked for first of all implies there's additional state in the lifecycle (that state will be checked from your proposed query). In the IDE currently we "own" the editor lifecycle. The idea of opening this up so different editors can have additional lifecycle behaviour is a concern wrt consistency in the IDE. Second, your request doesn't address the issue of feedback, like the * currently does. I agree btw, with your comment #0 that we shouldn't overload *. Because of the consistency in the IDE issue, we need to understand the usage case better. One way to avoid this is to add the API to the WorkbenchAdvisor, letting the IDEWorkbenchAdvisor maintain the current behaviour but allowing RCP apps to extend. But even if we went that route I'd like to understand if this is something we should or should not allow on a per editor basis within the current IDE.
Re: comment #5 Our RCP application (Lotus Notes 8) is scriptable whereby scripts can participate in lifecycle events such as "OnLoad", "OnFocus", "QueryClose" etc. Some of these events are specific to the Lotus Notes Forms framework and others like "QueryClose" rely on some underlying support from Eclipse RCP. Without the feature that is being requested in this Bugzilla report the "QueryClose" event handling does not work as expected in Lotus Notes 8. This is a significant regression. Maintaining tight control over the part life-cycle may have it's advantages for the IDE. But I think this is something that should at least be configurable by the RCP level bits.
The open ended nature of this worries me for the IDE. But for RCP, I see no problem, go wild! :) Paul/Eric, what would you think of RCP specific API in say WorkbenchAdvisor? On the surface it seems an odd place for the API, but then I think the right model is for such "policy" kind of behaviours to be configurable at the RCP level only.
Kevin, what type of method do you envision? We could do something along the lines of WorkbenchAdvisor#okToClose(IWorkbenchPart part). This would allow us to get away without have to add extra 'okToSave' state to the Editor/View parts themselves but means that the writer of the advisor has to manage which of their parts work this way (and under what conditions). I agree though that this is an odd construction...
(In reply to comment #8) > Kevin, what type of method do you envision? We could do something along the > lines of WorkbenchAdvisor#okToClose(IWorkbenchPart part). This would allow us > to get away without have to add extra 'okToSave' state to the Editor/View parts > themselves but means that the writer of the advisor has to manage which of > their parts work this way (and under what conditions). Yes. But Matt, do you in fact need this on views too? Your description is mostly about editors but then at the end of comment #0 you mention views. We need to consider whether we really want to open it wider than editors. While its true I can save from the view, there is no lifecycle and therefore I'm not sure this is the way to introduce one. Finally, we need to discuss about sequence. I assume that this is a veto. We now have two conditions of veto of close (with feedback): editor is dirty, and then this new query. I assume if the editor is dirty, then this is the first condition checked. So we have these conditions: 1) Editor is clean :: okToClose is queried 2) Editor is dirty, user is asked to save changes Yes/No/Cancel Yes :: okToClose is then queried No :: okToClose is then queried (this is essentially like user first did Revert then its case (1)) Cancel :: nothing called
>> But Matt, do you in fact need this on views too? Your description is mostly about editors but then at the end of comment #0 you mention views. We need this for views and editor parts.
Traditionally all of our listeners are "post" listeners. partOpenend(*), partClosed(*), windowClosed(*), etc. We stuck to that for many of the reasons mentioned here, but it has lead to some unfortunate consequences (i.e. the widespread use of ISaveablePart2 simply to prevent part closing). Maybe we should consider something similar to org.eclipse.ui.IWorkbenchListener (added in 3.2). IPartListener3 (snicker, ignore the name for the moment) adds boolean partOpening(IWorkbenchPartReference) and boolean partClosing(IWorkbenchPartReference). Returning false from either method will abort the part opening or closing in normal operations. But not during a hard close (during shutdown or a forced window close). There will be no error logged during either veto (but we will add trace statements for the .options file). Since the return value can sometimes be discarded, we can (via a number of ways, either preference or workbench advisor method) allow the application to select if it honours the veto. We could turn it loose in the IDE along with a "I'm a confident user so just blow the part away" command :-), or leave it restricted to RCP apps. The important thing is this is orthogonal to save (and doesn't interfere with save's current operation). PW
While I agree that the new functionality is orthogonal to the current dirty/save cycle I'm pretty sure that we want the UI handling to dove-tail. If I have a combination of dirty parts (either editors or views) and ones that participate in the close 'veto' then I, as a user, certainly don't want to get multiple, separate prompts...even worse if they happen to be the same part..."your editor is dirty, would you like to save" followed by "Did you really, really mean to close this editor" would quickly send me running for the Tylenol... I think we need to walk a few scenarios to determine how this new functionality would integrate with our current savable part story. At first blush I'd place this new functionality 'above' the current story; 'dirty' editors/views are really just special cases of things that need a veto-able close. First scenario: User selects "File->Exit" with a mixture of dirty editors, dirty views and assorted parts participating in the close veto strategy. How many different prompts do they get?
To add to Eric's comments, My ideal would be that lifecycle was plugable at the RCP level. We'd provide a default one which RCP apps could replace or extend. Ours would mark dirty editors with the *, prompt on close if dirty, not prompt if emergency shutdown, etc. The notion being that the current lifecycle and dirty editor management just happen to be *our* workbench policy for the IDE. OK we may never actually re-implement it like that because it'd be a tonne of work, but I'd prefer a design that was along those lines for extension. The issue for me with IWorkbenchListener and IPartListner(N) is that anybody can register for them (and in the case if IPartListener2, lots do today). So I wasn't thinking of it in terms of listeners but rather in terms of an object to reify the lifecycle, along the lines of: public abstract class PartLifeCycle { public abstract void partModified(IWorkbenchPart); /** * return boolean true if the part can be closed */ public abstract boolean okToClose(IWorkbenchPart); public abstract void close(IWorkbenchPart); } public class IDEPartLifeCycle extends PartLifeCycle { ... ... public boolean getDirty(IWorkbenchPart); } public class LotusPartLifeCycle extends PartLifeCycle { public void okToClose(IWorkbenchPart part) { if(getDirty(part) && super.okToClose(part)) //notify } } with matching WorkbenchAdvisor.setPartLifeCycle(PartLifeCycle partLifeCycle)
Kevin, I like the concept. How would we control the current "*" adornment policy?
(In reply to comment #14) > How would we control the current "*" adornment policy? In theory (because in practice I don't know if we'd bother refactoring working code) you'd be notified via public abstract void partModified(IWorkbenchPart); which would put the adornment on. Presently we just pre-pend the TabItem label string with * in DefaultTabItem.updateTabText() (ugh), so the direct mapping would be to move that code to the PartLifeCycle.partModified(IWorkbenchPart). In the future you could imagine this controlled by pseudo-class state change and matching CSS styling. Now that I think about it we'd also need: for "Revert". What I like about this approach is that it makes the lifecycle events clear (they're the names of PartLifeCycle methods) and treats lifecycle as a first class entity, which in our case lends itself to extension in a controlled manner. I've always found DefaultTabItem to be an odd class, maybe this is a step to getting rid of it. Hmmm, another thing it does is setBusy(boolean). Again this a state change. It should be the job of PartLifeCycle(), so now the API looks like: public abstract class PartLifeCycle { public abstract void partBusy(IWorkbenchPart); public abstract void partUnBusy(IWorkbenchPart); public abstract void partModified(IWorkbenchPart); public abstract void partUnModified(IWorkbenchPart); public abstract boolean okToClose(IWorkbenchPart); public abstract void close(IWorkbenchPart); } I like it! (Of course we could instead have public abstract void partBusy(IWorkbenchPart, boolean); public abstract void partModified(IWorkbenchPart, boolean); but I kind of like having dedicated methods for each state machine transition). Side note: DefaultTabItem has a very strange mix of state and style methods. For example it has setBusy(boolean) which is the state notion, but it also has setBold(boolean) which just changes the look but doesn't describe why. And the dirty label gets applied via updateText() but really it should've had setModified(boolean). Yet another example of why our current stuff is confusing. In the future with CSS making style first class, we then should look to making state first class.
oops, should've read: > Now that I think about it we'd also need: public abstract void partUnModified(IWorkbenchPart); > for "Revert".
Nice one Kevin! This is certainly on the right track. The concept of formalizing the various life-cycles into state machines and hooking code flow(s) onto the transitions makes everything else more understandable. Now all we need is another FSM for the WorkbenchWindow itself...;-) This one would effectively match the WB states that we currently track to control the Commands infrastructure + it's specific life-cycle (Splash setup/run/teardown, state loaded...).
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
