I think it's fine to keep it in the combo.
If it's all fixed now, pls go ahead and commit. :-)
On 4/23/10, Guillaume Lelarge <guillaume@lelarge.info> wrote:
> Le 23/04/2010 15:00, Dave Page a écrit :
>> On Thu, Apr 22, 2010 at 8:26 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>>> Well, we didn't do it for the actual Servers node. I don't think we
>>> should do it for this one.
>>
>> Right - that's why I wasn't sure. And now that groups are root nodes,
>> it seems even less important.
>>
>>> Finally, I had time to work on this. I changed my patch so that it
>>> follows what you expected to see. I was not really able to reproduce the
>>> problem you explain above after my changes.
>>>
>>> Can you try this last one? Thanks.
>>
>> Still seeing a crash on OSX, but mostly I think it's good now. Here's
>> what happened:
>>
>> - I cleared my prefs file so I just had 4 servers detected and added
>> to the 'Servers' group at startup.
>>
>> - I moved 3 of the servers to a new 'PostgreSQL' group.
>>
>> - I moved 1 server to a new 'Advanced Server' group.
>>
>> At this point I had three groups visible:
>>
>> Servers (0)
>> PostgreSQL (3)
>> - Server 1
>> - Server 2
>> - Server 3
>> Advanced Server (1)
>> Server 4
>>
>> Gripe 1: I think the Servers group should be hidden if it's empty. In
>> previous versions it didn't really matter as the first thing you'd do
>> is add a server. Now though, it might remain empty forever.
>>
>
> Fixed. I still keep a "Servers" item in the combobox, even if there is
> no "Servers" group. No sure this is really smart. Should I get rid of it?
>
>> Gripe 2: I think the groups should be shown in alphabetical order.
>> Should be a one-line change :-)
>>
>
> Fixed.
>
>> - I then right-clicked Server 4 and selected Properties
>>
>> Gripe 3: pgAdmin crashes reliably at this point:
>>
>
> Fixed.
>
> Thanks for your comments. See patch attached.
>
>
> --
> Guillaume.
> http://www.postgresqlfr.org
> http://dalibo.com
>
--
Dave Page
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise Postgres Company
