Re: Patch: Query favourites - Mailing list 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