Dave Page a écrit :
On Tue, Jun 17, 2008 at 10:51 PM, Guillaume Lelarge
<[EMAIL PROTECTED]> 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();
}
}
--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
