From 553f4f8dcea90eb6241f7906479f5e7d24f02abd Mon Sep 17 00:00:00 2001 From: andybarcia Date: Fri, 19 Mar 2021 19:50:53 +0100 Subject: [PATCH] Improve error reporting when parsing CSV translation file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #46682. Also fix unit test suite to separate generic FileAccess CSV testing from using CSV as translation. And add more CSV translation tests. Co-authored-by: Rémi Verschelde --- core/io/file_access.cpp | 5 +++ .../resource_importer_csv_translation.cpp | 11 +++---- tests/core/io/test_file_access.h | 10 +++--- tests/core/string/test_translation.h | 32 ++++++++++++++++--- tests/data/testdata.csv | 8 +++++ tests/data/translations.csv | 11 ++----- 6 files changed, 54 insertions(+), 23 deletions(-) create mode 100644 tests/data/testdata.csv diff --git a/core/io/file_access.cpp b/core/io/file_access.cpp index a6a1a224b30..b669afdc991 100644 --- a/core/io/file_access.cpp +++ b/core/io/file_access.cpp @@ -441,6 +441,11 @@ Vector FileAccess::get_csv_line(const String &p_delim) const { current += c; } } + + if (in_quote) { + WARN_PRINT(vformat("Reached end of file before closing '\"' in CSV file '%s'.", get_path())); + } + strings.push_back(current); return strings; diff --git a/editor/import/resource_importer_csv_translation.cpp b/editor/import/resource_importer_csv_translation.cpp index 5971f3b07f7..92d244c79a4 100644 --- a/editor/import/resource_importer_csv_translation.cpp +++ b/editor/import/resource_importer_csv_translation.cpp @@ -107,18 +107,17 @@ Error ResourceImporterCSVTranslation::import(const String &p_source_file, const translations.push_back(translation); } - line = f->get_csv_line(delimiter); - - while (line.size() == locales.size() + 1) { + do { + line = f->get_csv_line(delimiter); String key = line[0]; if (!key.is_empty()) { + ERR_FAIL_COND_V_MSG(line.size() != locales.size() + 1, ERR_PARSE_ERROR, vformat("Error importing CSV translation: expected %d locale(s), but the '%s' key has %d locale(s).", locales.size(), key, line.size() - 1)); + for (int i = 1; i < line.size(); i++) { translations.write[i - 1]->add_message(key, line[i].c_unescape()); } } - - line = f->get_csv_line(delimiter); - } + } while (!f->eof_reached()); for (int i = 0; i < translations.size(); i++) { Ref xlt = translations[i]; diff --git a/tests/core/io/test_file_access.h b/tests/core/io/test_file_access.h index 243b75626fe..a4d3fd1d704 100644 --- a/tests/core/io/test_file_access.h +++ b/tests/core/io/test_file_access.h @@ -38,25 +38,27 @@ namespace TestFileAccess { TEST_CASE("[FileAccess] CSV read") { - Ref f = FileAccess::open(TestUtils::get_data_path("translations.csv"), FileAccess::READ); + Ref f = FileAccess::open(TestUtils::get_data_path("testdata.csv"), FileAccess::READ); REQUIRE(!f.is_null()); Vector header = f->get_csv_line(); // Default delimiter: ",". - REQUIRE(header.size() == 3); + REQUIRE(header.size() == 4); Vector row1 = f->get_csv_line(","); // Explicit delimiter, should be the same. - REQUIRE(row1.size() == 3); + REQUIRE(row1.size() == 4); CHECK(row1[0] == "GOOD_MORNING"); CHECK(row1[1] == "Good Morning"); CHECK(row1[2] == "Guten Morgen"); + CHECK(row1[3] == "Bonjour"); Vector row2 = f->get_csv_line(); - REQUIRE(row2.size() == 3); + REQUIRE(row2.size() == 4); CHECK(row2[0] == "GOOD_EVENING"); CHECK(row2[1] == "Good Evening"); CHECK(row2[2].is_empty()); // Use case: not yet translated! // https://github.com/godotengine/godot/issues/44269 CHECK_MESSAGE(row2[2] != "\"", "Should not parse empty string as a single double quote."); + CHECK(row2[3] == "\"\""); // Intentionally testing only escaped double quotes. Vector row3 = f->get_csv_line(); REQUIRE(row3.size() == 6); diff --git a/tests/core/string/test_translation.h b/tests/core/string/test_translation.h index 3519f3050b9..bf9674d6b17 100644 --- a/tests/core/string/test_translation.h +++ b/tests/core/string/test_translation.h @@ -151,7 +151,7 @@ TEST_CASE("[OptimizedTranslation] Generate from Translation and read messages") } #ifdef TOOLS_ENABLED -TEST_CASE("[Translation] CSV import") { +TEST_CASE("[TranslationCSV] CSV import") { Ref import_csv_translation = memnew(ResourceImporterCSVTranslation); HashMap options; @@ -163,17 +163,39 @@ TEST_CASE("[Translation] CSV import") { Error result = import_csv_translation->import(TestUtils::get_data_path("translations.csv"), "", options, nullptr, &gen_files); CHECK(result == OK); - CHECK(gen_files.size() == 2); + CHECK(gen_files.size() == 4); + + TranslationServer *ts = TranslationServer::get_singleton(); for (const String &file : gen_files) { Ref translation = ResourceLoader::load(file); CHECK(translation.is_valid()); - TranslationServer::get_singleton()->add_translation(translation); + ts->add_translation(translation); } - TranslationServer::get_singleton()->set_locale("de"); + ts->set_locale("en"); - CHECK(Object().tr("GOOD_MORNING", "") == "Guten Morgen"); + // `tr` can be called on any Object, we reuse TranslationServer for convenience. + CHECK(ts->tr("GOOD_MORNING") == "Good Morning"); + CHECK(ts->tr("GOOD_EVENING") == "Good Evening"); + + ts->set_locale("de"); + + CHECK(ts->tr("GOOD_MORNING") == "Guten Morgen"); + CHECK(ts->tr("GOOD_EVENING") == "Good Evening"); // Left blank in CSV, should source from 'en'. + + ts->set_locale("ja"); + + CHECK(ts->tr("GOOD_MORNING") == String::utf8("おはよう")); + CHECK(ts->tr("GOOD_EVENING") == String::utf8("こんばんは")); + + /* FIXME: This passes, but triggers a chain reaction that makes test_viewport + * and test_text_edit explode in a billion glittery Unicode particles. + ts->set_locale("fa"); + + CHECK(ts->tr("GOOD_MORNING") == String::utf8("صبح بخیر")); + CHECK(ts->tr("GOOD_EVENING") == String::utf8("عصر بخیر")); + */ } #endif // TOOLS_ENABLED diff --git a/tests/data/testdata.csv b/tests/data/testdata.csv new file mode 100644 index 00000000000..8f5cc914acd --- /dev/null +++ b/tests/data/testdata.csv @@ -0,0 +1,8 @@ +Header 1,Header 2,Header 3,Header 4 +GOOD_MORNING,"Good Morning","Guten Morgen","Bonjour" +GOOD_EVENING,"Good Evening","","""""" +Without quotes,"With, comma","With ""inner"" quotes","With ""inner"", quotes"","" and comma","With ""inner +split"" quotes and +line breaks","With \nnewline chars" +Some other~delimiter~should still work, shouldn't it? +What about tab separated lines, good? diff --git a/tests/data/translations.csv b/tests/data/translations.csv index 8cb7b800c5c..6b5efc9b91c 100644 --- a/tests/data/translations.csv +++ b/tests/data/translations.csv @@ -1,8 +1,3 @@ -keys,en,de -GOOD_MORNING,"Good Morning","Guten Morgen" -GOOD_EVENING,"Good Evening","" -Without quotes,"With, comma","With ""inner"" quotes","With ""inner"", quotes"","" and comma","With ""inner -split"" quotes and -line breaks","With \nnewline chars" -Some other~delimiter~should still work, shouldn't it? -What about tab separated lines, good? +keys,en,de,ja,fa +GOOD_MORNING,"Good Morning","Guten Morgen","おはよう","صبح بخیر" +GOOD_EVENING,"Good Evening","","こんばんは","عصر بخیر"