Saturday, July 5, 2008

Re: [pgadmin-hackers] Enabling SQL text field in the SQL tab of object dialog

Index: pgadmin/include/dlg/dlgProperty.h
===================================================================
--- pgadmin/include/dlg/dlgProperty.h (revision 7390)
+++ pgadmin/include/dlg/dlgProperty.h (working copy)
@@ -60,6 +60,8 @@
void EnableOK(bool enable);
virtual bool IsUpToDate() { return true; };
void ShowObject();
+
+ void FillSQLTextfield();

void CheckValid(bool &enable, const bool condition, const wxString &msg);
static dlgProperty *CreateDlg(frmMain *frame, pgObject *node, bool asNew, pgaFactory *factory=0);
@@ -86,6 +88,7 @@
void OnChange(wxCommandEvent &ev);
void OnChangeOwner(wxCommandEvent &ev);
void OnChangeStc(wxStyledTextEvent& event);
+ void OnChangeReadOnly(wxCommandEvent& event);

protected:
void AddUsers(ctlComboBoxFix *cb1, ctlComboBoxFix *cb2=0);
@@ -97,7 +100,7 @@
pgDatabase *database;

frmMain *mainForm;
- ctlSQLBox *sqlPane;
+ wxPanel *sqlPane;

wxTextValidator numericValidator;

@@ -105,6 +108,10 @@
wxTextCtrl *txtName, *txtOid, *txtComment;
ctlComboBox *cbOwner;
ctlComboBox *cbClusterSet;
+ wxCheckBox *chkReadOnly;
+ ctlSQLBox *sqlTextField1;
+ ctlSQLBox *sqlTextField2;
+ bool enableSQL2;

int width, height;
wxTreeItemId item, owneritem;
Index: pgadmin/dlg/dlgProperty.cpp
===================================================================
--- pgadmin/dlg/dlgProperty.cpp (revision 7390)
+++ pgadmin/dlg/dlgProperty.cpp (working copy)
@@ -59,8 +59,6 @@
#include "schema/pgUser.h"


-
-
class replClientData : public wxClientData
{
public:
@@ -72,6 +70,9 @@
};


+#define CTRLID_CHKSQLTEXTFIELD 1000
+
+
BEGIN_EVENT_TABLE(dlgProperty, DialogWithHelp)
EVT_NOTEBOOK_PAGE_CHANGED(XRCID("nbNotebook"), dlgProperty::OnPageSelect)

@@ -80,6 +81,8 @@
EVT_COMBOBOX(XRCID("cbOwner"), dlgProperty::OnChange)
EVT_TEXT(XRCID("txtComment"), dlgProperty::OnChange)

+ EVT_CHECKBOX(CTRLID_CHKSQLTEXTFIELD, dlgProperty::OnChangeReadOnly)
+
EVT_BUTTON(wxID_HELP, dlgProperty::OnHelp)
EVT_BUTTON(wxID_OK, dlgProperty::OnOK)
EVT_BUTTON(wxID_APPLY, dlgProperty::OnApply)
@@ -90,6 +93,8 @@
{
readOnly=false;
sqlPane=0;
+ sqlTextField1=0;
+ sqlTextField2=0;
processing=false;
mainForm=frame;
database=0;
@@ -117,6 +122,11 @@
txtComment = CTRL_TEXT("txtComment");
cbOwner = CTRL_COMBOBOX2("cbOwner");
cbClusterSet = CTRL_COMBOBOX2("cbClusterSet");
+
+ wxString db = wxT("Database");
+ wxString ts = wxT("Tablespace");
+ enableSQL2 = db.Cmp(factory->GetTypeName()) == 0
+ || ts.Cmp(factory->GetTypeName()) == 0;

wxNotebookPage *page=nbNotebook->GetPage(0);
wxASSERT(page != NULL);
@@ -311,8 +321,41 @@

void dlgProperty::CreateAdditionalPages()
{
- sqlPane = new ctlSQLBox(nbNotebook, CTL_PROPSQL, wxDefaultPosition, wxDefaultSize, wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_READONLY | wxTE_RICH2);
+ // create a panel
+ sqlPane = new wxPanel(nbNotebook);
+
+ // add panel to the notebook
nbNotebook->AddPage(sqlPane, wxT("SQL"));
+
+ // create a flex grid sizer
+ wxFlexGridSizer *fgsizer = new wxFlexGridSizer(1, 5, 5);
+
+ // add checkbox to the panel
+ chkReadOnly = new wxCheckBox(sqlPane, CTRLID_CHKSQLTEXTFIELD, wxT("Read only"));
+ chkReadOnly->SetValue(true);
+ fgsizer->Add(chkReadOnly, 1, wxALL | wxALIGN_LEFT, 5);
+
+ // add the first text entry box
+ sqlTextField1 = new ctlSQLBox(sqlPane, CTL_PROPSQL,
+ wxDefaultPosition, wxDefaultSize,
+ wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_RICH2);
+ fgsizer->Add(sqlTextField1, 1, wxALL | wxEXPAND, 5);
+
+ // add the second text entry box (if needed)
+ if (enableSQL2)
+ {
+ sqlTextField2 = new ctlSQLBox(sqlPane, CTL_PROPSQL,
+ wxDefaultPosition, wxDefaultSize,
+ wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_RICH2);
+ fgsizer->Add(sqlTextField2, 1, wxALL | wxEXPAND, 5);
+ }
+
+ fgsizer->AddGrowableCol(0);
+ fgsizer->AddGrowableRow(1);
+ fgsizer->AddGrowableRow(2);
+
+ sqlPane->SetAutoLayout(true);
+ sqlPane->SetSizer(fgsizer);
}


@@ -506,6 +549,85 @@
}


