Обсуждение: Refresh objects on Click
Hi Dave, I'm sending the patch on the Refresh object on Click[1]. I put the three options. "None", "Refresh object on click" and "Refresh object and children on click" You can review it? PS: Forgive me, of a mistake in my English. Thank You. [1] = http://archives.postgresql.org/pgadmin-hackers/2011-06/msg00039.php
Hi,
On Wed, Aug 10, 2011 at 4:30 AM, Vinicius Santos
<vinicius.santos.lista@gmail.com> wrote:
> Hi Dave,
>
> I'm sending the patch on the Refresh object on Click[1].
>
> I put the three options. "None", "Refresh object on click" and "Refresh
> object and children on click"
>
> You can review it?
Still seems to need some work unfortunately:
- Don't use the translated strings to test settings (eg. 'if
(settingRefreshOnClick == _("Refresh object on click"))' ). Use an
enum instead, to ensure it continues to work if the user switches
language.
- The new option should be on the Browser tab of the Options dialogue,
not the Query Tool tab.
- When using "Refresh object on click", clicking on a table adds
multiple copies of the child nodes - see the attached screenshot. It
also seemed to break the reverse engineered SQL.
- Clicking the first "Columns" collection under the "test" table led to a crash:
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 pgAdmin3-Debug 0x00911e51
pgaFactory::GetMetaType() + 15 (factory.cpp:187)
1 pgAdmin3-Debug 0x00754f71 pgObject::GetMetaType()
const + 37 (pgObject.cpp:82)
2 pgAdmin3-Debug 0x007be0f0
pgTableObjCollection::CanCreate() + 36 (pgTable.cpp:1676)
3 pgAdmin3-Debug 0x00746a1c pgObject::GetNewMenu() +
70 (pgObject.cpp:200)
4 pgAdmin3-Debug 0x003f3020
frmMain::setDisplay(pgObject*, ctlListView*, ctlSQLBox*) + 846
(events.cpp:524)
5 pgAdmin3-Debug 0x003f4391
frmMain::execSelChange(wxTreeItemId, bool) + 2313 (events.cpp:455)
6 pgAdmin3-Debug 0x003ef7af
frmMain::OnTreeSelChanged(wxTreeEvent&) + 131 (events.cpp:368)
7 libwx_base_carbonud-2.8.0.dylib 0x0210f91d
wxAppConsole::HandleEvent(wxEvtHandler*, void
(wxEvtHandler::*)(wxEvent&), wxEvent&) const + 65
8 libwx_base_carbonud-2.8.0.dylib 0x021afedc
wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
wxEvtHandler*, wxEvent&) + 212
9 libwx_base_carbonud-2.8.0.dylib 0x021b1888
wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 204
10 libwx_base_carbonud-2.8.0.dylib 0x021b19e3
wxEvtHandler::ProcessEvent(wxEvent&) + 311
11 libwx_base_carbonud-2.8.0.dylib 0x021b1a24
wxEvtHandler::ProcessEvent(wxEvent&) + 376
12 libwx_macud_core-2.8.0.dylib 0x01ae5dd2
wxWindowBase::TryParent(wxEvent&) + 162
13 libwx_base_carbonud-2.8.0.dylib 0x021b1a4a
wxEvtHandler::ProcessEvent(wxEvent&) + 414
14 libwx_base_carbonud-2.8.0.dylib 0x021b1a24
wxEvtHandler::ProcessEvent(wxEvent&) + 376
15 libwx_macud_core-2.8.0.dylib 0x01b17a5a
wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 52
16 libwx_macud_core-2.8.0.dylib 0x01b256e7
wxGenericTreeCtrl::DoSelectItem(wxTreeItemId const&, bool, bool) +
1099
17 libwx_macud_core-2.8.0.dylib 0x01b2675b
wxGenericTreeCtrl::OnMouse(wxMouseEvent&) + 4139
18 libwx_base_carbonud-2.8.0.dylib 0x0210f91d
wxAppConsole::HandleEvent(wxEvtHandler*, void
(wxEvtHandler::*)(wxEvent&), wxEvent&) const + 65
19 libwx_base_carbonud-2.8.0.dylib 0x021afedc
wxEvtHandler::ProcessEventIfMatches(wxEventTableEntryBase const&,
wxEvtHandler*, wxEvent&) + 212
20 libwx_base_carbonud-2.8.0.dylib 0x021b1888
wxEventHashTable::HandleEvent(wxEvent&, wxEvtHandler*) + 204
21 libwx_base_carbonud-2.8.0.dylib 0x021b19e3
wxEvtHandler::ProcessEvent(wxEvent&) + 311
22 libwx_base_carbonud-2.8.0.dylib 0x021b1a24
wxEvtHandler::ProcessEvent(wxEvent&) + 376
23 libwx_macud_core-2.8.0.dylib 0x01b17a5a
wxScrollHelperEvtHandler::ProcessEvent(wxEvent&) + 52
24 libwx_macud_core-2.8.0.dylib 0x01a13394
wxMacTopLevelMouseEventHandler(OpaqueEventHandlerCallRef*,
OpaqueEventRef*, void*) + 1461
25 libwx_macud_core-2.8.0.dylib 0x01a13ee4
wxMacTopLevelEventHandler(OpaqueEventHandlerCallRef*, OpaqueEventRef*,
void*) + 198
26 com.apple.HIToolbox 0x92772e54
_InvokeEventHandlerUPP(OpaqueEventHandlerCallRef*, OpaqueEventRef*,
void*, long (*)(OpaqueEventHandlerCallRef*, OpaqueEventRef*, void*)) +
36
27 com.apple.HIToolbox 0x925ee82b
_ZL23DispatchEventToHandlersP14EventTargetRecP14OpaqueEventRefP14HandlerCallRec
+ 1602
28 com.apple.HIToolbox 0x925edca8
_ZL30SendEventToEventTargetInternalP14OpaqueEventRefP20OpaqueEventTargetRefP14HandlerCallRec
+ 482
29 com.apple.HIToolbox 0x92602ac9 SendEventToEventTarget + 76
30 com.apple.HIToolbox 0x92603249
_ZL29ToolboxEventDispatcherHandlerP25OpaqueEventHandlerCallRefP14OpaqueEventRefPv
+ 1915
31 com.apple.HIToolbox 0x925eece6
_ZL23DispatchEventToHandlersP14EventTargetRecP14OpaqueEventRefP14HandlerCallRec
+ 2813
32 com.apple.HIToolbox 0x925edca8
_ZL30SendEventToEventTargetInternalP14OpaqueEventRefP20OpaqueEventTargetRefP14HandlerCallRec
+ 482
33 com.apple.HIToolbox 0x92602ac9 SendEventToEventTarget + 76
34 libwx_macud_core-2.8.0.dylib 0x01996a6d
wxApp::MacHandleOneEvent(void*) + 41
35 libwx_macud_core-2.8.0.dylib 0x01996b84 wxApp::MacDoOneEvent() + 214
36 libwx_macud_core-2.8.0.dylib 0x019b8734 wxEventLoop::Dispatch() + 42
37 libwx_macud_core-2.8.0.dylib 0x01a7dd84 wxEventLoopManual::Run() + 294
38 libwx_macud_core-2.8.0.dylib 0x01a4f789 wxAppBase::MainLoop() + 85
39 libwx_macud_core-2.8.0.dylib 0x01a4ef75 wxAppBase::OnRun() + 47
40 libwx_base_carbonud-2.8.0.dylib 0x02153072 wxEntry(int&, wchar_t**) + 168
41 libwx_base_carbonud-2.8.0.dylib 0x0215327b wxEntry(int&, char**) + 63
42 pgAdmin3-Debug 0x000bdec4 main + 36 (pgAdmin3.cpp:118)
43 pgAdmin3-Debug 0x0009bda5 start + 53
- In "Refresh object and children on click" mode, if I click on a
table, and then on a database, it seems to refresh the database, but
then displays the properties of the table, not the database.
Please see about fixing those issues, and then I can give it a more in
depth review.
Thanks!
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Em 10/08/2011 07:34, Dave Page escreveu: > Please see about fixing those issues, and then I can give it a more in > depth review. I fixed the items in question, and performed some more tests. Apparently it's OK. If any bugs or any suggestions, please let me know. The new patch is attached. Thank you.
On Thu, Aug 11, 2011 at 4:20 AM, Vinicius Santos <vinicius.santos.lista@gmail.com> wrote: > Em 10/08/2011 07:34, Dave Page escreveu: >> >> Please see about fixing those issues, and then I can give it a more in >> depth review. > > I fixed the items in question, and performed some more tests. > > Apparently it's OK. > > If any bugs or any suggestions, please let me know. Looking better :-) - On frmOptions, the combo box to select the refresh mode should be to the right of the label, not underneath. The sizing should following the layout of other tabs. - In "Refresh object on click" mode, if I click a table, and then click the parent database, all nodes below the "Schemas" node are collapsed. - Please use an actual declared enum for the setting, so we can avoid using "magic" numbers in execSelChange() - eg. if (settingRefreshOnClick == REFRESH_OBJECT_ONLY) -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Em 11/08/2011 07:25, Dave Page escreveu: > - On frmOptions, the combo box to select the refresh mode should be to > the right of the label, not underneath. The sizing should following > the layout of other tabs. Done. > - In "Refresh object on click" mode, if I click a table, and then > click the parent database, all nodes below the "Schemas" node are > collapsed. Done. > > - Please use an actual declared enum for the setting, so we can avoid > using "magic" numbers in execSelChange() - eg. if > (settingRefreshOnClick == REFRESH_OBJECT_ONLY) Done. Unfortunately, I have no time to make tests more heavy. If you find errors or have more suggestions, please let me know. Thank You.
Hi, I just came to review this again (following a hectic week - sorry!). Unfortunately it no longer applies as frmOptions has been redesigned. Can you update the patch please? Thanks. On Sat, Aug 13, 2011 at 5:16 AM, Vinicius Santos <vinicius.santos.lista@gmail.com> wrote: > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Em 19/08/2011 16:13, Vinicius Santos escreveu:
Only changes made in frmOptions.
Thanks!
The patch was updated. Is attached.2011/8/19 Dave Page <dpage@pgadmin.org>Hi,
I just came to review this again (following a hectic week - sorry!).
Unfortunately it no longer applies as frmOptions has been redesigned.
Can you update the patch please?
Only changes made in frmOptions.
Thanks!
On Sun, Aug 21, 2011 at 10:14 PM, Vinicius Santos <vinicius.santos.lista@gmail.com> wrote: > Em 19/08/2011 16:13, Vinicius Santos escreveu: > > 2011/8/19 Dave Page <dpage@pgadmin.org> >> >> Hi, >> >> I just came to review this again (following a hectic week - sorry!). >> Unfortunately it no longer applies as frmOptions has been redesigned. >> Can you update the patch please? > > The patch was updated. Is attached. > > Only changes made in frmOptions. Thanks - that applies cleanly now. Unfortunately, it still doesn't work properly. Please see the attached screenshot. That was taken with "refresh object and children" turned on - I clicked on the table "test" and then clicked on the parent schema "public", but the properties and SQL panes are showing the details of the table. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, 2011-08-22 at 12:04 +0100, Dave Page wrote: > On Sun, Aug 21, 2011 at 10:14 PM, Vinicius Santos > <vinicius.santos.lista@gmail.com> wrote: > > Em 19/08/2011 16:13, Vinicius Santos escreveu: > > > > 2011/8/19 Dave Page <dpage@pgadmin.org> > >> > >> Hi, > >> > >> I just came to review this again (following a hectic week - sorry!). > >> Unfortunately it no longer applies as frmOptions has been redesigned. > >> Can you update the patch please? > > > > The patch was updated. Is attached. > > > > Only changes made in frmOptions. > > Thanks - that applies cleanly now. Unfortunately, it still doesn't > work properly. Please see the attached screenshot. That was taken with > "refresh object and children" turned on - I clicked on the table > "test" and then clicked on the parent schema "public", but the > properties and SQL panes are showing the details of the table. > Did you forget to add the screenshot? I don't see it. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On Mon, Aug 22, 2011 at 12:27 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > On Mon, 2011-08-22 at 12:04 +0100, Dave Page wrote: >> On Sun, Aug 21, 2011 at 10:14 PM, Vinicius Santos >> <vinicius.santos.lista@gmail.com> wrote: >> > Em 19/08/2011 16:13, Vinicius Santos escreveu: >> > >> > 2011/8/19 Dave Page <dpage@pgadmin.org> >> >> >> >> Hi, >> >> >> >> I just came to review this again (following a hectic week - sorry!). >> >> Unfortunately it no longer applies as frmOptions has been redesigned. >> >> Can you update the patch please? >> > >> > The patch was updated. Is attached. >> > >> > Only changes made in frmOptions. >> >> Thanks - that applies cleanly now. Unfortunately, it still doesn't >> work properly. Please see the attached screenshot. That was taken with >> "refresh object and children" turned on - I clicked on the table >> "test" and then clicked on the parent schema "public", but the >> properties and SQL panes are showing the details of the table. >> > > Did you forget to add the screenshot? I don't see it. Yes. Yes I did. :-) -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Вложения
Em 22/08/2011 08:04, Dave Page escreveu: > Thanks - that applies cleanly now. Unfortunately, it still doesn't > work properly. Please see the attached screenshot. That was taken with > "refresh object and children" turned on - I clicked on the table > "test" and then clicked on the parent schema "public", but the > properties and SQL panes are showing the details of the table. I did make some tests, and now seems to be OK. Thanks again.
On Wed, Aug 24, 2011 at 3:29 AM, Vinicius Santos <vinicius.santos.lista@gmail.com> wrote: > Em 22/08/2011 08:04, Dave Page escreveu: >> >> Thanks - that applies cleanly now. Unfortunately, it still doesn't >> work properly. Please see the attached screenshot. That was taken with >> "refresh object and children" turned on - I clicked on the table >> "test" and then clicked on the parent schema "public", but the >> properties and SQL panes are showing the details of the table. > > I did make some tests, and now seems to be OK. > > Thanks again. Seems to work for me too - thanks, patch applied! -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, 2011-08-24 at 13:01 +0100, Dave Page wrote: > On Wed, Aug 24, 2011 at 3:29 AM, Vinicius Santos > <vinicius.santos.lista@gmail.com> wrote: > > Em 22/08/2011 08:04, Dave Page escreveu: > >> > >> Thanks - that applies cleanly now. Unfortunately, it still doesn't > >> work properly. Please see the attached screenshot. That was taken with > >> "refresh object and children" turned on - I clicked on the table > >> "test" and then clicked on the parent schema "public", but the > >> properties and SQL panes are showing the details of the table. > > > > I did make some tests, and now seems to be OK. > > > > Thanks again. > > Seems to work for me too - thanks, patch applied! > I think there is a huge issue on this patch. I cannot alter a column since it has been applied. I always end up with this message: There are properties dialogues open for one or more objects that would be refreshed. Please close the properties dialogues and try again. To reproduce it, it's quite simple. Open a column's property dialog, change something (set it NOT NULL or not NOT NULL), and click OK. Bingo. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On Thu, Aug 25, 2011 at 10:16 PM, Guillaume Lelarge <guillaume@lelarge.info> wrote: > I think there is a huge issue on this patch. I cannot alter a column > since it has been applied. I always end up with this message: > > There are properties dialogues open for one or more objects that would > be refreshed. Please close the properties dialogues and try again. > > To reproduce it, it's quite simple. Open a column's property dialog, > change something (set it NOT NULL or not NOT NULL), and click OK. Bingo. I rolled back Vinicius' patch and tested, and still see the error. I think it's your patch that's at fault :-( -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Em 25/08/2011 19:23, Dave Page escreveu: > I rolled back Vinicius' patch and tested, and still see the error. I > think it's your patch that's at fault:-( I tested here too, with all the lines of my patch comment, and the error yet occurs.
On Thu, 2011-08-25 at 22:14 -0300, Vinicius Santos wrote: > Em 25/08/2011 19:23, Dave Page escreveu: > > I rolled back Vinicius' patch and tested, and still see the error. I > > think it's your patch that's at fault:-( > I tested here too, with all the lines of my patch comment, and the error > yet occurs. > Turns out you're both right. My apologies. I'll fix it ASAP. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com
On Fri, 2011-08-26 at 23:10 +0200, Guillaume Lelarge wrote: > On Thu, 2011-08-25 at 22:14 -0300, Vinicius Santos wrote: > > Em 25/08/2011 19:23, Dave Page escreveu: > > > I rolled back Vinicius' patch and tested, and still see the error. I > > > think it's your patch that's at fault:-( > > I tested here too, with all the lines of my patch comment, and the error > > yet occurs. > > > > Turns out you're both right. My apologies. I'll fix it ASAP. > Fixed. Sorry about this. -- Guillaume http://blog.guillaume.lelarge.info http://www.dalibo.com