Thread: Ticket 3: groups of servers
Hi, I have an almost working patch. I still have one bug: if someone adds the first server in a group, the group and the server don't appear in the treeview. After quite some tests and debugging, the program adds them in the treeview, but after the CreateObjects call, they disappear. If someone can take a look at the patch, I would be glad to know if they find something strange in it. The bug fix is probably something obvious but I don't see what. BTW, it is not the final patch, some work remain to be done (the group combobox is only used as a textbox right now, it's badly displayed, some code are redundant, etc.) Thanks. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Attachment
On Sun, Apr 11, 2010 at 10:36 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Hi, > > I have an almost working patch. I still have one bug: if someone adds > the first server in a group, the group and the server don't appear in > the treeview. After quite some tests and debugging, the program adds > them in the treeview, but after the CreateObjects call, they disappear. I didn't see that here. It seemed to work fine - though it sure would be nice if it would dynamically move the nodes around. Restarting pgAdmin to see the changes doesn't feel nice. It also feels like I should be able to right-click a group node, though I appreciate you cannot do that with the Servers node either. Finally (I think!) - it seems weird that the Servers node lists the total number of Servers, even those that aren't direct children. Y'know - as I type I can't help thinking that the correct way to do this is to consider the root node to be the group, in which we currently have a single, fixed group called 'Servers'. Additional groups would then be additional root nodes... but I don't know if you can do that on all operating systems. > BTW, it is not the final patch, some work remain to be done (the group > combobox is only used as a textbox right now, it's badly displayed, some > code are redundant, etc.) Yeah, I saw that. BTW - I noticed another issue while I was there. The default colour shown by the colour select button seems to be random, so for servers for which I never previously chose a colour updating them to set a group is also changing them to a random colour, unless I manually choose that too. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com
Le 12/04/2010 14:14, Dave Page a écrit : > On Sun, Apr 11, 2010 at 10:36 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: >> Hi, >> >> I have an almost working patch. I still have one bug: if someone adds >> the first server in a group, the group and the server don't appear in >> the treeview. After quite some tests and debugging, the program adds >> them in the treeview, but after the CreateObjects call, they disappear. > Fixed it. > I didn't see that here. It seemed to work fine - though it sure would > be nice if it would dynamically move the nodes around. Restarting > pgAdmin to see the changes doesn't feel nice. Sure, it's way nicer. Done. > It also feels like I > should be able to right-click a group node, though I appreciate you > cannot do that with the Servers node either. > What do you want with a right click in a group node? access to the properties? > Finally (I think!) - it seems weird that the Servers node lists the > total number of Servers, even those that aren't direct children. > Fixed. > Y'know - as I type I can't help thinking that the correct way to do > this is to consider the root node to be the group, in which we > currently have a single, fixed group called 'Servers'. Additional > groups would then be additional root nodes... but I don't know if you > can do that on all operating systems. > Done. Seems to work everywhere I checked. So, now, when you launch pgAdmin, it behaves exactly as before (ie you still have one one group, Servers). But you can add more groups and affects servers into those groups). >> BTW, it is not the final patch, some work remain to be done (the group >> combobox is only used as a textbox right now, it's badly displayed, some >> code are redundant, etc.) > > Yeah, I saw that. > They are all fixed by now. > BTW - I noticed another issue while I was there. The default colour > shown by the colour select button seems to be random, so for servers > for which I never previously chose a colour updating them to set a > group is also changing them to a random colour, unless I manually > choose that too. > Fixed too. New patch attached. Seems good to go for me. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Attachment
On Sat, Apr 17, 2010 at 7:09 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > What do you want with a right click in a group node? access to the > properties? Same as other nodes - Add xxx etc. It's not that important though. >> Y'know - as I type I can't help thinking that the correct way to do >> this is to consider the root node to be the group, in which we >> currently have a single, fixed group called 'Servers'. Additional >> groups would then be additional root nodes... but I don't know if you >> can do that on all operating systems. >> > > Done. Seems to work everywhere I checked. There's something pretty broken. I'm not sure how to describe it though... see if you can make sense of this! - I edited a server. It had picked up my 'custom' group correctly, but also had a blank entry in the list. I expected to see the default 'Servers' there. - I moved the server to 'blank'. Nothing seemed to happen. - I looked at the properties for the server again. Now I only have 2 blank groups listed. - I restarted pgAdmin. Now I have 2 identical servers under 'Servers'. What I would expect to see is: - Any existing servers automatically get moved into the 'Servers' group on first run of the new version. - Any new servers default to the first group in the combo box. This will obviously be 'Servers' on a new installation. - The Servers group is merely a default that's always available. Otherwise it is no different from any other group. Regards, Dave -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com The Enterprise Postgres Company
Le 19/04/2010 13:03, Dave Page a écrit : > On Sat, Apr 17, 2010 at 7:09 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: >> What do you want with a right click in a group node? access to the >> properties? > > Same as other nodes - Add xxx etc. It's not that important though. > Well, we didn't do it for the actual Servers node. I don't think we should do it for this one. >>> Y'know - as I type I can't help thinking that the correct way to do >>> this is to consider the root node to be the group, in which we >>> currently have a single, fixed group called 'Servers'. Additional >>> groups would then be additional root nodes... but I don't know if you >>> can do that on all operating systems. >>> >> >> Done. Seems to work everywhere I checked. > > There's something pretty broken. I'm not sure how to describe it > though... see if you can make sense of this! > > - I edited a server. It had picked up my 'custom' group correctly, but > also had a blank entry in the list. I expected to see the default > 'Servers' there. > - I moved the server to 'blank'. Nothing seemed to happen. > - I looked at the properties for the server again. Now I only have 2 > blank groups listed. > - I restarted pgAdmin. Now I have 2 identical servers under 'Servers'. > > What I would expect to see is: > > - Any existing servers automatically get moved into the 'Servers' > group on first run of the new version. > - Any new servers default to the first group in the combo box. This > will obviously be 'Servers' on a new installation. > - The Servers group is merely a default that's always available. > Otherwise it is no different from any other group. > Finally, I had time to work on this. I changed my patch so that it follows what you expected to see. I was not really able to reproduce the problem you explain above after my changes. Can you try this last one? Thanks. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Attachment
On Thu, Apr 22, 2010 at 8:26 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Well, we didn't do it for the actual Servers node. I don't think we > should do it for this one. Right - that's why I wasn't sure. And now that groups are root nodes, it seems even less important. > Finally, I had time to work on this. I changed my patch so that it > follows what you expected to see. I was not really able to reproduce the > problem you explain above after my changes. > > Can you try this last one? Thanks. Still seeing a crash on OSX, but mostly I think it's good now. Here's what happened: - I cleared my prefs file so I just had 4 servers detected and added to the 'Servers' group at startup. - I moved 3 of the servers to a new 'PostgreSQL' group. - I moved 1 server to a new 'Advanced Server' group. At this point I had three groups visible: Servers (0) PostgreSQL (3) - Server 1 - Server 2 - Server 3 Advanced Server (1) Server 4 Gripe 1: I think the Servers group should be hidden if it's empty. In previous versions it didn't really matter as the first thing you'd do is add a server. Now though, it might remain empty forever. Gripe 2: I think the groups should be shown in alphabetical order. Should be a one-line change :-) - I then right-clicked Server 4 and selected Properties Gripe 3: pgAdmin crashes reliably at this point: Thread 0 Crashed: Dispatch queue: com.apple.main-thread 0 pgAdmin3-Debug 0x00141fe5 dlgServer::dlgServer(pgaFactory*, frmMain*, pgServer*) + 2725 1 pgAdmin3-Debug 0x00142307 pgServerFactory::CreateDialog(frmMain*, pgObject*, pgObject*) + 55 2 pgAdmin3-Debug 0x0011ae59 dlgProperty::CreateDlg(frmMain*, pgObject*, bool, pgaFactory*) + 201 3 pgAdmin3-Debug 0x00122b66 dlgProperty::EditObjectDialog(frmMain*, ctlSQLBox*, pgObject*) + 166 4 pgAdmin3-Debug 0x00123063 propertyFactory::StartDialog(frmMain*, pgObject*) + 35 5 pgAdmin3-Debug 0x00193e86 frmMain::OnAction(wxCommandEvent&) + 70 6 libwx_base_carbonu-2.8.0.dylib 0x01404463 wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) + 131 7 libwx_base_carbonu-2.8.0.dylib 0x01404526 wxEvtHandler::SearchDynamicEventTable(wxEvent&) + 86 8 libwx_base_carbonu-2.8.0.dylib 0x01404df3 wxEvtHandler::ProcessEvent(wxEvent&) + 179 9 libwx_base_carbonu-2.8.0.dylib 0x01404da9 wxEvtHandler::ProcessEvent(wxEvent&) + 105 10 libwx_macu_core-2.8.0.dylib 0x01004f28 wxWindowBase::TryParent(wxEvent&) + 88 11 libwx_base_carbonu-2.8.0.dylib 0x01404db9 wxEvtHandler::ProcessEvent(wxEvent&) + 121 12 libwx_base_carbonu-2.8.0.dylib 0x01404da9 wxEvtHandler::ProcessEvent(wxEvent&) + 105 13 libwx_macu_core-2.8.0.dylib 0x0102fb6e wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 46 14 libwx_macu_core-2.8.0.dylib 0x00fdff80 wxMenuBase::SendEvent(int, int) + 144 15 libwx_macu_core-2.8.0.dylib 0x00f2a6ac wxMenu::MacHandleCommandProcess(wxMenuItem*, int, wxWindow*) + 60 16 libwx_macu_core-2.8.0.dylib 0x00f5b3ab wxMacWindowEventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 427 17 com.apple.HIToolbox 0x92f8d0a9 DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 1567 18 com.apple.HIToolbox 0x92f8c370 SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 411 19 com.apple.HIToolbox 0x92faeb55 SendEventToEventTarget + 52 20 com.apple.HIToolbox 0x92fdb147 SendHICommandEvent(unsigned long, HICommand const*, unsigned long, unsigned long, unsigned char, void const*, OpaqueEventTargetRef*, OpaqueEventTargetRef*, OpaqueEventRef**) + 448 21 com.apple.HIToolbox 0x92fffe40 SendMenuCommandWithContextAndModifiers + 66 22 com.apple.HIToolbox 0x92fffdf5 SendMenuItemSelectedEvent + 121 23 com.apple.HIToolbox 0x92fffcfa FinishMenuSelection(SelectionData*, MenuResult*, MenuResult*) + 152 24 com.apple.HIToolbox 0x931813d4 PopUpMenuSelectCore(MenuData*, Point, double, Point, unsigned short, unsigned int, Rect const*, unsigned short, unsigned long, Rect const*, Rect const*, __CFString const*, OpaqueMenuRef**, unsigned short*) + 1851 25 com.apple.HIToolbox 0x931818b0 PopUpMenuSelect + 253 26 libwx_macu_core-2.8.0.dylib 0x00f54db8 wxWindow::DoPopupMenu(wxMenu*, int, int) + 120 27 pgAdmin3-Debug 0x001944d3 frmMain::doPopup(wxWindow*, wxPoint, pgObject*) + 739 28 pgAdmin3-Debug 0x0019473b frmMain::OnSelRightClick(wxTreeEvent&) + 107 29 libwx_base_carbonu-2.8.0.dylib 0x01404463 wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) + 131 30 libwx_base_carbonu-2.8.0.dylib 0x01404ae1 wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 113 31 libwx_base_carbonu-2.8.0.dylib 0x01404e0f wxEvtHandler::ProcessEvent(wxEvent&) + 207 32 libwx_base_carbonu-2.8.0.dylib 0x01404da9 wxEvtHandler::ProcessEvent(wxEvent&) + 105 33 libwx_macu_core-2.8.0.dylib 0x01004f28 wxWindowBase::TryParent(wxEvent&) + 88 34 libwx_base_carbonu-2.8.0.dylib 0x01404db9 wxEvtHandler::ProcessEvent(wxEvent&) + 121 35 libwx_base_carbonu-2.8.0.dylib 0x01404da9 wxEvtHandler::ProcessEvent(wxEvent&) + 105 36 libwx_macu_core-2.8.0.dylib 0x0102fb6e wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 46 37 libwx_macu_core-2.8.0.dylib 0x0103d43e wxGenericTreeCtrl::OnMouse(wxMouseEvent&) + 1902 38 libwx_base_carbonu-2.8.0.dylib 0x01404463 wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&, wxEvtHandler*, wxEvent&) + 131 39 libwx_base_carbonu-2.8.0.dylib 0x01404ae1 wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 113 40 libwx_base_carbonu-2.8.0.dylib 0x01404e0f wxEvtHandler::ProcessEvent(wxEvent&) + 207 41 libwx_base_carbonu-2.8.0.dylib 0x01404da9 wxEvtHandler::ProcessEvent(wxEvent&) + 105 42 libwx_macu_core-2.8.0.dylib 0x0102fb6e wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 46 43 libwx_macu_core-2.8.0.dylib 0x00f5296e wxMacTopLevelMouseEventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 798 44 libwx_macu_core-2.8.0.dylib 0x00f52e56 wxMacTopLevelEventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 198 45 com.apple.HIToolbox 0x92f8d0a9 DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 1567 46 com.apple.HIToolbox 0x92f8c370 SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 411 47 com.apple.HIToolbox 0x92faeb55 SendEventToEventTarget + 52 48 com.apple.HIToolbox 0x92fc063b ToolboxEventDispatcherHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*) + 1257 49 com.apple.HIToolbox 0x92f8d4fa DispatchEventToHandlers(EventTargetRec*, OpaqueEventRef*, HandlerCallRec*) + 2672 50 com.apple.HIToolbox 0x92f8c370 SendEventToEventTargetInternal(OpaqueEventRef*, OpaqueEventTargetRef*, HandlerCallRec*) + 411 51 com.apple.HIToolbox 0x92faeb55 SendEventToEventTarget + 52 52 libwx_macu_core-2.8.0.dylib 0x00eedfe6 wxApp::MacHandleOneEvent(void*) + 38 53 libwx_macu_core-2.8.0.dylib 0x00eee80b wxApp::MacDoOneEvent() + 123 54 libwx_macu_core-2.8.0.dylib 0x00f07fd3 wxEventLoop::Dispatch() + 35 55 libwx_macu_core-2.8.0.dylib 0x00fae708 wxEventLoopManual::Run() + 136 56 libwx_macu_core-2.8.0.dylib 0x00f87bf3 wxAppBase::MainLoop() + 83 57 libwx_base_carbonu-2.8.0.dylib 0x013ba81a wxEntry(int&, wchar_t**) + 106 58 pgAdmin3-Debug 0x0003fc58 main + 24 59 pgAdmin3-Debug 0x0003f979 start + 53 -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com The Enterprise Postgres Company
Le 23/04/2010 15:00, Dave Page a écrit : > On Thu, Apr 22, 2010 at 8:26 PM, Guillaume Lelarge > <guillaume@lelarge.info> wrote: >> Well, we didn't do it for the actual Servers node. I don't think we >> should do it for this one. > > Right - that's why I wasn't sure. And now that groups are root nodes, > it seems even less important. > >> Finally, I had time to work on this. I changed my patch so that it >> follows what you expected to see. I was not really able to reproduce the >> problem you explain above after my changes. >> >> Can you try this last one? Thanks. > > Still seeing a crash on OSX, but mostly I think it's good now. Here's > what happened: > > - I cleared my prefs file so I just had 4 servers detected and added > to the 'Servers' group at startup. > > - I moved 3 of the servers to a new 'PostgreSQL' group. > > - I moved 1 server to a new 'Advanced Server' group. > > At this point I had three groups visible: > > Servers (0) > PostgreSQL (3) > - Server 1 > - Server 2 > - Server 3 > Advanced Server (1) > Server 4 > > Gripe 1: I think the Servers group should be hidden if it's empty. In > previous versions it didn't really matter as the first thing you'd do > is add a server. Now though, it might remain empty forever. > Fixed. I still keep a "Servers" item in the combobox, even if there is no "Servers" group. No sure this is really smart. Should I get rid of it? > Gripe 2: I think the groups should be shown in alphabetical order. > Should be a one-line change :-) > Fixed. > - I then right-clicked Server 4 and selected Properties > > Gripe 3: pgAdmin crashes reliably at this point: > Fixed. Thanks for your comments. See patch attached. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com
Attachment
I think it's fine to keep it in the combo. If it's all fixed now, pls go ahead and commit. :-) On 4/23/10, Guillaume Lelarge <guillaume@lelarge.info> wrote: > Le 23/04/2010 15:00, Dave Page a écrit : >> On Thu, Apr 22, 2010 at 8:26 PM, Guillaume Lelarge >> <guillaume@lelarge.info> wrote: >>> Well, we didn't do it for the actual Servers node. I don't think we >>> should do it for this one. >> >> Right - that's why I wasn't sure. And now that groups are root nodes, >> it seems even less important. >> >>> Finally, I had time to work on this. I changed my patch so that it >>> follows what you expected to see. I was not really able to reproduce the >>> problem you explain above after my changes. >>> >>> Can you try this last one? Thanks. >> >> Still seeing a crash on OSX, but mostly I think it's good now. Here's >> what happened: >> >> - I cleared my prefs file so I just had 4 servers detected and added >> to the 'Servers' group at startup. >> >> - I moved 3 of the servers to a new 'PostgreSQL' group. >> >> - I moved 1 server to a new 'Advanced Server' group. >> >> At this point I had three groups visible: >> >> Servers (0) >> PostgreSQL (3) >> - Server 1 >> - Server 2 >> - Server 3 >> Advanced Server (1) >> Server 4 >> >> Gripe 1: I think the Servers group should be hidden if it's empty. In >> previous versions it didn't really matter as the first thing you'd do >> is add a server. Now though, it might remain empty forever. >> > > Fixed. I still keep a "Servers" item in the combobox, even if there is > no "Servers" group. No sure this is really smart. Should I get rid of it? > >> Gripe 2: I think the groups should be shown in alphabetical order. >> Should be a one-line change :-) >> > > Fixed. > >> - I then right-clicked Server 4 and selected Properties >> >> Gripe 3: pgAdmin crashes reliably at this point: >> > > Fixed. > > Thanks for your comments. See patch attached. > > > -- > Guillaume. > http://www.postgresqlfr.org > http://dalibo.com > -- Dave Page EnterpriseDB UK: http://www.enterprisedb.com The Enterprise Postgres Company
Le 24/04/2010 00:32, Dave Page a écrit : > I think it's fine to keep it in the combo. > > If it's all fixed now, pls go ahead and commit. :-) > Commited. Thanks. -- Guillaume. http://www.postgresqlfr.org http://dalibo.com