Re: pgAdmin III commit: Fix selection of objects in backup/restore dialogs

Le 14/03/2011 22:17, Thom Brown a écrit :
> On 12 March 2011 09:27, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
>> Fix selection of objects in backup/restore dialogs
>>
>> When a user selects a table, the schema and the database should be selected as
>> well. This patch handles this automatically.
>>
>> Report from Melvin Davidson.
>>
>> Branch
>> ------
>> REL-1_12_0_PATCHES
>>
>> Details
>> -------
>> http://git.postgresql.org/gitweb?p=pgadmin3.git;a=commitdiff;h=cbf2e0765ebf18da6256852bf53021d26287630a
>>
>> Modified Files
>> --------------
>> CHANGELOG | 2 ++
>> pgadmin/ctl/ctlCheckTreeView.cpp | 15 ++++++++++++---
>> pgadmin/include/ctl/ctlCheckTreeView.h | 1 +
>> 3 files changed, 15 insertions(+), 3 deletions(-)
>
> Sorry for the following bit of noise, but...
>
> Is there any need for this line?:
>
> if (newimage == 0 || newimage == 1)
>
> newimage is initialised as 0, and can only otherwise be set to 1, so
> I'm not sure where this logic comes in.
>

Right now, no, there's no really good reason to have this check. As you
say, there are only two states, 0 and 1, so the check is not useful. I
first added it because I was thinking about a third state. But I never
implemented it, so I guess we could get rid of the check.

> Also, I'm not clear as to why if, when newimage = 1, the parent image
> is seemingly set twice. I'm referring to the function being called
> suggesting both the parent and child image is being set, followed by
> the parent image. Shouldn't there at least be an "else if" in there,
> or is one of them misnamed?
>

I guess it's misnamed. SetParentAndChildImage() should now be
SetChildrenImage().

--
Guillaume
http://www.postgresql.fr
http://dalibo.com