Please let me know if you get this - I'm not sure how the notification mechanism works. When looking at this recent bug I noticed the change to scape listener and runner. The basic idea makes sense but we should discuss these deep engine changes before making them. I think we may need to end up backing it out or changing it. Here's why:
1. It's a breaking change. Anyone implementing plain ScapeListener will have to add the unimplemented methods to have their exiting code build, and old models will break. Ascape is a really mature API so we shouldn't be introducing breaking changes and at worst they should happen at a major version change.
2. The engine stuff is - hoe to put this - quite finely balanced. It's actually been quite difficult to get all of the pieces flowing correctly. I haven't run it yet and I don't see by inspection any major issues, but I worry that a change of this significance could cause other stuff (especially consumers) that rely on this might break something.
3. There is already a mechanism for this and so this change kind of violates the contract - which is scape notifies listeners, listeners get a chance to respond, and then Scape does what it wants. The only thing that views are allowed to do is to refuse to update, and that should be a last resort. On scapeClosing, if a view really wishes to veto a close, it should just block. A view should never actually be able to keep the entire environment or a particular model execution to exit. That's actually what "closeFinally" accomplishes if a view is particularly recalcitrant. For example, in the Escape implementation, a close action will ask the model politely to quit. If after a while the scape still hasn't quite and the user insists, we'll just kill the thread.
Let's discuss what you need to accomplish and perhaps there is a better way of doing it.
cheers,
Miles
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Breaking the API is not good! I assumed most code extended DefaultScapeListener (which has an default implementation of the new ScapeListener interface method I added) instead of implementing ScapeListener; sounds like this is not the case.
The functionality I'm trying to achieve is the ability for JInternalFrames on the JDesktopFrame to veto an Ascape application close (either via clicking the close window X in the top right hand corner, Alt-F4, or the File - Quit menu item) if for example they have unsaved changes. In this instance they can present a "Do you want to save changes?" Yes-No-Cancel dialog and if cancel is selected then Ascape application won't close. In my specific application I have some text panels inside JInternalFrames that the user can edit and this might not have been saved before the user goes to quit.
These text panel JInternalFrames don't actually have to be ScapeListeners. They just ended up that way so I could display them easily by adding them to the scape. I could add them directly via SwingEnvironment.createFrame. JInternalFrames have a internalFrameClosing method which can prevent itself from closing. I could make the Ascape environment check to see all JInternalFrames have closed, and only quit if they all have closed. Would this work, or would it undermine the scape listener notification contract?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Ah, I see. So this is in effect the user not using the toolbar button. I can see why this is needed. The issue I see with this solution is that a JInternal frame simply not being closable for some reason would prevent the user from exiting.
One other option is to have a scape listener that returns false for isLifeOfScape. This makes the listener a singleton for the environment. Within this listener one could simply check for all windows being saved when the quit request happens and if they do, getting the user input and responding as appropriate - I think its ok for the environment to choose to ignore a quit request and do something different as this isn't interfering w/ the model life-cycle per se. See SwingRunner.closeAndOpenSavedFinally(Scape) for a similar usage.
But I'm wondering if it might not be simpler just to intercept the user close action before even sending it to the Runner? I guess the above solution is more general though.
Taking your first more general solution, I took a look SwingRunner.closeAndOpenSavedFinally(Scape) but couldn't work out how this is related. So if a listener is a singleton for the environment (because isLifeOfScape returns false) what are the existing methods in which ignoring a quit request is possible, or would have to create them (in which case, how is this different from what I have already implemented?)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Sorry, ignore that part about the listeners - you don't need them at all. Runner is simply responding to a ControlEvent.REQUEST_QUIT, right? What we can do is choose wether to actually execute this or not based on what the user decides. If the runner never forwards it on to the Scape, nothing happens. And this is from the runner side, so the views don't have a say in the matter.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
OK, I went ahead and rolled it back, just because I needed to get the serialization stuff fixed and that required a working environment. But that is all in core and I think the changes are going to be in Swing anyway.
To the serialization issue, I also had to roll back one other change that might end up breaking something that was fixed on 02 10. The thread locking stuff in DataGroup broke serialization. But no worries, I had a couple of early changes that also broke it. ;) We now have a unit test taht hopefully will prevent stepping on the (non-ui at least) serialization from now on. I'llhave a separate bug for a related issue.
Also AbstractUIEnvironment appears to have a missing signature change. I put in what I think is the right new signature.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
OK, no problem. I'll take the latest rollback and work from there on the Runner side, although I've some other work to do first so probably won't get to it until next month.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I just did one other change as well that should get all of the serialization stuff up to snuff. Let me know if it doesn't. As there are a number of important bug fixes in the last little bit, I'm thinking of rolling out a maintenance release. Let me know if you happen to change your mind in the week or so and want to make some changes prior to 5.5.1.
cheers,
Miles
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
HI Oman,
Please let me know if you get this - I'm not sure how the notification mechanism works. When looking at this recent bug I noticed the change to scape listener and runner. The basic idea makes sense but we should discuss these deep engine changes before making them. I think we may need to end up backing it out or changing it. Here's why:
1. It's a breaking change. Anyone implementing plain ScapeListener will have to add the unimplemented methods to have their exiting code build, and old models will break. Ascape is a really mature API so we shouldn't be introducing breaking changes and at worst they should happen at a major version change.
2. The engine stuff is - hoe to put this - quite finely balanced. It's actually been quite difficult to get all of the pieces flowing correctly. I haven't run it yet and I don't see by inspection any major issues, but I worry that a change of this significance could cause other stuff (especially consumers) that rely on this might break something.
3. There is already a mechanism for this and so this change kind of violates the contract - which is scape notifies listeners, listeners get a chance to respond, and then Scape does what it wants. The only thing that views are allowed to do is to refuse to update, and that should be a last resort. On scapeClosing, if a view really wishes to veto a close, it should just block. A view should never actually be able to keep the entire environment or a particular model execution to exit. That's actually what "closeFinally" accomplishes if a view is particularly recalcitrant. For example, in the Escape implementation, a close action will ask the model politely to quit. If after a while the scape still hasn't quite and the user insists, we'll just kill the thread.
Let's discuss what you need to accomplish and perhaps there is a better way of doing it.
cheers,
Miles
Breaking the API is not good! I assumed most code extended DefaultScapeListener (which has an default implementation of the new ScapeListener interface method I added) instead of implementing ScapeListener; sounds like this is not the case.
The functionality I'm trying to achieve is the ability for JInternalFrames on the JDesktopFrame to veto an Ascape application close (either via clicking the close window X in the top right hand corner, Alt-F4, or the File - Quit menu item) if for example they have unsaved changes. In this instance they can present a "Do you want to save changes?" Yes-No-Cancel dialog and if cancel is selected then Ascape application won't close. In my specific application I have some text panels inside JInternalFrames that the user can edit and this might not have been saved before the user goes to quit.
These text panel JInternalFrames don't actually have to be ScapeListeners. They just ended up that way so I could display them easily by adding them to the scape. I could add them directly via SwingEnvironment.createFrame. JInternalFrames have a internalFrameClosing method which can prevent itself from closing. I could make the Ascape environment check to see all JInternalFrames have closed, and only quit if they all have closed. Would this work, or would it undermine the scape listener notification contract?
Oliver,
Ah, I see. So this is in effect the user not using the toolbar button. I can see why this is needed. The issue I see with this solution is that a JInternal frame simply not being closable for some reason would prevent the user from exiting.
One other option is to have a scape listener that returns false for isLifeOfScape. This makes the listener a singleton for the environment. Within this listener one could simply check for all windows being saved when the quit request happens and if they do, getting the user input and responding as appropriate - I think its ok for the environment to choose to ignore a quit request and do something different as this isn't interfering w/ the model life-cycle per se. See SwingRunner.closeAndOpenSavedFinally(Scape) for a similar usage.
But I'm wondering if it might not be simpler just to intercept the user close action before even sending it to the Runner? I guess the above solution is more general though.
BTW, since you're going to be looking at save behavior anyway, could you take care of the recent https://sourceforge.net/tracker/?func=detail&aid=2976116&group_id=203744&atid=986824 bug and just make sure its working correctly?
cheers,
Miles
Sorry again for mashing up your with your ID above. :))
Never mind on that serialization issue. I'm finding it to be more complex than I first thought.
Hi Miles,
Taking your first more general solution, I took a look SwingRunner.closeAndOpenSavedFinally(Scape) but couldn't work out how this is related. So if a listener is a singleton for the environment (because isLifeOfScape returns false) what are the existing methods in which ignoring a quit request is possible, or would have to create them (in which case, how is this different from what I have already implemented?)
Sorry, ignore that part about the listeners - you don't need them at all. Runner is simply responding to a ControlEvent.REQUEST_QUIT, right? What we can do is choose wether to actually execute this or not based on what the user decides. If the runner never forwards it on to the Scape, nothing happens. And this is from the runner side, so the views don't have a say in the matter.
OK, I went ahead and rolled it back, just because I needed to get the serialization stuff fixed and that required a working environment. But that is all in core and I think the changes are going to be in Swing anyway.
To the serialization issue, I also had to roll back one other change that might end up breaking something that was fixed on 02 10. The thread locking stuff in DataGroup broke serialization. But no worries, I had a couple of early changes that also broke it. ;) We now have a unit test taht hopefully will prevent stepping on the (non-ui at least) serialization from now on. I'llhave a separate bug for a related issue.
Also AbstractUIEnvironment appears to have a missing signature change. I put in what I think is the right new signature.
OK, no problem. I'll take the latest rollback and work from there on the Runner side, although I've some other work to do first so probably won't get to it until next month.
I just did one other change as well that should get all of the serialization stuff up to snuff. Let me know if it doesn't. As there are a number of important bug fixes in the last little bit, I'm thinking of rolling out a maintenance release. Let me know if you happen to change your mind in the week or so and want to make some changes prior to 5.5.1.
cheers,
Miles