Re: Patch: Query favourites

From: "Magnus Hagander" <mha(at)sollentuna(dot)net>
To: "Dave Page" <dpage(at)vale-housing(dot)co(dot)uk>
Cc: <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Patch: Query favourites
Date: 2006-02-07 20:05:23
Message-ID: 6BCB9D8A16AC4241919521715F4D8BCEA0F760@algol.sollentuna.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

> > > > As for the libxml2/msxml - I think going with *just*
> > libxml2 is the
> > > > way to go. The APIs are so completely different that it
> > > would be two
> > > > completely different implementations. And AFAIK, there are
> > > no problems
> > > > with libxml on Win32 in general.
> >
> > Here's a version that uses libxml2 natively, including updated unix
> > build scripts. No longer any dependency on wxxml2.
>
> Finally got around to looking at this :-). Couple of things I noticed:
>
> - If the ctlSQLBox is empty, the 'Add Favourites' option is
> still enabled, but does nothing. SVN trunk calls updateMenu
> (or whatever it's
> called) on every change now, so this should be a 30 second fix.

Indeed. Tried that before, had the problem that it didn't activate,
couldn't figure out why. Seems you've fixed that one, so yes, 30 second
fix.

> - On Linux, with no favourites at all, the root node in the
> Manage Favourites tree has no icon. Add a favourite, and the
> root node gains a folder icon, but the favourites themselves
> remain without an icon. Works properly on Windows, haven't tried Mac.

Ah. Turns out there is a default image for a tree node when running on
Windows, but not when running on Linux. Cross-platform? Hmm... Fixed.

> - You can only create one new folder on the manage dialogue.
> To create mode, you need to close/re-open the dialogue.

Major logic bomb. Actually, could create more than one subfolder - as
long as the new one was added to a *different* one. It overwrote the
treeid of the parent folder instead of the child...
Along this, also fixed a bug with empty folders.

> - You can create folders with duplicate names in the same
> folder. Is this a bug? Dunno...

Dunno either, but it seems stupid. So now it refuses it.

> - The buttons on the manage dialogue should left-align with
> the side of the tree control.

Yeah. Fixed.

> - There is no doc update. An update to the build instructions
> on the website would also be nice :-)

Hmm. It was included in the first one, missed it in the libxml version.
Included again in this one.

Build instructions attached as a separate patch. Don't have an env to
test it in, but it should be simple 'nuf.

> On a related note, this introduces dependencies on libxml2 and iconv.
> These are both available from www.xmlsoft.org, precompiled
> for Windows, and are both on most unixes already, however, on
> Windows there is no standard place for them to live. There
> are two sensible options I can see for these, and wxWidgets:
>
> C:\wxWidgets-2.6\
> C:\libxml2\
> C:\iconv\
>
> Or
>
> C:\pgadmin-prereqs (or some other name)
> \wxWidgets-2.6\
> \libxml2\
> \iconv\
>
> Thoughts/preferences?

I like the latter. Even better if it could be made a relative directory
wrt to the pgadmin directory. Say I have c:\src, then I could have
c:\src\pgadmin3 and c:\src\pgamdin-preqreqs. Or so.

> Ooops, forgot one - I get:
>
> precomp.cpp
> c:\documents and settings\dpage\my
> documents\svn\pgadmin3\src\include\favourites.h(43) : warning C4284:
> return type for
> 'queryFavouriteArray::reverse_iterator::operator ->' is
> 'class queryFavouriteItem ** ' (ie; not a UDT or reference to a UDT.
> Wil
> l produce errors if applied using infix notation)
> c:\documents and settings\dpage\my
> documents\svn\pgadmin3\src\include\favourites.h(43) : warning C4284:
> return type for 'queryFavouriteArray::const_reverse_iterator::operator
> ->' is 'class queryFavouriteItem *const * ' (ie; not a UDT or
> reference
> to
> a UDT. Will produce errors if applied using infix notation)
>
> Compiling in VC++ 6.0

Doesn't show up in VC7 or GCC. Google told me to use
WX_DEFINE_ARRAY_PTR() instead of WX_DEFINE_ARRAY(). Can't test it
though. (Google also says the warning is harmless, but if there is a way
to make it go away it should be done)

Unix build system patch unchanged and thus not included.

//Magnus

Attachment Content-Type Size
pgadmin_favourites_libxml.zip application/x-zip-compressed 8.8 KB
pgadmin_favourites_web.patch application/octet-stream 611 bytes

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Edward Di Geronimo Jr. 2006-02-07 21:37:55 More data edit grid fixes
Previous Message Bastiaan Wakkie 2006-02-07 14:19:58 Re: FW: Sorting and filtering in PgAdmin