Re: Enabling SQL text field in the SQL tab of object dialog

Dave Page a écrit :
> On Tue, Jun 17, 2008 at 8:59 AM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>
>>> That can be
>>> potentially dangerous if the user has another property dialogue for a
>>> sibling object open though (actually that's an issue now iirc - this
>>> would just make it far more likely to occur).
>>>
>> What kind of issue are you talking about? can you give me an example?
>
> When you open a properties dialogue, it gets passed a pointer to the
> pgObject which it may use right up until it is closed. If you refresh
> part of the tree, you delete and recreate all the pgObjects under the
> node you refresh, so the dialogue can end up with a pointer to an
> object that's been deleted.
>

OK, but this is already an issue.

Here is patch revision 2. It creates a new textfield for the second SQL
query, and it takes care of the refresh of the tree. This patch seems to
resolve all issues I could find (apart from the one above).

Comments?


--
Guillaume.
  http://www.postgresqlfr.org
  http://dalibo.com
Index: pgadmin/include/dlg/dlgProperty.h
===================================================================
--- pgadmin/include/dlg/dlgProperty.h    (revision 7376)
+++ 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,9 @@
     wxTextCtrl *txtName, *txtOid, *txtComment;
     ctlComboBox *cbOwner;
     ctlComboBox *cbClusterSet;
+    wxCheckBox *chkReadOnly;
+    ctlSQLBox *sqlTextField1;
+    ctlSQLBox *sqlTextField2;

     int width, height;
     wxTreeItemId item, owneritem;
Index: pgadmin/dlg/dlgProperty.cpp
===================================================================
--- pgadmin/dlg/dlgProperty.cpp    (revision 7376)
+++ 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;
@@ -311,7 +316,39 @@

 void dlgProperty::CreateAdditionalPages()
 {
-    sqlPane = new ctlSQLBox(nbNotebook, CTL_PROPSQL, wxDefaultPosition, wxDefaultSize, wxTE_MULTILINE |
wxSUNKEN_BORDER| wxTE_READONLY | wxTE_RICH2); 
+    int width, height;
+
+    // get a few sizes and widths
+#ifdef __WIN32__
+    GetClientSize(&width, &height);
+#else
+    nbNotebook->GetClientSize(&width, &height);
+    height -= ConvertDialogToPixels(wxPoint(0, 20)).y;   // sizes of tabs
+#endif
+    wxPoint zeroPos=ConvertDialogToPixels(wxPoint(5, 5));
+    wxSize chkSize=ConvertDialogToPixels(wxSize(65,12));
+
+    // add a panel
+    sqlPane = new wxPanel(nbNotebook);
+
+    // add checkbox to the panel
+    chkReadOnly = new wxCheckBox(sqlPane, CTRLID_CHKSQLTEXTFIELD, wxT("Read only"),
+      wxPoint(zeroPos.x, zeroPos.y),
+      chkSize);
+    chkReadOnly->SetValue(true);
+
+    // add ctlSQLBox to the panel
+    sqlTextField1 = new ctlSQLBox(sqlPane, CTL_PROPSQL,
+      wxPoint(zeroPos.x, zeroPos.y + chkSize.GetHeight()),
+      wxSize(width - 2 * zeroPos.x, (height - 3 * zeroPos.y) / 2),
+      wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_RICH2);
+
+    sqlTextField2 = new ctlSQLBox(sqlPane, CTL_PROPSQL,
+      wxPoint(zeroPos.x, zeroPos.y + chkSize.GetHeight() + (height - 3 * zeroPos.y) / 2),
+      wxSize(width - 2 * zeroPos.x, (height - 3 * zeroPos.y) / 2),
+      wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_RICH2);
+
+    // add panel to the notebook
     nbNotebook->AddPage(sqlPane, wxT("SQL"));
 }

@@ -506,6 +543,54 @@
 }


+void dlgProperty::OnChangeReadOnly(wxCommandEvent &ev)
+{
+    size_t pos;
+
+    sqlTextField1->SetReadOnly(chkReadOnly->GetValue());
+    sqlTextField2->SetReadOnly(chkReadOnly->GetValue());
+    for (pos = 0; pos < nbNotebook->GetPageCount() - 1; pos++)
+    {
+        nbNotebook->GetPage(pos)->Enable(chkReadOnly->GetValue());
+    }
+
+    if (chkReadOnly->GetValue())
+    {
+        if (wxMessageBox(_("Are you sure you wish to cancel your edit?"), _("SQL editor"), wxYES_NO) == wxNO)
+            return;
+    }
+}
+
+
+void dlgProperty::FillSQLTextfield()
+{
+    // create a function because this is a duplicated code
+    sqlTextField1->SetReadOnly(false);
+    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());
+        sqlTextField2->SetText(GetSql2());
+    }
+    else
+    {
+        if (GetObject())
+            sqlTextField1->SetText(_("-- nothing to change"));
+        else
+            sqlTextField1->SetText(_("-- definition incomplete"));
+        sqlTextField2->SetText(wxT(""));
+    }
+    sqlTextField1->SetReadOnly(true);
+    sqlTextField2->SetReadOnly(true);
+}
+
+
 bool dlgProperty::tryUpdate(wxTreeItemId collectionItem)
 {
     ctlTree *browser=mainForm->GetBrowser();
@@ -608,7 +693,7 @@
             mainForm->GetSqlPane()->SetReadOnly(true);
         }
     }
-    else if (item)
+    else if (item && chkReadOnly->GetValue())
     {
         wxTreeItemId collectionItem=item;

@@ -753,8 +838,18 @@
         return;
     }

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

     if (!apply(sql, sql2))
     {
@@ -768,27 +863,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();
     }
 }