+void dlgProperty::OnChangeReadOnly(wxCommandEvent &ev)
+{
+ size_t pos;
+ bool showmessage;
+
+ showmessage = chkReadOnly->GetValue()
+ && ! (!enableSQL2 && GetSql().Length() == 0 && sqlTextField1->GetText().Cmp(_("-- nothing to change")) == 0)
+ && ! (!enableSQL2 && GetSql().Length() == 0 && sqlTextField1->GetText().Cmp(_("-- definition incomplete")) == 0)
+ && ! (enableSQL2 && GetSql().Length() == 0 && GetSql2().Length() == 0 && sqlTextField1->GetText().Cmp(_("-- nothing to change")) == 0 && sqlTextField2->GetText().Length() == 0)
+ && ! (enableSQL2 && GetSql().Length() == 0 && GetSql2().Length() == 0 && sqlTextField1->GetText().Cmp(_("-- definition incomplete")) == 0 && sqlTextField2->GetText().Length() == 0)
+ && (sqlTextField1->GetText().Cmp(GetSql()) != 0 || (enableSQL2 && sqlTextField2->GetText().Cmp(GetSql2()) != 0));
+
+ if (showmessage)
+ {
+ if (wxMessageBox(_("Are you sure you wish to cancel your edit?"), _("SQL editor"), wxYES_NO|wxNO_DEFAULT) == wxNO)
+ {
+ chkReadOnly->SetValue(false);
+ return;
+ }
+ }
+
+ sqlTextField1->SetReadOnly(chkReadOnly->GetValue());
+ if (enableSQL2)
+ {
+ sqlTextField2->SetReadOnly(chkReadOnly->GetValue());
+ }
+ for (pos = 0; pos < nbNotebook->GetPageCount() - 1; pos++)
+ {
+ nbNotebook->GetPage(pos)->Enable(chkReadOnly->GetValue());
+ }
+
+ if (chkReadOnly->GetValue())
+ {
+ FillSQLTextfield();
+ }
+}
+
+
+void dlgProperty::FillSQLTextfield()
+{
+ // create a function because this is a duplicated code
+ sqlTextField1->SetReadOnly(false);
+ if (enableSQL2)
+ {
+ sqlTextField2->SetReadOnly(false);
+ }
+ if (btnOK->IsEnabled())
+ {
+ wxString tmp;
+ if (cbClusterSet && cbClusterSet->GetSelection() > 0)
+ {
+ replClientData *data=(replClientData*)cbClusterSet->GetClientData(cbClusterSet->GetSelection());
+ tmp.Printf(_("-- Execute replicated using cluster \"%s\", set %ld\n"), data->cluster.c_str(), data->setId);
+ }
+ sqlTextField1->SetText(tmp + GetSql());
+ if (enableSQL2)
+ {
+ sqlTextField2->SetText(GetSql2());
+ }
+ }
+ else
+ {
+ if (GetObject())
+ sqlTextField1->SetText(_("-- nothing to change"));
+ else
+ sqlTextField1->SetText(_("-- definition incomplete"));
+ if (enableSQL2)
+ {
+ sqlTextField2->SetText(wxT(""));
+ }
+ }
+ sqlTextField1->SetReadOnly(true);
+ if (enableSQL2)
+ {
+ sqlTextField2->SetReadOnly(true);
+ }
+}
+
+
bool dlgProperty::tryUpdate(wxTreeItemId collectionItem)
{
ctlTree *browser=mainForm->GetBrowser();
@@ -608,7 +730,7 @@
mainForm->GetSqlPane()->SetReadOnly(true);
}
}
- else if (item)
+ else if (item && chkReadOnly->GetValue())
{
wxTreeItemId collectionItem=item;

@@ -753,8 +875,25 @@
return;
}

