From d4de531de5a4615a5fb37012018bbc0500ec4f89 Mon Sep 17 00:00:00 2001 From: Oliver Hamlet Date: Sat, 19 Feb 2022 13:31:39 +0000 Subject: [PATCH] Make File::GetDisplayName() a trivial accessor Don't escape any markdown characters or return the name field if the display name is empty. They're really presentation issues so are left to the UI. --- include/loot/metadata/file.h | 7 +-- src/api/helpers/text.cpp | 8 --- src/api/helpers/text.h | 2 - src/api/metadata/file.cpp | 4 -- src/api/metadata/yaml/file.h | 11 ++--- src/tests/api/internals/metadata/file_test.h | 51 ++------------------ 6 files changed, 7 insertions(+), 76 deletions(-) diff --git a/include/loot/metadata/file.h b/include/loot/metadata/file.h index 71d9b031..3ab92150 100644 --- a/include/loot/metadata/file.h +++ b/include/loot/metadata/file.h @@ -84,12 +84,7 @@ public: /** * Get the display name of the file. - * - * If the File was constructed with an empty display string, the name field - * will be returned instead, with any `ASCII punctuation characters - * `_ escaped. - * Escaping is not performed if returning the value of the display string. - * @return The file's display name or filename. + * @return The file's display name. */ LOOT_API std::string GetDisplayName() const; diff --git a/src/api/helpers/text.cpp b/src/api/helpers/text.cpp index b988e421..8ab81cd3 100644 --- a/src/api/helpers/text.cpp +++ b/src/api/helpers/text.cpp @@ -58,14 +58,6 @@ static constexpr const char* pseudosemVersionRegex = 'v' or 'version:. */ static constexpr const char* digitsVersionRegex = R"((?:^|v|version:\s*)(\d+))"; -std::string EscapeMarkdownASCIIPunctuation(const std::string& text) { - // As defined by . - static const regex asciiPunctuationCharacters( - "([!\"#$%&'()*+,\\-./:;<=>?@\\[\\\\\\]^_`{|}~])"); - - return std::regex_replace(text, asciiPunctuationCharacters, "\\$1"); -} - std::vector ExtractBashTags(const std::string& description) { std::vector tags; diff --git a/src/api/helpers/text.h b/src/api/helpers/text.h index d8c1a0b9..26703d52 100644 --- a/src/api/helpers/text.h +++ b/src/api/helpers/text.h @@ -36,8 +36,6 @@ static constexpr const char* GHOST_FILE_EXTENSION = ".ghost"; static constexpr std::size_t GHOST_FILE_EXTENSION_LENGTH = std::char_traits::length(GHOST_FILE_EXTENSION); -std::string EscapeMarkdownASCIIPunctuation(const std::string& text); - std::vector ExtractBashTags(const std::string& description); std::optional ExtractVersion(const std::string& text); diff --git a/src/api/metadata/file.cpp b/src/api/metadata/file.cpp index bde5b967..b57b633c 100644 --- a/src/api/metadata/file.cpp +++ b/src/api/metadata/file.cpp @@ -73,10 +73,6 @@ bool File::operator==(const File& rhs) const { Filename File::GetName() const { return name_; } std::string File::GetDisplayName() const { - if (display_.empty()) { - return EscapeMarkdownASCIIPunctuation(std::string(name_)); - } - return display_; } diff --git a/src/api/metadata/yaml/file.h b/src/api/metadata/yaml/file.h index d38ede2a..d0b979bb 100644 --- a/src/api/metadata/yaml/file.h +++ b/src/api/metadata/yaml/file.h @@ -45,9 +45,7 @@ struct convert { if (rhs.IsConditional()) node["condition"] = rhs.GetCondition(); - auto escapedName = - loot::EscapeMarkdownASCIIPunctuation(std::string(rhs.GetName())); - if (rhs.GetDisplayName() != escapedName) { + if (!rhs.GetDisplayName().empty()) { node["display"] = rhs.GetDisplayName(); } @@ -118,11 +116,8 @@ struct convert { }; inline Emitter& operator<<(Emitter& out, const loot::File& rhs) { - auto escapedName = - loot::EscapeMarkdownASCIIPunctuation(std::string(rhs.GetName())); - if (!rhs.IsConditional() && rhs.GetDetail().empty() && - (rhs.GetDisplayName().empty() || rhs.GetDisplayName() == escapedName)) + rhs.GetDisplayName().empty()) out << YAML::SingleQuoted << std::string(rhs.GetName()); else { out << BeginMap << Key << "name" << Value << YAML::SingleQuoted @@ -132,7 +127,7 @@ inline Emitter& operator<<(Emitter& out, const loot::File& rhs) { out << Key << "condition" << Value << YAML::SingleQuoted << rhs.GetCondition(); - if (rhs.GetDisplayName() != escapedName) + if (!rhs.GetDisplayName().empty()) out << Key << "display" << Value << YAML::SingleQuoted << rhs.GetDisplayName(); diff --git a/src/tests/api/internals/metadata/file_test.h b/src/tests/api/internals/metadata/file_test.h index aa7c24ec..914cad30 100644 --- a/src/tests/api/internals/metadata/file_test.h +++ b/src/tests/api/internals/metadata/file_test.h @@ -342,32 +342,12 @@ TEST( EXPECT_TRUE(file1 >= file2); } -TEST(File, getDisplayNameShouldReturnDisplayStringIfItIsNotEmpty) { +TEST(File, getDisplayNameShouldReturnDisplayString) { File file("name", "display"); EXPECT_EQ("display", file.GetDisplayName()); } -TEST(File, getDisplayNameShouldReturnNameStringIfDisplayStringIsEmpty) { - File file("name", ""); - - EXPECT_EQ("name", file.GetDisplayName()); -} -TEST(File, getDisplayNameShouldNotEscapeASCIIPunctuationInDisplayString) { - auto display = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"; - File file("name", display); - - EXPECT_EQ(display, file.GetDisplayName()); -} - -TEST(File, getDisplayNameShouldEscapeASCIIPunctuationInNameString) { - File file("!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~"); - - EXPECT_EQ( - R"raw(\!\"\#\$\%\&\'\(\)\*\+\,\-\.\/\:\;\<\=\>\?\@\[\\\]\^\_\`\{\|\}\~)raw", - file.GetDisplayName()); -} - TEST(File, emittingAsYamlShouldSingleQuoteValues) { File file( "name1", "display1", "condition1", {MessageContent("english", "en")}); @@ -389,16 +369,6 @@ TEST(File, emittingAsYamlShouldOutputAsAScalarIfOnlyTheNameStringIsNotEmpty) { EXPECT_EQ("'" + std::string(file.GetName()) + "'", emitter.c_str()); } -TEST( - File, - emittingAsYamlShouldOmitDisplayFieldIfItMatchesTheNameFieldAfterEscapingASCIIPunctuation) { - File file("file.esp", "file\\.esp"); - YAML::Emitter emitter; - emitter << file; - - EXPECT_STREQ("'file.esp'", emitter.c_str()); -} - TEST(File, emittingAsYamlShouldOmitAnEmptyConditionString) { File file("name1", "display1"); YAML::Emitter emitter; @@ -453,19 +423,6 @@ TEST(File, encodingAsYamlShouldOmitEmptyFields) { EXPECT_FALSE(node["detail"]); } -TEST( - File, - encodingAsYamlShouldOmitDisplayFieldIfItMatchesTheNameFieldAfterEscapingASCIIPunctuation) { - File file("file.esp", "file\\.esp"); - YAML::Node node; - node = file; - - 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\")', " @@ -528,14 +485,12 @@ TEST( EXPECT_THROW(node.as(), YAML::RepresentationException); } -TEST( - File, - decodingFromYamlScalarShouldUseNameValueForDisplayNameAndLeaveConditionEmpty) { +TEST(File, decodingFromYamlScalarShouldLeaveDisplayNameAndConditionEmpty) { YAML::Node node = YAML::Load("name1"); File file = node.as(); EXPECT_EQ(node.as(), std::string(file.GetName())); - EXPECT_EQ(node.as(), file.GetDisplayName()); + EXPECT_TRUE(file.GetDisplayName().empty()); EXPECT_TRUE(file.GetCondition().empty()); EXPECT_TRUE(file.GetDetail().empty()); }