Refactor Catalog::Validate() to not duplicate code

vabaME · Mar 21, 2023 · 767d348 · 767d348
1 parent 7053a1f
commit 767d348
Show file tree

Hide file tree

Showing 9 changed files with 48 additions and 75 deletions.
diff --git a/src/catalog.cpp b/src/catalog.cpp
@@ -1105,6 +1105,28 @@ wxString CatalogItem::GetOldMsgid() const
 }
 
 
+Catalog::ValidationResults Catalog::Validate(const wxString& /*fileWithSameContent*/)
+{
+    ValidationResults res;
+
+    for (auto& i: m_items)
+        i->ClearIssue();
+    res.errors = 0;
+
+    if (!HasCapability(Catalog::Cap::Translations))
+        return res; // no errors in POT files
+
+#if wxUSE_GUI
+    if (Config::ShowWarnings())
+    {
+        res.warnings = QAChecker::GetFor(*this)->Check(*this);
+    }
+#endif
+
+    return res;
+}
+
+
 void Catalog::PostCreation()
 {
     if (!m_sourceLanguage.IsValid())

diff --git a/src/catalog.h b/src/catalog.h
@@ -582,7 +582,7 @@ class Catalog
 
         /// Validates correctness of the translation by running msgfmt
         /// Returns number of errors (i.e. 0 if no errors).
-        virtual ValidationResults Validate(bool wasJustLoaded = false) = 0;
+        virtual ValidationResults Validate(const wxString& fileWithSameContent = wxString());
 
         void AttachCloudSync(std::shared_ptr<CloudSyncDestination> c) { m_cloudSync = c; }
         std::shared_ptr<CloudSyncDestination> GetCloudSync() const { return m_cloudSync; }

diff --git a/src/catalog_json.cpp b/src/catalog_json.cpp
@@ -25,7 +25,6 @@
 
 #include "catalog_json.h"
 
-#include "qa_checks.h"
 #include "configuration.h"
 #include "str_helpers.h"
 #include "utility.h"
@@ -138,7 +137,7 @@ std::shared_ptr<JSONCatalog> JSONCatalog::Open(const wxString& filename)
 
 
 bool JSONCatalog::Save(const wxString& filename, bool /*save_mo*/,
-                        ValidationResults& /*validation_results*/,
+                        ValidationResults& validation_results,
                         CompilationStatus& /*mo_compilation_status*/)
 {
     if ( wxFileExists(filename) && !wxFile::Access(filename, wxFile::write) )
@@ -161,6 +160,8 @@ bool JSONCatalog::Save(const wxString& filename, bool /*save_mo*/,
         return false;
     }
 
+    validation_results = Validate();
+
     SetFileName(filename);
     return true;
 }
@@ -184,26 +185,6 @@ std::string JSONCatalog::SaveToBuffer()
 }
 
 