- wxString sql=GetSql();
- wxString sql2=GetSql2();
+ wxString sql;
+ wxString sql2;
+ if (chkReadOnly->GetValue())
+ {
+ sql = GetSql();
+ sql2 = GetSql2();
+ }
+ else
+ {
+ sql = sqlTextField1->GetText();
+ if (enableSQL2)
+ {
+ sql2 = sqlTextField2->GetText();
+ }
+ else
+ {
+ sql2 = wxT("");
+ }
+ }

if (!apply(sql, sql2))
{
@@ -768,27 +907,10 @@

void dlgProperty::OnPageSelect(wxNotebookEvent& event)
{
- if (sqlPane && event.GetSelection() == (int)nbNotebook->GetPageCount()-1)
+ if (sqlTextField1 && chkReadOnly->GetValue() &&
+ event.GetSelection() == (int)nbNotebook->GetPageCount()-1)
{
- sqlPane->SetReadOnly(false);
- if (btnOK->IsEnabled())
- {
- wxString tmp;
- if (cbClusterSet && cbClusterSet->GetSelection() > 0)
- {
- replClientData *data=(replClientData*)cbClusterSet->GetClientData(cbClusterSet->GetSelection());
- tmp.Printf(_("-- Execute replicated using cluster \"%s\", set %ld\n"), data->cluster.c_str(), data->setId);
- }
- sqlPane->SetText(tmp + GetSql() + GetSql2());
- }
- else
- {
- if (GetObject())
- sqlPane->SetText(_("-- nothing to change"));
- else
- sqlPane->SetText(_("-- definition incomplete"));
- }
- sqlPane->SetReadOnly(true);
+ FillSQLTextfield();
}
}

Dave Page a écrit :
> On Tue, Jul 1, 2008 at 11:52 AM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>
>> I'm at the PostgreSQL booth, for the RMLL in Mont-de-Marsan
>> (http://rmll.info). I'm trying the wifi system :)
>
> :-)
>
>> Here is the patch that fixes your issue on Win32. Works great on Win32 and
>> Linux... I'm not sure for Mac OS X though.
>
> Hmm, this still isn't looking so good I'm afraid:
>

:-/

> - If I open the properties dialogue on a database, click the SQL tab
> and then uncheck Read only, having changed nothing, I get asked if I
> want to cancel my edit. I believe this is because in the edit case,
> GetSQL() will return nothing, whilst the textbox may contain '--
> nothing to change'
>

Fixed.

> - The sizing is still wrong on Mac. Worse, the text boxes no longer
> resize on the resizeable dialogues (dlgFunction, dlgPackage,
> dlgTrigger and friends).
>
> I would suggest avoiding trying to calculate the dimensions in code -

I like this idea.

> in my experience this almost never works well. Instead, I would
> suggest laying the two textboxes and the checkbox in a sizer and
> letting that do the work. Not only should that be easier to code (once
> you fully understand sizers!), but it should work properly on all
> platforms, and be resize-friendly.
>

OK, it wasn't easy to do. But it now works. I don't specify any size and
the rendering looks great... on Linux. On Windows, it doesn't. I spent
part of the last two days to try to understand the Windows behaviour but
I failed. Do you have any idea on the Windows behavior? is there
something Windows specific when we use the flex grid sizer?

> The same should probably be done for other standard tabs (eg,
> privileges, gucs) when you update all the XRC files as you discussed
> previously.
>
> Sorry, I know this isn't what you wanted to hear - but we'll get there :-)
>

Oh no problem with me on this. I'm just worried on the number of bad
patches.

The patch attached fixed some issues:
* The "Are you sure..." message is displayed only when "Read Only" is
checked.
* No button is the default on the "Are you sure" message (which has a
weird behaviour... hitting Esc will validate the dialog and the user
will lose his changes... I'm not sure we should keep this).
* the first issue you had.
* Sizing problems fixed on Linux with a wxFlexGridSizer.

Remaining issues:
* Objects' size on Windows (and Max OS X ?).


--
Guillaume.

http://www.postgresqlfr.org

http://dalibo.com

No comments: