Thread: Refresh objects on Click

Refresh objects on Click

From
Vinicius Santos
Date:
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

Re: Refresh objects on Click

From
Dave Page
Date:
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

Re: Refresh objects on Click

From
Vinicius Santos
Date:
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.

Re: Refresh objects on Click

From
Dave Page
Date:
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

Re: Refresh objects on Click

From
Vinicius Santos
Date:
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.



Re: Refresh objects on Click

From
Vinicius Santos
Date:

Re: Refresh objects on Click

From
Dave Page
Date:
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

Re: Refresh objects on Click

From
Vinicius Santos
Date:
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!

Re: Refresh objects on Click

From
Dave Page
Date:
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

Re: Refresh objects on Click

From
Guillaume Lelarge
Date:
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


Re: Refresh objects on Click

From
Dave Page
Date:
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

Attachment

Re: Refresh objects on Click

From
Vinicius Santos
Date:
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.

Re: Refresh objects on Click

From
Dave Page
Date:
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

Re: Refresh objects on Click

From
Guillaume Lelarge
Date:
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


Re: Refresh objects on Click

From
Dave Page
Date:
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

Re: Refresh objects on Click

From
Vinicius Santos
Date:
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.

Re: Refresh objects on Click

From
Guillaume Lelarge
Date:
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


Re: Refresh objects on Click

From
Guillaume Lelarge
Date:
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