Thread: pgAdmin III commit: Include a variant of sysSettings::Write() that take
Include a variant of sysSettings::Write() that takes a wxChar* value to write, as they are currently being cast to bools and stored as true/false. Branch ------ master Details ------- http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=6045df09a32326504720d4a165fb81cc7949c05d Modified Files -------------- pgadmin/include/utils/sysSettings.h | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
Re: pgAdmin III commit: Include a variant of sysSettings::Write() that take
From
Peter Geoghegan
Date:
On 16 February 2011 15:47, Dave Page <dpage@pgadmin.org> wrote: > Include a variant of sysSettings::Write() that takes a wxChar* value > to write, as they are currently being cast to bools and stored as > true/false. I anticipated this, and my latest patch doesn't have this problem - it's the same situation as ctlListView, where we changed AppendItem(const wxString&, bool) to AppendYesNoItem(const wxString&, bool). I changed bool Write(const wxString&, bool) to bool WriteBool(const wxString&, bool) in sysSettings's case. I think that we perhaps overload too much in both of those classes. The only reason that a call like this doesn't work with my code presently: sysSettings foo; foo.Write(true); is because we have an ambiguous conversion error (should it be a long or an int?). -- Regards, Peter Geoghegan
On Wed, Feb 16, 2011 at 4:20 PM, Peter Geoghegan <peter.geoghegan86@gmail.com> wrote: > On 16 February 2011 15:47, Dave Page <dpage@pgadmin.org> wrote: >> Include a variant of sysSettings::Write() that takes a wxChar* value >> to write, as they are currently being cast to bools and stored as >> true/false. > > I anticipated this, and my latest patch doesn't have this problem - > it's the same situation as ctlListView, where we changed > AppendItem(const wxString&, bool) to AppendYesNoItem(const wxString&, > bool). I changed bool Write(const wxString&, bool) to bool > WriteBool(const wxString&, bool) in sysSettings's case. The main issue I have with that is that we now have a bunch of overloaded Write() members, and WriteBool(). If we're going to deviate away from the API in wxConfig (which at least is private), then we should do so consistently. FYI, in wxPython and wxPerl they implement the following: Write(key, value) Writes a string WriteInt(key, value) Writes an integer WriteFloat(key, value) Writes a floating point number WriteBool(key, value) Writes a boo -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: pgAdmin III commit: Include a variant of sysSettings::Write() that take
From
Peter Geoghegan
Date:
On 16 February 2011 18:42, Dave Page <dpage@pgadmin.org> wrote: > The main issue I have with that is that we now have a bunch of > overloaded Write() members, and WriteBool(). If we're going to deviate > away from the API in wxConfig (which at least is private), then we > should do so consistently. > > FYI, in wxPython and wxPerl they implement the following: > > Write(key, value) Writes a string > WriteInt(key, value) Writes an integer > WriteFloat(key, value) Writes a floating point number > WriteBool(key, value) Writes a boo I totally agree. What about ctlListView? The fact that its AppendYesNoItem() member function doesn't overload AppendItem() is, as I said at the time, logical, because you aren't actually appending a true or false value - you're appending a "Yes" or a "No". -- Regards, Peter Geoghegan
On Wed, Feb 16, 2011 at 6:50 PM, Peter Geoghegan <peter.geoghegan86@gmail.com> wrote: > On 16 February 2011 18:42, Dave Page <dpage@pgadmin.org> wrote: >> The main issue I have with that is that we now have a bunch of >> overloaded Write() members, and WriteBool(). If we're going to deviate >> away from the API in wxConfig (which at least is private), then we >> should do so consistently. >> >> FYI, in wxPython and wxPerl they implement the following: >> >> Write(key, value) Writes a string >> WriteInt(key, value) Writes an integer >> WriteFloat(key, value) Writes a floating point number >> WriteBool(key, value) Writes a boo > > I totally agree. What about ctlListView? The fact that its > AppendYesNoItem() member function doesn't overload AppendItem() is, as > I said at the time, logical, because you aren't actually appending a > true or false value - you're appending a "Yes" or a "No". Yeah, I think that one is OK - AppendItem() is akin to Write() - ie. string by default - where AppendYesNoItem() is (sort of) akin to WriteBool(). -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company