Re: Enabling SQL text field in the SQL tab of object dialog - Mailing list pgadmin-hackers

Guillaume Lelarge a écrit :
> Dave Page a écrit :
>> On Sat, Jun 14, 2008 at 3:01 PM, Guillaume Lelarge
>> <guillaume@lelarge.info> wrote:
>>
>>> Finally, here is the prototype. As I first talked about this one year
>>> ago, I
>>> will summarize the idea : adding a checkbox on the SQL tab of an
>>> object's
>>> properties to let the user change the SQL query. Checking will
>>> disable the
>>> contents of the other tabs because we don't want to try to reverse
>>> engineer
>>> the user's changes.
>>>
>>> So, here is the patch that does this. I'm sure there's work left to
>>> do (most
>>> notably some duplicate code) but, at least, it works for me on two
>>> different
>>> scenarios : changing the SQL query and adding another SQL query.
>>
>> I haven't tested yet (just reviewed the diff), but a couple of
>> thoughts come to mind:
>>
>> - Can we disable the controls on the form, rather than the tabs so the
>> user can still browse the object details after modfying the SQL? (Or
>> does disabling the tab effectively do that?). The downside of that is
>> that we'd need to keep track of which controls were already disabled
>> when we disable them all, so if re-enabling them we end up in the
>> original state. I realise this is not what I suggested previously.
>>
>
> You can't really disable a tab. You disable the page of the tab, so you
> can still browse the different tabs.
>
>> - Before returning to GUI mode, we should warn the user (if he has
>> modified the SQL) that his changes will be lost. Of he accepts the
>> change, the SQL should be regenerated immediately.
>>
>
> The patch does not warn the user, but I think it would be a good
> addition. But the SQL is regenerated as soon as the user clicks on the
> checkbox a second time.
>
>> So, before I test this on my lapdog, I'm sure we all want to know -
>> did anything actually explode?
>>
>
> AFAIK, no. Nothing exploded.
>

Grmbl... There's a big flaw in my patch. You're right once again.

Problem is, pgAdmin divide the queries to launch in two groups, only for
  some objects : database and tablespace. The SQL textfield contains the
concatenation of the two groups. So we can't let the user change this
field if we aren't able to separate the two groups.

I think there are two possible ways to deal with this:

  * the simpler, but confusing, one: add another textfield, put each
    group in one textfield;
  * the difficult, but less error prone, one: prevent the user to use the
    read/write mode when he's on the database's properties (and the same
    for tablespace).

I prefer the latter because it's less confusing for the user (why two
SQL textfields?), less error prone (what happens if the user takes SQL
textfield 1's contents and put it in SQL textfield 2?).

Desactivating the read/write mode for database and tablespace doesn't
seem a big issue to me, does it?

Comments?


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com