Re: wxWidgets 2.9 build

On Sun, Jan 16, 2011 at 6:47 PM, Peter Geoghegan
<peter(dot)geoghegan86(at)gmail(dot)com> wrote:
> On 15 January 2011 21:26, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>> The switch from carbon to cocoa shouldn't require any effort - thats
>> the work the wxWidgets guys have done.
>
> Sure, but I would have imagined that there might be one or two small
> things to clean up.

That's not problem. I thought you were implying large changes might be
necessary, which should not be the case.

> I've whittled down the amount of output from make to a mere 250kb.

Nice.

> The main culprit that breaks compatibility here seems to be multiple
> conversion operators in wxString, in particular the addition of a new
> operator const void*(). This is apparently used as a hack to get
> greater type safety by creating an ambiguity that prevents code from
> compiling that relies on questionable implicit casts. If I came up
> with this, I definitely would have called it "the three stooges
> idiom". Conversion operators are a bit frowned upon, and it seems a
> bit strange that wxString has both a conversion operator and a
> std::string style c_str() (in fact, multiple variants of both), but I
> guess it's a legacy thing.

From what I can see in a quick glance at the docs, the only conversion
operator is to const char* in 2.8, so I guess that is a legacy thing
as you suggest. There are good reasons for the different conversion
members like c_str() and mb_str().

> Why does sysSettings::Write(const wxString&, const wxChar*) take a
> const wxChar* as its second argument rather than just a wxString? I'm
> inclined to just use a const wxString&, and avoid c strings if at all
> possible, which resolves the ambiguity in various places. Obviously we
> can't have null references, so I'm going to write workarounds where
> that was possible before, where there might have been different
> codepaths for NULL and so on.

Excellent question - especially as it just calls wxConfig::Write,
which is basically "bool wxConfigBase::Write(const wxString& key,
const wxString& value)"

So unless I'm missing something, it can just be changed.

> I'm changing our use of wxCalendarCtrl to wxGenericCalendarCtrl where
> required but not where unavailable. We won't get 2.9's new native GTK
> calendar controls on that platform, but I don't suppose it matters too
> much. Nothing changes, and nothing is broken. If we could afford to
> have the GTK calendar, it wouldn't have broken our code, because its
> interface is a subset of the old wxCalendarCtrl/current
> wxGenericCalendarCtrl interface. I'll write a typedef called
> wxCompatCalendar for either wxCalendarCtrl or wxGenericCalendarCtrl,
> depending on a preprocessor macro wxCHECK_VERSION(2, 9, 0) that
> indicates Wx version is 2.9+. This is sort of like WxChar (which can
> be char or wchar_t, depending on whether or not we're doing a unicode
> build or an ANSI build). I'm still working on this.

OK.

> in pgsDictionaryGen.cpp, why do we do this?:
>
>                        wxString line;
>                        while ((line = text.ReadLine()) && !input.Eof())
>                        {
>                                ++result;
>                        }
>
> We used to implicitly cast to WxChar*, but the ambiguity now occurs.
> It looks like the three stooges idiom has worked in our favour here.
> This piece of code looks broken, like someone has made the classic
> mistake of using an assignment operator in place of a comparison
> operator . Please comment.

It's getting late and I've had an extremely stressful weekend, so
maybe I'm missing something obvious but I don't see an issue there.
It's reading from the stream (text) line by line into a dummy variable
(line), and counting the lines until it gets to the end of the file
(input).

> I saw see lots and lots of errors like this:
>
> ../pgadmin/include/utils/sysLogger.h:52:1: error: expected initializer
> before ‘ATTRIBUTE_PRINTF_1’
>
> ATTRIBUTE_PRINTF_1 isn't #define'd. It's called WX_ATTRIBUTE_PRINTF_1
> is 2.9, so I've written a new macro called COMPAT_ATTRIBUTE_PRINTF_1
> that is conditionally #define'd as either variant.

OK.

> I'm going to go out now. I'll take another look tomorrow. It looks
> like supporting both 2.8 and 2.9 is quite possible.

Sounds like it.

How did the OGL port go? I looked at that briefly, and had a rough
build in 10 minutes or so iirc. Oh, and in answer to your previous
comments on the distribution; you're quite correct - the wxWidgets
licence isn't suitable for distribution with pgAdmin. We'd have to get
the patch applied upstream, or host our own port. If we need to do the
latter, then I guess we should also setup our own GIT repo.

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company