Add a description field for groups

This breaks ABI compatibility, but not API compatibility.

docs/metadata/data_structures/group.rst

@@ -12,6 +12,13 @@ This structure can be used to hold group definitions. It is a key-value map.
 
   **Required.** A case-sensitive name that identifies the group.
 
+.. describe:: description
+
+  ``string``
+
+  A description of the group, e.g. what sort of plugins it contains. If
+  undefined, the description is an empty string.
+
 .. describe:: after
 
   ``string set``
@@ -30,7 +37,8 @@ Merging Groups
 --------------
 
 When a group definition for an already-defined group is encountered, the
-``after`` sets of the two definitions are merged.
+``description`` field is replaced if the new value is not an empty string, and
+the ``after`` sets of the two definitions are merged.
 
 The ``default`` Group
 ---------------------
@@ -55,6 +63,7 @@ Examples
   # Create a group for map marker plugins that loads after the predefined
   # 'default' group.
   name: 'Map Markers'
+  description: 'A group for map marker plugins that need to load late.'
   after:
     - 'default'
 

include/loot/metadata/group.h

@@ -43,24 +43,19 @@ public:
   LOOT_API Group();
 
   /**
-   * Construct a File with the given name and an empty set of groups to load
-   * after.
-   * @param  name
-   *         The group name.
-   * @return A Group object.
-   */
-  LOOT_API Group(const std::string& name);
-
-  /**
-   * Construct a File with the given name and set of groups to load after.
+   * Construct a Group with the given name, description and set of groups to
+   * load after.
    * @param  name
    *         The group name.
    * @param  afterGroups
    *         The names of groups this group loads after.
+   * @param  description
+   *         A description of the group.
    * @return A Group object.
    */
   LOOT_API Group(const std::string& name,
-                 const std::unordered_set<std::string>& afterGroups);
+                 const std::unordered_set<std::string>& afterGroups = {},
+                 const std::string& description = "");
 
   /**
    * Check if two Group objects are equal by comparing their names.
@@ -74,6 +69,12 @@ public:
    */
   LOOT_API std::string GetName() const;
 
