Re: [pgScript patch] Output + bug fix

Hi!

Attached is a modified version of this patch, with one question left to
solve. Changes are:

* Moved the space after the ] in the prefix into the prefix instead, so
it doesn't have to be included in every string everywhere.
* Made strings translatable (excluding the prefix)
* Moved generate_spaces() into misc.cpp as it can be useful in many
places outside pgScript, and reimplemented it using Pad() to make it
simpler.


One question that remains: please see pgsParameterException.cpp line 28.
Should that not also use generate_spaces() and be dependent on the
prefix length?


//Magnus

Mickael Deloison wrote:
> Hi pgAdmin hackers,
>
> Here is a patch for pgAdmin. I have finally completed the patch for
> the pgScript outputs and am sorry for the delay.
> This patch also corrects a bug in pgsPrintStmt.cpp because of a thread
> double lock in case of exception.
> I have other updates to send but I would like this one to be committed first.
>
> I have noticed while making this patch that the
> pgQueryThread::GetMessagesAndClear() method does not return anything
> when my query returns a warning. It did before.
> The line 185 is:
>   conn->SetLastResultError(NULL);
> Before, it was:
>   appendMessage(conn->GetLastError() + wxT("\n"));
> When I put this line back to what it was, I do not have the problem
> anymore. Is this a problem or is there another way to get the last
> warning message?
>
> Best regards,
> Mickael
>
>
> ------------------------------------------------------------------------
>
>

Index: include/pgscript/pgScript.h
===================================================================
--- include/pgscript/pgScript.h    (revision 7650)
+++ include/pgscript/pgScript.h    (working copy)
@@ -28,6 +28,12 @@
 #include <wx/txtstrm.h>
 #define pgsOutputStream wxTextOutputStream

+const wxString PGSOUTPGSCRIPT (wxT("[PGSCRIPT ] "));
+const wxString PGSOUTEXCEPTION(wxT("[EXCEPTION] "));
+const wxString PGSOUTQUERY    (wxT("[QUERY    ] "));
+const wxString PGSOUTWARNING  (wxT("[WARNING  ] "));
+const wxString PGSOUTERROR    (wxT("[ERROR    ] "));
+
 /*** LOGGING ***/

 #include "utils/sysLogger.h"
Index: include/utils/misc.h
===================================================================
--- include/utils/misc.h    (revision 7650)
+++ include/utils/misc.h    (working copy)
@@ -100,6 +100,8 @@
 wxDateTime StrToDateTime(const wxString &value);
 OID StrToOid(const wxString& value);

+wxString generate_spaces(int length);
+
 // nls aware
 wxString BoolToYesNo(bool value);
 wxString NumToStr(long value);
Index: pgscript/exceptions/pgsArithmeticException.cpp
===================================================================
--- pgscript/exceptions/pgsArithmeticException.cpp    (revision 7650)
+++ pgscript/exceptions/pgsArithmeticException.cpp    (working copy)
@@ -24,6 +24,7 @@

 const wxString pgsArithmeticException::message() const
 {
-    return wxString() << wxT("[EXCEPT] Arithmetic Exception - Operation impossible between ")
-            << m_left << wxT(" and ") << m_right;
+    return wxString() << PGSOUTEXCEPTION <<
+        wxString::Format(_("Arithmetic Exception - Operation impossible between '%s' and '%s'"),
+            m_left.c_str(), m_right.c_str());
 }
Index: pgscript/exceptions/pgsCastException.cpp
===================================================================
--- pgscript/exceptions/pgsCastException.cpp    (revision 7650)
+++ pgscript/exceptions/pgsCastException.cpp    (working copy)
@@ -24,6 +24,7 @@

 const wxString pgsCastException::message() const
 {
-    return wxString() << wxT("[EXCEPT] Cast Exception - Cannot convert ")
-            << m_value << wxT(" to ") << m_type;
+    return wxString() << PGSOUTEXCEPTION <<
+            wxString::Format(_(" Cast Exception - Cannot convert '%s' to '%s'"),
+                             m_value.c_str(), m_type.c_str());
 }
Index: pgscript/exceptions/pgsInterruptException.cpp
===================================================================
--- pgscript/exceptions/pgsInterruptException.cpp    (revision 7650)
+++ pgscript/exceptions/pgsInterruptException.cpp    (working copy)
@@ -24,5 +24,5 @@

 const wxString pgsInterruptException::message() const
 {
-    return wxT("[EXCEPT] pgScript interrupted");
+    return wxString() << PGSOUTEXCEPTION << _("pgScript interrupted");
 }