-Catalog::ValidationResults JSONCatalog::Validate(bool)
-{
-    // FIXME: move this elsewhere, remove #include "qa_checks.h", configuration.h
-
-    ValidationResults res;
-
-    for (auto& i: m_items)
-        i->ClearIssue();
-
-    res.errors = 0;
-
-#if wxUSE_GUI
-    if (Config::ShowWarnings())
-        res.warnings = QAChecker::GetFor(*this)->Check(*this);
-#endif
-
-    return res;
-}
-
-
 class GenericJSONItem : public JSONCatalogItem
 {
 public:

diff --git a/src/catalog_json.h b/src/catalog_json.h
@@ -81,8 +81,6 @@ class JSONCatalog : public Catalog
 
     std::string SaveToBuffer() override;
 
-    ValidationResults Validate(bool wasJustLoaded) override;
-
     // FIXME: derive from filename too
     Language GetLanguage() const override { return m_language; }
     void SetLanguage(Language lang) override { m_language = lang; }

diff --git a/src/catalog_po.cpp b/src/catalog_po.cpp
@@ -29,7 +29,6 @@
 #include "errors.h"
 #include "extractors/extractor.h"
 #include "gexecute.h"
-#include "qa_checks.h"
 #include "str_helpers.h"
 #include "utility.h"
 #include "version.h"
@@ -1126,7 +1125,7 @@ bool POCatalog::Save(const wxString& po_file, bool save_mo,
 
     try
     {
-        validation_results = DoValidate(po_file_temp);
+        validation_results = Validate(/*fileWithSameContent=*/po_file_temp);
     }
     catch (...)
     {
@@ -1356,7 +1355,7 @@ bool POCatalog::CompileToMO(const wxString& mo_file,
         return false;
     }
 
-    validation_results = DoValidate(po_file_temp);
+    validation_results = Validate(/*fileWithSameContent=*/po_file_temp);
 
     TempOutputFileFor mo_file_temp_obj(mo_file);
     const wxString mo_file_temp = mo_file_temp_obj.FileName();
@@ -1555,49 +1554,43 @@ bool POCatalog::FixDuplicateItems()
 }
 
 
-Catalog::ValidationResults POCatalog::Validate(bool wasJustLoaded)
+Catalog::ValidationResults POCatalog::Validate(const wxString& fileWithSameContent)
 {
+    ValidationResults res = Catalog::Validate(fileWithSameContent);
+
     if (!HasCapability(Catalog::Cap::Translations))
-        return ValidationResults();  // no errors in POT files
+        return res;  // no errors in POT files
 
-    if (wasJustLoaded)
+    if (!fileWithSameContent.empty())
     {
-        return DoValidate(GetFileName());
+        ValidateWithMsgfmt(res, fileWithSameContent);
     }
     else
     {
         TempDirectory tmpdir;
         if ( !tmpdir.IsOk() )
-            return ValidationResults();
+            return res;
 
         wxString tmp_po = tmpdir.CreateFileName("validated.po");
         if ( !DoSaveOnly(tmp_po, wxTextFileType_Unix) )
-            return ValidationResults();
+            return res;
 
-        return DoValidate(tmp_po);
+        ValidateWithMsgfmt(res, tmp_po);
     }
+
+    return res;
 }
 
-Catalog::ValidationResults POCatalog::DoValidate(const wxString& po_file)
+void POCatalog::ValidateWithMsgfmt(Catalog::ValidationResults& res, const wxString& po_file)
 {
-    ValidationResults res;
-
     GettextErrors err;
     ExecuteGettextAndParseOutput
     (
         wxString::Format("msgfmt -o /dev/null -c %s", QuoteCmdlineArg(CliSafeFileName(po_file))),
         err
     );
 
-    for (auto& i: m_items)
-        i->ClearIssue();
-
-    res.errors = (int)err.size();
-
-#if wxUSE_GUI
-    if (Config::ShowWarnings())
-        res.warnings = QAChecker::GetFor(*this)->Check(*this);
-#endif
+    res.errors += (int)err.size();
 
     for ( GettextErrors::const_iterator i = err.begin(); i != err.end(); ++i )
     {
@@ -1613,8 +1606,6 @@ Catalog::ValidationResults POCatalog::DoValidate(const wxString& po_file)
         // if not matched to an item:
         wxLogError(i->text);
     }
-
-    return res;
 }
 
 

diff --git a/src/catalog_po.h b/src/catalog_po.h
@@ -173,7 +173,7 @@ class POCatalog : public Catalog
 
     std::string SaveToBuffer() override;
 
-    ValidationResults Validate(bool wasJustLoaded) override;
+    ValidationResults Validate(const wxString& fileWithSameContent) override;
 
     /// Compiles the catalog into binary MO file.
     bool CompileToMO(const wxString& mo_file,
@@ -225,7 +225,7 @@ class POCatalog : public Catalog
     /// Fix commonly encountered fixable problems with loaded files
     void FixupCommonIssues();
 
-    ValidationResults DoValidate(const wxString& po_file);
+    void ValidateWithMsgfmt(ValidationResults& res, const wxString& po_file);
     bool DoSaveOnly(const wxString& po_file, wxTextFileType crlf);
     bool DoSaveOnly(wxTextBuffer& f, wxTextFileType crlf);
 

diff --git a/src/catalog_xliff.cpp b/src/catalog_xliff.cpp
@@ -25,7 +25,6 @@
 
 #include "catalog_xliff.h"
 
-#include "qa_checks.h"
 #include "configuration.h"
 #include "str_helpers.h"
 #include "utility.h"
@@ -499,7 +498,7 @@ std::shared_ptr<XLIFFCatalog> XLIFFCatalog::Open(const wxString& filename)
 
 
 bool XLIFFCatalog::Save(const wxString& filename, bool /*save_mo*/,
-                        ValidationResults& /*validation_results*/,
+                        ValidationResults& validation_results,
                         CompilationStatus& /*mo_compilation_status*/)
 {
     if ( wxFileExists(filename) && !wxFile::Access(filename, wxFile::write) )
@@ -519,6 +518,8 @@ bool XLIFFCatalog::Save(const wxString& filename, bool /*save_mo*/,
         return false;
     }
 
+    validation_results = Validate();
+
     SetFileName(filename);
     return true;
 }
@@ -532,25 +533,6 @@ std::string XLIFFCatalog::SaveToBuffer()
 }
 
 
-Catalog::ValidationResults XLIFFCatalog::Validate(bool)
-{
-    // FIXME: move this elsewhere, remove #include "qa_checks.h", configuration.h
-
-    ValidationResults res;
-
-    for (auto& i: m_items)
-        i->ClearIssue();
-
-    res.errors = 0;
-
-#if wxUSE_GUI
-    if (Config::ShowWarnings())
-        res.warnings = QAChecker::GetFor(*this)->Check(*this);
-#endif
-
-    return res;
-}
-
 std::string XLIFFCatalog::GetXPathValue(const char* xpath) const
 {
     auto x = m_doc.child("xliff").select_node(xpath);

diff --git a/src/catalog_xliff.h b/src/catalog_xliff.h
@@ -99,8 +99,6 @@ class XLIFFCatalog : public Catalog
 
     std::string SaveToBuffer() override;
 
-    ValidationResults Validate(bool wasJustLoaded) override;
-
     Language GetLanguage() const override { return m_language; }
     void SetLanguage(Language lang) override { m_language = lang; }
 

diff --git a/src/edframe.cpp b/src/edframe.cpp
@@ -2304,7 +2304,8 @@ void PoeditFrame::ReadCatalog(const CatalogPtr& cat)
 #endif
         {
             wxLogNull null;  // don't report non-item warnings
-            cat->Validate(/*wasJustLoaded:*/true);
+            // the file was just loaded, it is identical to in-memory content and we can pass `fileWithSameContent`
+            cat->Validate(/*fileWithSameContent=*/cat->GetFileName());
         }
 
         m_catalog = cat;