+  /**
+   * Get the description of the group.
+   * @return The group's description.
+   */
+  LOOT_API std::string GetDescription() const;
+
   /**
    * Get the set of groups this group loads after.
    * @return A set of group names.
@@ -82,6 +83,7 @@ public:
 
 private:
   std::string name_;
+  std::string description_;
   std::unordered_set<std::string> afterGroups_;
 };
 }

src/api/metadata/group.cpp

@@ -29,12 +29,12 @@
 namespace loot {
 Group::Group() : name_("default") {}
 
-Group::Group(const std::string& name) : name_(name) {}
-
 Group::Group(const std::string& name,
-           const std::unordered_set<std::string>& afterGroups) :
+             const std::unordered_set<std::string>& afterGroups,
+             const std::string& description) :
     name_(name),
-    afterGroups_(afterGroups) {}
+    afterGroups_(afterGroups),
+    description_(description) {}
 
 bool Group::operator==(const Group& rhs) const {
   return name_ == rhs.name_;
@@ -42,6 +42,8 @@ bool Group::operator==(const Group& rhs) const {
 
 std::string Group::GetName() const { return name_; }
 
+std::string Group::GetDescription() const { return description_; }
+
 std::unordered_set<std::string> Group::GetAfterGroups() const {
   return afterGroups_;
 }

src/api/metadata/yaml/group.h

@@ -41,6 +41,10 @@ struct convert<loot::Group> {
     Node node;
     node["name"] = rhs.GetName();
 
+    if (!rhs.GetDescription().empty()) {
+      node["description"] = rhs.GetDescription();
+    }
+
     auto afterGroups = rhs.GetAfterGroups();
     if (!afterGroups.empty())
       node["after"] = afterGroups;
@@ -54,29 +58,40 @@ struct convert<loot::Group> {
           node.Mark(), "bad conversion: 'group' object must be a map");
 
     if (!node["name"])
-    throw RepresentationException(
-        node.Mark(),
-        "bad conversion: 'name' key missing from 'file' map object");
+      throw RepresentationException(
+          node.Mark(),
+          "bad conversion: 'name' key missing from 'file' map object");
 
     std::string name = node["name"].as<std::string>();
+    std::string description;
+    std::unordered_set<std::string> afterGroups;
+
+    if (node["description"]) {
+      description = node["description"].as<std::string>();
+    }
 
     if (node["after"]) {
-        rhs = loot::Group(name, node["after"].as<std::unordered_set<std::string>>());
-    } else {
-        rhs = loot::Group(name);
+      afterGroups = node["after"].as<std::unordered_set<std::string>>();
     }
 
+    rhs = loot::Group(name, afterGroups, description);
+
     return true;
   }
 };
 
 inline Emitter& operator<<(Emitter& out, const loot::Group& rhs) {
-  out << BeginMap
-    << Key << "name" << Value << YAML::SingleQuoted << rhs.GetName();
+  out << BeginMap << Key << "name" << Value << YAML::SingleQuoted
+      << rhs.GetName();
+
+  if (!rhs.GetDescription().empty()) {
+    out << Key << "description" << Value << YAML::SingleQuoted
+        << rhs.GetDescription();
+  }
 
   auto afterGroups = rhs.GetAfterGroups();
   if (!afterGroups.empty()) {
-      out << Key << "after" << Value << afterGroups;
+    out << Key << "after" << Value << afterGroups;
   }
 
   out << EndMap;

src/tests/api/internals/metadata/group_test.h

@@ -40,18 +40,23 @@ TEST(Group, defaultConstructorShouldCreateDefaultGroup) {
   EXPECT_TRUE(group.GetAfterGroups().empty());
 }
 
-TEST(Group, nameConstructorShouldCreateNamedGroup) {
+TEST(Group,
+     allArgsConstructorShouldSetDescriptionAndAfterGroupsDefaultsAsEmpty) {
   Group group("group1");
 
   EXPECT_EQ("group1", group.GetName());
+  EXPECT_TRUE(group.GetDescription().empty());
   EXPECT_TRUE(group.GetAfterGroups().empty());
 }
 
 TEST(Group, allArgsConstructorShouldStoreGivenValues) {
-  Group group("group1", std::unordered_set<std::string>({"other_group"}));
+  Group group(
+      "group1", std::unordered_set<std::string>({"other_group"}), "test");
 
   EXPECT_EQ("group1", group.GetName());
-  EXPECT_EQ(std::unordered_set<std::string>({"other_group"}), group.GetAfterGroups());
+  EXPECT_EQ("test", group.GetDescription());
+  EXPECT_EQ(std::unordered_set<std::string>({"other_group"}),
+            group.GetAfterGroups());
 }
 
 TEST(Group, groupsWithCaseInsensitiveEqualNameStringsShouldNotBeEqual) {
@@ -84,17 +89,47 @@ TEST(Group, emittingAsYamlShouldOmitAfterKeyIfAfterGroupsIsEmpty) {
   EXPECT_STREQ("name: 'default'", emitter.c_str());
 }
 
-TEST(Group, emittingAsYamlShouldIncludeAfterKeyIfAfterGroupsIsNotEmpty) {
-  Group group("group1", std::unordered_set<std::string>({ "other_group" }));
+TEST(Group, emittingAsYamlShouldIncludeDescriptionKeyIfDescriptionIsNotEmpty) {
+  Group group("group1", {}, "test");
 
   YAML::Emitter emitter;
   emitter << group;
 
   EXPECT_STREQ(
-    "name: 'group1'\n"
-    "after:\n"
-    "  - other_group", 
-    emitter.c_str());
+      "name: 'group1'\n"
+      "description: 'test'",
+      emitter.c_str());
+}
+
+TEST(Group, emittingAsYamlShouldIncludeAfterKeyIfAfterGroupsIsNotEmpty) {
+  Group group("group1", std::unordered_set<std::string>({"other_group"}));
+
+  YAML::Emitter emitter;
+  emitter << group;
+
+  EXPECT_STREQ(
+      "name: 'group1'\n"
+      "after:\n"
+      "  - other_group",
+      emitter.c_str());
+}
+
+TEST(Group, encodingAsYamlShouldOmitDescriptionKeyIfDescriptionIsEmpty) {
+  Group group;
+  YAML::Node node;
+  node = group;
+
+  EXPECT_EQ("default", node["name"].as<std::string>());
+  EXPECT_FALSE(node["description"]);
+}
+
+TEST(Group, encodingAsYamlShouldIncludeDescriptionKeyIfDescriptionIsNotEmpty) {
+  Group group("group1", {}, "test");
+  YAML::Node node;
+  node = group;
+
+  EXPECT_EQ("group1", node["name"].as<std::string>());
+  EXPECT_EQ("test", node["description"].as<std::string>());
 }
 
 TEST(Group, encodingAsYamlShouldOmitAfterKeyIfAfterGroupsIsEmpty) {
@@ -107,13 +142,14 @@ TEST(Group, encodingAsYamlShouldOmitAfterKeyIfAfterGroupsIsEmpty) {
 }
 
 TEST(Group, encodingAsYamlShouldIncludeAfterKeyIfAfterGroupsIsNotEmpty) {
-  Group group("group1", std::unordered_set<std::string>({ "other_group" }));
+  Group group("group1", std::unordered_set<std::string>({"other_group"}));
   YAML::Node node;
   node = group;
 
-  std::unordered_set<std::string> expectedAfterGroups = { "other_group" };
+  std::unordered_set<std::string> expectedAfterGroups = {"other_group"};
   EXPECT_EQ("group1", node["name"].as<std::string>());
-  EXPECT_EQ(expectedAfterGroups, node["after"].as<std::unordered_set<std::string>>());
+  EXPECT_EQ(expectedAfterGroups,
+            node["after"].as<std::unordered_set<std::string>>());
 }
 
 TEST(Group, decodingFromYamlShouldSetGivenName) {
@@ -124,11 +160,19 @@ TEST(Group, decodingFromYamlShouldSetGivenName) {
   EXPECT_TRUE(group.GetAfterGroups().empty());
 }
 
+TEST(Group, decodingFromYamlShouldSetDescriptionIfOneIsGiven) {
+  YAML::Node node = YAML::Load("{name: group1, description: test}");
+  Group group = node.as<Group>();
+
+  EXPECT_EQ("group1", group.GetName());
+  EXPECT_EQ("test", group.GetDescription());
+}
+
 TEST(Group, decodingFromYamlShouldSetAfterGroupsIfAnyAreGiven) {
   YAML::Node node = YAML::Load("{name: group1, after: [ other_group ]}");
   Group group = node.as<Group>();
 
-  std::unordered_set<std::string> expectedAfterGroups = { "other_group" };
+  std::unordered_set<std::string> expectedAfterGroups = {"other_group"};
   EXPECT_EQ("group1", group.GetName());
   EXPECT_EQ(expectedAfterGroups, group.GetAfterGroups());
 }

src/tests/api/internals/sorting/group_sort_test.h

@@ -38,7 +38,7 @@ TEST(GetTransitiveAfterGroups, shouldMapGroupsToTheirTransitiveAfterGroups) {
   std::unordered_set<Group> groups({
     Group("a"),
     Group("b", std::unordered_set<std::string>({ "a" })),
-    Group("c", std::unordered_set<std::string>({ "b" }))
+    Group("c", std::unordered_set<std::string>({"b"}))
   });
 
   auto mapped = GetTransitiveAfterGroups(groups);