diff --git a/docs/metadata/data_structures/file.rst b/docs/metadata/data_structures/file.rst index 58dcd7f3..5bd844fc 100644 --- a/docs/metadata/data_structures/file.rst +++ b/docs/metadata/data_structures/file.rst @@ -14,6 +14,16 @@ Map Form A GitHub Flavored Markdown string, to be displayed instead of the file path in any generated messages, eg. the name of the mod the file belongs to. +.. describe:: detail + + ``string`` or ``localised content list`` + + if this file causes an error message to be displayed (e.g. because it's a + missing requirement), this detail message content will be appended to that + error message. If a string is provided, it will be interpreted as GitHub + Flavored Markdown. If a localised content list is provided, one of the + structures must be for English. If undefined, defaults to an empty string. + .. describe:: condition A condition string that is evaluated to determine whether this file data should be used: if it evaluates to true, the data is used, otherwise it is ignored. See :doc:`../conditions` for details. diff --git a/include/loot/metadata/file.h b/include/loot/metadata/file.h index 59e0c4f4..471c99d2 100644 --- a/include/loot/metadata/file.h +++ b/include/loot/metadata/file.h @@ -29,6 +29,7 @@ #include "loot/api_decorator.h" #include "loot/metadata/conditional_metadata.h" #include "loot/metadata/filename.h" +#include "loot/metadata/message_content.h" namespace loot { /** @@ -55,7 +56,8 @@ public: */ LOOT_API explicit File(const std::string& name, const std::string& display = "", - const std::string& condition = ""); + const std::string& condition = "", + const std::vector& detail = {}); /** * A less-than operator implemented with no semantics so that File objects can @@ -87,9 +89,28 @@ public: */ LOOT_API std::string GetDisplayName() const; + /** + * Get the detail message content of the file. + * + * If this file causes an error message to be displayed, the detail message + * content should be appended to that message, as it provides more detail + * about the error (e.g. suggestions for how to resolve it). + */ + LOOT_API std::vector GetDetail() const; + + /** + * Choose a detail MessageContent object given a preferred language. + * @param language + * The preferred language's code. + * @return The MessageContent object for the preferred language, or if one + * does not exist, the English-language MessageContent object. + */ + LOOT_API MessageContent ChooseDetail(const std::string& language) const; + private: Filename name_; std::string display_; + std::vector detail_; }; /** diff --git a/src/api/metadata/file.cpp b/src/api/metadata/file.cpp index 59ac9518..18006aaf 100644 --- a/src/api/metadata/file.cpp +++ b/src/api/metadata/file.cpp @@ -32,9 +32,11 @@ File::File() {} File::File(const std::string& name, const std::string& display, - const std::string& condition) : + const std::string& condition, + const std::vector& detail) : name_(Filename(name)), display_(display), + detail_(detail), ConditionalMetadata(condition) {} bool File::operator<(const File& rhs) const { @@ -54,12 +56,20 @@ bool File::operator<(const File& rhs) const { return false; } - return name_ < rhs.name_; + if (name_ < rhs.name_) { + return true; + } + + if (rhs.name_ < name_) { + return false; + } + + return detail_ < rhs.detail_; } bool File::operator==(const File& rhs) const { return display_ == rhs.display_ && GetCondition() == rhs.GetCondition() && - name_ == rhs.name_; + name_ == rhs.name_ && detail_ == rhs.detail_; } Filename File::GetName() const { return name_; } @@ -72,6 +82,12 @@ std::string File::GetDisplayName() const { return display_; } +std::vector File::GetDetail() const { return detail_; } + +MessageContent File::ChooseDetail(const std::string& language) const { + return MessageContent::Choose(detail_, language); +} + bool operator!=(const File& lhs, const File& rhs) { return !(lhs == rhs); } bool operator>(const File& lhs, const File& rhs) { return rhs < lhs; } diff --git a/src/api/metadata/yaml/file.h b/src/api/metadata/yaml/file.h index fce37cfa..b35b96d5 100644 --- a/src/api/metadata/yaml/file.h +++ b/src/api/metadata/yaml/file.h @@ -26,12 +26,13 @@ #define YAML_CPP_SUPPORT_MERGE_KEYS -#include - #include -#include "loot/metadata/file.h" +#include + #include "api/helpers/text.h" +#include "api/metadata/yaml/message_content.h" +#include "loot/metadata/file.h" namespace YAML { template<> @@ -49,6 +50,10 @@ struct convert { node["display"] = rhs.GetDisplayName(); } + if (!rhs.GetDetail().empty()) { + node["detail"] = rhs.GetDetail(); + } + return node; } @@ -65,11 +70,36 @@ struct convert { std::string name = node["name"].as(); std::string condition, display; + std::vector detail; if (node["condition"]) condition = node["condition"].as(); if (node["display"]) display = node["display"].as(); - rhs = loot::File(name, display, condition); + + if (node["detail"]) { + if (node["detail"].IsSequence()) { + detail = node["detail"].as>(); + } else { + detail.push_back( + loot::MessageContent(node["detail"].as())); + } + } + + // Check now that at least one item in info is English if there are + // multiple items. + if (detail.size() > 1) { + bool found = false; + for (const auto& mc : detail) { + if (mc.GetLanguage() == loot::MessageContent::defaultLanguage) + found = true; + } + if (!found) + throw RepresentationException(node.Mark(), + "bad conversion: multilingual messages " + "must contain an English info string"); + } + + rhs = loot::File(name, display, condition, detail); } else rhs = loot::File(node.as()); @@ -87,11 +117,11 @@ struct convert { }; inline Emitter& operator<<(Emitter& out, const loot::File& rhs) { - auto escapedName = loot::EscapeMarkdownASCIIPunctuation(std::string(rhs.GetName())); + auto escapedName = + loot::EscapeMarkdownASCIIPunctuation(std::string(rhs.GetName())); - if (!rhs.IsConditional() && - (rhs.GetDisplayName().empty() || - rhs.GetDisplayName() == escapedName)) + if (!rhs.IsConditional() && rhs.GetDetail().empty() && + (rhs.GetDisplayName().empty() || rhs.GetDisplayName() == escapedName)) out << YAML::SingleQuoted << std::string(rhs.GetName()); else { out << BeginMap << Key << "name" << Value << YAML::SingleQuoted @@ -105,6 +135,13 @@ inline Emitter& operator<<(Emitter& out, const loot::File& rhs) { out << Key << "display" << Value << YAML::SingleQuoted << rhs.GetDisplayName(); + if (rhs.GetDetail().size() == 1) { + out << Key << "detail" << Value << YAML::SingleQuoted + << rhs.GetDetail().front().GetText(); + } else if (!rhs.GetDetail().empty()) { + out << Key << "detail" << Value << rhs.GetDetail(); + } + out << EndMap; } diff --git a/src/tests/api/internals/metadata/file_test.h b/src/tests/api/internals/metadata/file_test.h index 95fa445a..836f3e9d 100644 --- a/src/tests/api/internals/metadata/file_test.h +++ b/src/tests/api/internals/metadata/file_test.h @@ -25,11 +25,10 @@ along with LOOT. If not, see #ifndef LOOT_TESTS_API_INTERNALS_METADATA_FILE_TEST #define LOOT_TESTS_API_INTERNALS_METADATA_FILE_TEST -#include "loot/metadata/file.h" - #include #include "api/metadata/yaml/file.h" +#include "loot/metadata/file.h" namespace loot { namespace test { @@ -42,11 +41,13 @@ TEST(File, defaultConstructorShouldInitialiseEmptyStrings) { } TEST(File, stringsConstructorShouldStoreGivenStrings) { - File file("name", "display", "condition"); + std::vector detail = {MessageContent("text", "en")}; + File file("name", "display", "condition", detail); EXPECT_EQ("name", std::string(file.GetName())); EXPECT_EQ("display", file.GetDisplayName()); EXPECT_EQ("condition", file.GetCondition()); + EXPECT_EQ(detail, file.GetDetail()); } TEST(File, equalityShouldBeCaseInsensitiveOnNameAndDisplay) { @@ -93,9 +94,32 @@ TEST(File, equalityShouldBeCaseSensitiveOnDisplayAndCondition) { EXPECT_FALSE(file1 == file2); } +TEST(File, equalityShouldCompareTheDetailVectors) { + File file1("", "", "", {MessageContent("text", "en")}); + File file2("", "", "", {MessageContent("text", "en")}); + + EXPECT_TRUE(file1 == file2); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("Text", "en")}); + + EXPECT_FALSE(file1 == file2); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("text", "En")}); + + EXPECT_FALSE(file1 == file2); + + file1 = File( + "", "", "", {MessageContent("text", "en"), MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("text", "en")}); + + EXPECT_FALSE(file1 == file2); +} + TEST(File, inequalityShouldBeTheInverseOfEquality) { - File file1("name", "display", "condition"); - File file2("name", "display", "condition"); + File file1("name", "display", "condition", {MessageContent("text", "en")}); + File file2("name", "display", "condition", {MessageContent("text", "en")}); EXPECT_FALSE(file1 != file2); @@ -118,6 +142,11 @@ TEST(File, inequalityShouldBeTheInverseOfEquality) { file2 = File("name", "display", "condition2"); EXPECT_TRUE(file1 != file2); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("Text", "en")}); + + EXPECT_TRUE(file1 != file2); } TEST(File, @@ -175,9 +204,32 @@ TEST( EXPECT_FALSE(file2 < file1); } +TEST(File, lessThanOperatorShouldCompareTheDetailVectors) { + File file1("", "", "", {MessageContent("text", "en")}); + File file2("", "", "", {MessageContent("text", "en")}); + + EXPECT_FALSE(file1 < file2); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("Text", "en")}); + + EXPECT_FALSE(file1 < file2); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("text", "En")}); + + EXPECT_FALSE(file1 < file2); + + file1 = File( + "", "", "", {MessageContent("text", "en"), MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("text", "en")}); + + EXPECT_FALSE(file1 < file2); +} + TEST(File, shouldAllowComparisonUsingGreaterThanOperator) { - File file1("name", "display", "condition"); - File file2("name", "display", "condition"); + File file1("name", "display", "condition", {MessageContent("text", "en")}); + File file2("name", "display", "condition", {MessageContent("text", "en")}); EXPECT_FALSE(file1 > file2); EXPECT_FALSE(file2 > file1); @@ -205,13 +257,18 @@ TEST(File, shouldAllowComparisonUsingGreaterThanOperator) { EXPECT_FALSE(file1 > file2); EXPECT_TRUE(file2 > file1); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("Text", "en")}); + + EXPECT_TRUE(file1 > file2); } TEST( File, lessThanOrEqualToOperatorShouldReturnTrueIfFirstFileIsNotGreaterThanSecondFile) { - File file1("name", "display", "condition"); - File file2("name", "display", "condition"); + File file1("name", "display", "condition", {MessageContent("text", "en")}); + File file2("name", "display", "condition", {MessageContent("text", "en")}); EXPECT_TRUE(file1 <= file2); EXPECT_TRUE(file2 <= file1); @@ -239,13 +296,18 @@ TEST( EXPECT_TRUE(file1 <= file2); EXPECT_FALSE(file2 <= file1); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("Text", "en")}); + + EXPECT_FALSE(file1 <= file2); } TEST( File, greaterThanOrEqualToOperatorShouldReturnTrueIfFirstFileIsNotLessThanSecondFile) { - File file1("name", "display", "condition"); - File file2("name", "display", "condition"); + File file1("name", "display", "condition", {MessageContent("text", "en")}); + File file2("name", "display", "condition", {MessageContent("text", "en")}); EXPECT_TRUE(file1 >= file2); EXPECT_TRUE(file2 >= file1); @@ -273,6 +335,11 @@ TEST( EXPECT_FALSE(file1 >= file2); EXPECT_TRUE(file2 >= file1); + + file1 = File("", "", "", {MessageContent("text", "en")}); + file2 = File("", "", "", {MessageContent("Text", "en")}); + + EXPECT_TRUE(file1 >= file2); } TEST(File, getDisplayNameShouldReturnDisplayStringIfItIsNotEmpty) { @@ -296,17 +363,48 @@ TEST(File, getDisplayNameShouldNotEscapeASCIIPunctuationInDisplayString) { TEST(File, getDisplayNameShouldEscapeASCIIPunctuationInNameString) { File file("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"); - EXPECT_EQ(R"raw(\!\"\#\$\%\&\'\(\)\*\+\,\-\.\/\:\;\<\=\>\?\@\[\\\]\^\_\`\{\|\}\~)raw", - file.GetDisplayName()); + EXPECT_EQ( + R"raw(\!\"\#\$\%\&\'\(\)\*\+\,\-\.\/\:\;\<\=\>\?\@\[\\\]\^\_\`\{\|\}\~)raw", + file.GetDisplayName()); +} + +TEST(File, chooseDetailShouldReturnTheGivenLanguageMessageContentIfItExists) { + std::vector content = {MessageContent("english", "en"), + MessageContent("french", "fr")}; + File file("", "", "", content); + + EXPECT_EQ(content[1], file.ChooseDetail("fr")); +} + +TEST( + File, + chooseDetailShouldReturnTheEnglishMessageContentIfTheGivenLanguageDoesNotHaveContent) { + std::vector content = {MessageContent("english", "en"), + MessageContent("french", "fr")}; + File file("", "", "", content); + + EXPECT_EQ(content[0], file.ChooseDetail("de")); +} + +TEST( + File, + chooseDetailShouldReturnEmptyMessageContentIfTheGivenLanguageAndEnglishDoNotHaveContent) { + std::vector content = {MessageContent("german", "de"), + MessageContent("french", "fr")}; + File file("", "", "", content); + + EXPECT_EQ(MessageContent(), file.ChooseDetail("es")); } TEST(File, emittingAsYamlShouldSingleQuoteValues) { - File file("name1", "display1", "condition1"); + File file( + "name1", "display1", "condition1", {MessageContent("english", "en")}); YAML::Emitter emitter; emitter << file; std::string expected = "name: '" + std::string(file.GetName()) + "'\ncondition: '" + file.GetCondition() + - "'\ndisplay: '" + file.GetDisplayName() + "'"; + "'\ndisplay: '" + file.GetDisplayName() + + "'\ndetail: '" + file.GetDetail()[0].GetText() + "'"; EXPECT_EQ(expected, emitter.c_str()); } @@ -339,14 +437,37 @@ TEST(File, emittingAsYamlShouldOmitAnEmptyConditionString) { EXPECT_EQ(expected, emitter.c_str()); } +TEST( + File, + emittingAsYamlShouldWriteDetailAsAListIfTheVectorContainsMoreThanOneElement) { + File file("", + "", + "", + {MessageContent("english", "en"), MessageContent("french", "fr")}); + YAML::Emitter emitter; + emitter << file; + std::string expected = + "name: ''\n" + "detail:\n" + " - lang: en\n" + " text: 'english'\n" + " - lang: fr\n" + " text: 'french'"; + + EXPECT_EQ(expected, emitter.c_str()); +} + TEST(File, encodingAsYamlShouldStoreDataCorrectly) { - File file("name1", "display1", "condition1"); + auto detail = {MessageContent("english", "en"), + MessageContent("french", "fr")}; + File file("name1", "display1", "condition1", detail); YAML::Node node; node = file; EXPECT_EQ(std::string(file.GetName()), node["name"].as()); EXPECT_EQ(file.GetDisplayName(), node["display"].as()); EXPECT_EQ(file.GetCondition(), node["condition"].as()); + EXPECT_EQ(file.GetDetail(), node["detail"].as>()); } TEST(File, encodingAsYamlShouldOmitEmptyFields) { @@ -357,6 +478,7 @@ TEST(File, encodingAsYamlShouldOmitEmptyFields) { EXPECT_EQ(std::string(file.GetName()), node["name"].as()); EXPECT_FALSE(node["display"]); EXPECT_FALSE(node["condition"]); + EXPECT_FALSE(node["detail"]); } TEST( @@ -369,16 +491,22 @@ TEST( EXPECT_EQ(std::string(file.GetName()), node["name"].as()); EXPECT_FALSE(node["display"]); EXPECT_FALSE(node["condition"]); + EXPECT_FALSE(node["detail"]); } TEST(File, decodingFromYamlShouldSetDataCorrectly) { YAML::Node node = YAML::Load( - "{name: name1, display: display1, condition: 'file(\"Foo.esp\")'}"); + "{name: name1, display: display1, condition: 'file(\"Foo.esp\")', " + "detail: 'details'}"); File file = node.as(); + std::vector expectedDetail = { + MessageContent("details", "en")}; + EXPECT_EQ(node["name"].as(), std::string(file.GetName())); EXPECT_EQ(node["display"].as(), file.GetDisplayName()); EXPECT_EQ(node["condition"].as(), file.GetCondition()); + EXPECT_EQ(expectedDetail, file.GetDetail()); } TEST(File, @@ -389,6 +517,44 @@ TEST(File, EXPECT_EQ(node["name"].as(), std::string(file.GetName())); EXPECT_EQ(node["display"].as(), file.GetDisplayName()); EXPECT_TRUE(file.GetCondition().empty()); + EXPECT_TRUE(file.GetDetail().empty()); +} + +TEST(File, decodingFromYamlWithAListOfMessageContentDetailsShouldReadThemAll) { + YAML::Node node = YAML::Load( + "{name: name1, display: display1, condition: 'file(\"Foo.esp\")', " + "detail: [{text: english, lang: en}, {text: french, lang: fr}]}"); + File file = node.as(); + + std::vector expectedDetail = {MessageContent("english", "en"), + MessageContent("french", "fr")}; + + EXPECT_EQ(expectedDetail, file.GetDetail()); +} + +TEST(File, + decodingFromYamlShouldNotThrowIfTheOnlyDetailStringIsNotEnglish) { + YAML::Node node = YAML::Load( + "name: name1\n" + "detail:\n" + " - lang: fr\n" + " text: content1"); + + EXPECT_NO_THROW(node.as()); +} + +TEST( + File, + decodingFromYamlShouldThrowIfMultipleContentStringsAreGivenAndNoneAreEnglish) { + YAML::Node node = YAML::Load( + "name: name1\n" + "detail:\n" + " - lang: de\n" + " text: content1\n" + " - lang: fr\n" + " text: content2"); + + EXPECT_THROW(node.as(), YAML::RepresentationException); } TEST( @@ -400,6 +566,7 @@ TEST( EXPECT_EQ(node.as(), std::string(file.GetName())); EXPECT_EQ(node.as(), file.GetDisplayName()); EXPECT_TRUE(file.GetCondition().empty()); + EXPECT_TRUE(file.GetDetail().empty()); } TEST(File, decodingFromYamlShouldThrowIfAnInvalidMapIsGiven) {