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

Dave Page a écrit :
> On Tue, Jun 17, 2008 at 10:51 PM, Guillaume Lelarge
> <guillaume@lelarge.info> wrote:
>> Dave Page a écrit :
>>> 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.
>
> Right, but my point is that this is going to make it more likely to occur.
>
> I wonder if we need some reference counting on objects in the tree,
> and only allow any kind of refresh if all child nodes have a ref count
> of zero. I'm not entirely sure how we'd implement that.
>
>> 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?
>
> - Can we only have the dual textboxes on the dialogues that actually
> need them please?
>

Done. I thought we didn't hide UI objects, only disable them, to get a
more consistant UI.

> - The 'do you want to lose your changes' prompt  should only be shown
> if changes have actually been made.
>

Done too.

> - There is a drawing artifact on Mac. I may have to look at that after
> you commit if you have no access to suitable hardware (maybe you can
> get JPA to spring for a Mac Mini - they're nice and cheap :-p )
>

I forwarded this mail to jpa and damien, just to know what they think
about your great idea :) Unfortunately, no answer yet.

The button text doesn't please me. Neither do the message box text. If
one has a better idea, I'll be glad to know about it.

Cheers.


--
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,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 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;
@@ -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,7 +321,50 @@

 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 ctlSQLBoxes to the panel
+
+    if (enableSQL2)
+    {
+        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);
+    }
+    else
+    {
+        sqlTextField1 = new ctlSQLBox(sqlPane, CTL_PROPSQL,
+            wxPoint(zeroPos.x, zeroPos.y + chkSize.GetHeight()),
+            wxSize(width - 2 * zeroPos.x, (height - 3 * zeroPos.y)),
+            wxTE_MULTILINE | wxSUNKEN_BORDER | wxTE_RICH2);
+    }
+
+    // add panel to the notebook
     nbNotebook->AddPage(sqlPane, wxT("SQL"));
 }

@@ -506,6 +559,78 @@
 }


+void dlgProperty::OnChangeReadOnly(wxCommandEvent &ev)
+{
+    size_t pos;
+
+    if (sqlTextField1->GetText().Cmp(GetSql()) != 0 ||
+        (enableSQL2 && sqlTextField2->GetText().Cmp(GetSql2()) != 0))
+    {
+        if (wxMessageBox(_("Are you sure you wish to cancel your edit?"), _("SQL editor"), wxYES_NO) == 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 +733,7 @@
             mainForm->GetSqlPane()->SetReadOnly(true);
         }
     }
-    else if (item)
+    else if (item && chkReadOnly->GetValue())
     {
         wxTreeItemId collectionItem=item;

@@ -753,8 +878,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 +910,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();
     }
 }