Index: pgscript/exceptions/pgsParameterException.cpp
===================================================================
--- pgscript/exceptions/pgsParameterException.cpp    (revision 7650)
+++ pgscript/exceptions/pgsParameterException.cpp    (working copy)
@@ -25,7 +25,8 @@
 const wxString pgsParameterException::message() const
 {
     wxString message(m_message);
-    message.Replace(wxT("\n"), wxT("\n         "));
-    return wxString() << wxT("[EXCEPT] Parameter Exception - Some parameters ")
-            << wxT("are invalid:\n>> ") << message;
+    message.Replace(wxT("\n"), wxT("\n         ")); // FIXME: use length of PGSOUTEXCEPTION?
+    return wxString() << PGSOUTEXCEPTION <<
+            wxString::Format(_("Parameter Exception - Some parameters are invalid:\n>> %s"),
+                message.c_str());
 }
Index: pgscript/exceptions/pgsAssertException.cpp
===================================================================
--- pgscript/exceptions/pgsAssertException.cpp    (revision 7650)
+++ pgscript/exceptions/pgsAssertException.cpp    (working copy)
@@ -24,5 +24,5 @@

 const wxString pgsAssertException::message() const
 {
-    return wxString() << wxT("[EXCEPT] Assert Exception - ") << m_message;
+    return wxString() << PGSOUTEXCEPTION << _("Assert Exception - ") << m_message;
 }
Index: pgscript/expressions/pgsExecute.cpp
===================================================================
--- pgscript/expressions/pgsExecute.cpp    (revision 7650)
+++ pgscript/expressions/pgsExecute.cpp    (working copy)
@@ -116,12 +116,13 @@
                     {
                         m_app->LockOutput();

-                        (*m_cout) << wxT("[WRNING] ");
+                        (*m_cout) << PGSOUTWARNING;
                         wxString message(stmt + wxT("\n") + thread
                                 .GetMessagesAndClear().Strip(wxString::both));
                         wxRegEx multilf(wxT("(\n)+"));
                         multilf.ReplaceAll(&message, wxT("\n"));
-                        message.Replace(wxT("\n"), wxT("\n         "));
+                        message.Replace(wxT("\n"), wxT("\n")
+                                + generate_spaces(PGSOUTWARNING.Length() + 1));
                         (*m_cout) << message << wxT("\n");

                         m_app->UnlockOutput();
@@ -133,7 +134,7 @@
                     {
                         m_app->LockOutput();

-                        (*m_cout) << wxT("[NOTICE] ");
+                        (*m_cout) << PGSOUTQUERY;
                         wxString message(thread.GetMessagesAndClear()
                                 .Strip(wxString::both));
                         if (!message.IsEmpty())
@@ -142,7 +143,8 @@
                             message = stmt;
                         wxRegEx multilf(wxT("(\n)+"));
                         multilf.ReplaceAll(&message, wxT("\n"));
-                        message.Replace(wxT("\n"), wxT("\n         "));
+                        message.Replace(wxT("\n"), wxT("\n")
+                                + generate_spaces(PGSOUTQUERY.Length() + 1));
                         (*m_cout) << message << wxT("\n");

                         m_app->UnlockOutput();
Index: pgscript/statements/pgsPrintStmt.cpp
===================================================================
--- pgscript/statements/pgsPrintStmt.cpp    (revision 7650)
+++ pgscript/statements/pgsPrintStmt.cpp    (working copy)
@@ -11,6 +11,7 @@
 #include "pgAdmin3.h"
 #include "pgscript/statements/pgsPrintStmt.h"

+#include "pgscript/exceptions/pgsException.h"
 #include "pgscript/utilities/pgsThread.h"
 #include "pgscript/utilities/pgsUtilities.h"

@@ -33,8 +34,20 @@
         m_app->LockOutput();
     }

-    m_cout << wxT("[OUTPUT] ") << wx_static_cast(const wxString,
-            m_var->eval(vars)->value()) << wxT("\n");
+    try
+    {
+        m_cout << PGSOUTPGSCRIPT << wx_static_cast(const wxString,
+                m_var->eval(vars)->value()) << wxT("\n");
+    }
+    catch (const pgsException &)
+    {
+        if (m_app != 0)
+        {
+            m_app->UnlockOutput();
+        }
+
+        throw;
+    }

     if (m_app != 0)
     {
Index: pgscript/statements/pgsStmtList.cpp
===================================================================
--- pgscript/statements/pgsStmtList.cpp    (revision 7650)
+++ pgscript/statements/pgsStmtList.cpp    (working copy)
@@ -82,7 +82,7 @@
                     m_app->LockOutput();
                 }

-                m_cout << wxT("[ERROR] Unknown exception:\n")
+                m_cout << PGSOUTERROR << _("Unknown exception:\n")
                         << wx_static_cast(const wxString,
                                 wxString(e.what(), wxConvUTF8));
                 m_exception_thrown = true;
Index: utils/misc.cpp
===================================================================
--- utils/misc.cpp    (revision 7650)
+++ utils/misc.cpp    (working copy)
@@ -132,6 +132,10 @@
     return (OID)strtoul(value.ToAscii(), 0, 10);
 }

+wxString generate_spaces(int length)
+{
+    return wxString().Pad(length);
+}

 wxString NumToStr(double value)
 {