Обсуждение: 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(-)

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

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