Thread: ESC closes window without warning
I had a function almost finished, written in function editor. Then I accidentally hit ESC making editor closed without any warning. Terrific :( I would like to see a confirmation which protect against such loses. with regards.
I've already reported this issue. I hope we get this fixed before next release... Regards, Edson Em 06/02/2013 13:12, Michal Kozusznik escreveu: > I had a function almost finished, written in function editor. > Then I accidentally hit ESC making editor closed without any warning. > Terrific :( > > I would like to see a confirmation which protect against such loses. > > with regards. > > >
On Wed, Feb 6, 2013 at 3:13 PM, Edson Richter <edsonrichter@hotmail.com> wrote: > I've already reported this issue. > > I hope we get this fixed before next release... It won't be fixed in 1.16.2 I doubt, as it's a change in behaviour that's been established for over 10 years with no previous complaints that I can recall. The attached patch is a quick stab at doing something. It has a couple of quirks though, and I suspect that fixing them might be a *much* larger task: - It will apply to all dialogues, so if you open the backup dialogue for example, type a filename and then click cancel, you'll get the same message (which doesn't hurt, but isn't really the best wording in that situation). - It will only show the warning if you've changed something *and* what you've changed is valid enough that the dialogue is happy with it. In other words, you could write a 200 line function but not specify a name, then get no warning if you hit escape. I'm not really happy with those quirks if I'm honest. The other option would be to have a warning added only to the dialogues that may take real effort to recreate - e.g. dlgFunction, dlgView, dlgPackage and dlgTable. Thoughts? -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
On 6.2.2013 18:19, Dave Page wrote: > It won't be fixed in 1.16.2 I doubt, as it's a change in behaviour > that's been established for over 10 years with no previous complaints > that I can recall. I don't want to be rude, but maybe no one has used this feature due bugs appearing from time to time. pgAdmin is full of inconsistent resolutions established for over 10 years (in case of requesters, some appears even if are not needed at all why in other places there is no any attempt to confirm action). But this is not a reason to stay with it for next 10. It is quite clear that reported lack of confirmation, opens a way to lose the work. I have feeling that even you agree with this. So why the hell do not agree to improve it? You can even make it optional. I believe adding another checkbox to options tab is not impossible mission (sorry but some answers sounds like this) Thanx for the patch. It's presence proving that this feature is needed. However there are 2 issues: - most of pgAdmin users don't want to maintain own fork of pgadmin, recompile any new version applying patches etc - as you noted, the behavior is not quite correct. Confirmation should appear when value of any single form item has been changed (doesn't matter if dialog is happy with it or not). I believe it is not hard to implement such checking for any window. with regards.
On Thu, Feb 7, 2013 at 10:20 AM, Michal Kozusznik <kozusznik.michal@ifortuna.cz> wrote: > On 6.2.2013 18:19, Dave Page wrote: >> >> It won't be fixed in 1.16.2 I doubt, as it's a change in behaviour >> that's been established for over 10 years with no previous complaints >> that I can recall. > > > I don't want to be rude, but maybe no one has used this feature due bugs > appearing from time to time. pgAdmin is full of inconsistent resolutions > established for over 10 years (in case of requesters, some appears even if > are not needed at all why in other places there is no any attempt to confirm > action). But this is not a reason to stay with it for next 10. > It is quite clear that reported lack of confirmation, opens a way to lose > the work. I have feeling that even you agree with this. So why the hell do > not agree to improve it? You can even make it optional. I believe adding > another checkbox to options tab is not impossible mission (sorry but some > answers sounds like this) You are missing the point. I have no problem changing this for the next major version, but it's a change in functionality and requires additional translation work, neither of which are things we do for minor releases except where necessary to fix serious bugs - which this is not. > Thanx for the patch. It's presence proving that this feature is needed. > However there are 2 issues: > - most of pgAdmin users don't want to maintain own fork of pgadmin, > recompile any new version applying patches etc You have missed the point again. I'm not expecting anyone to apply the patch themselves except other developers, but am presenting it as an option for consideration. That's how open source work is done - patches are written, and the pros/cons and alternate approaches discussed before a solution is settled upon. > - as you noted, the behavior is not quite correct. Confirmation should > appear when value of any single form item has been changed (doesn't matter > if dialog is happy with it or not). I believe it is not hard to implement > such checking for any window. Well that is exactly the point I was raising. I think it would be a lot of work to add the check in that way (though it is undoubtedly the best way to do it). If you have a simple way to make it work, I'd love to see a proposed patch because at the moment, I don't see a way to do it that wouldn't take at least a few days of work. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
OK. Thanx for deeper explanation. Looks like I missed a point twice :) > If you have a simple way to make it work, I'd love > to see a proposed patch because at the moment, I don't see a way to do > it that wouldn't take at least a few days of work. Don't know architecture of pgadmin, but if requesters inherit from some base class, it should be enough to modify only this one. Store old values into some array (or maybe consider some hash) and compare it to current while hitting ESC/Cancel. I would do that this way if I was author of code. Basically small change. With regards MK
On Thu, Feb 7, 2013 at 10:57 AM, Michal Kozusznik <kozusznik.michal@ifortuna.cz> wrote: > OK. Thanx for deeper explanation. Looks like I missed a point twice :) > > >> If you have a simple way to make it work, I'd love >> to see a proposed patch because at the moment, I don't see a way to do >> it that wouldn't take at least a few days of work. > > > > Don't know architecture of pgadmin, but if requesters inherit from some base > class, it should be enough to modify only this one. Store old values into > some array (or maybe consider some hash) and compare it to current while > hitting ESC/Cancel. I would do that this way if I was author of code. > Basically small change. That's basically what the patch I posted does. The problem is that the base class doesn't have any knowledge of what controls are on an instantiated dialogue, or how to test if they've been changed from whatever the default is (for new objects) or from whatever pgObject derived object exists for edits. The only way I can see to do this is to write a function for each individual dialogue that can compare the values of each control to the defaults or an existing object - pretty similar to what CheckChange() already does - the difference being that it is validating as well. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
> That's basically what the patch I posted does. The problem is that the > base class doesn't have any knowledge of what controls are on an > instantiated dialogue, or how to test if they've been changed from > whatever the default is (for new objects) or from whatever pgObject > derived object exists for edits. > > The only way I can see to do this is to write a function for each > individual dialogue that can compare the values of each control to the > defaults or an existing object - pretty similar to what CheckChange() > already does - the difference being that it is validating as well. Couldn't method from base class scan all associated editable UI elements (members of the window) and store their values just after window opening? Also, I believe there must be some event fired on closing window, also processed by this base class. It's good place to do comparison. Just ideas. I'm sure you know the architecture better. with regards.
On Thu, Feb 7, 2013 at 12:30 PM, Michal Kozusznik <kozusznik.michal@ifortuna.cz> wrote: > >> That's basically what the patch I posted does. The problem is that the >> base class doesn't have any knowledge of what controls are on an >> instantiated dialogue, or how to test if they've been changed from >> whatever the default is (for new objects) or from whatever pgObject >> derived object exists for edits. >> >> The only way I can see to do this is to write a function for each >> individual dialogue that can compare the values of each control to the >> defaults or an existing object - pretty similar to what CheckChange() >> already does - the difference being that it is validating as well. > > > Couldn't method from base class scan all associated editable UI elements > (members of the window) and store their values just after window opening? In theory something like that could be done - there's a wxWindow::GetChildren() function that returns a list of wxWindow objects. It's somewhat more complex to actually get the values from each control though - we'd need to try to figure out what type of object it is, and whether or not it's user-updated, and then stash away the starting value. It's hard to do that after the window has been setup though, unless we do it for every class (because we already do the setup using the event we'd want to use). I'd have to play with it, but it's still not as easy as you might think. > Also, I believe there must be some event fired on closing window, also > processed by this base class. It's good place to do comparison. > Just ideas. I'm sure you know the architecture better. There is - that's what I used in my earlier patch. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company