YAML Order Preserving Map Parsing (#21)

* add ability to parse yaml maps in the order they are specified in the file

* use template syntax

config_utilities/include/config_utilities/internal/visitor.h

@@ -47,6 +47,11 @@
 #include "config_utilities/internal/yaml_parser.h"
 
 namespace config {
+
+//! Aliased vector type to represent "insertion" ordered map (instead of key ordered)
+template <typename Key, typename ConfigT>
+using OrderedMap = std::vector<std::pair<Key, ConfigT>>;
+
 namespace internal {
 
 /**
@@ -99,10 +104,16 @@ struct Visitor {
   template <typename ConfigT, typename std::enable_if<isConfig<ConfigT>(), bool>::type = true>
   static void visitField(std::vector<ConfigT>& config, const std::string& field_name, const std::string& /* unit */);
 
-  // Map (ordered) of config types.
+  // Map (ordered by key) of config types.
   template <typename Key, typename ConfigT, typename std::enable_if<isConfig<ConfigT>(), bool>::type = true>
   static void visitField(std::map<Key, ConfigT>& config, const std::string& field_name, const std::string& /* unit */);
 
+  // Map (ordered by yaml order) of config types.
+  template <typename Key, typename ConfigT, typename std::enable_if<isConfig<ConfigT>(), bool>::type = true>
+  static void visitField(OrderedMap<Key, ConfigT>& config,
+                         const std::string& field_name,
+                         const std::string& /* unit */);
+
   // Execute a check.
   static void visitCheck(const CheckBase& check);
 
@@ -179,6 +190,11 @@ void declare_config(std::map<K, T>& map_config) {
   Visitor::visitField(map_config, "", "");
 }
 
+template <typename K, typename T, typename std::enable_if<isConfig<T>(), bool>::type = true>
+void declare_config(OrderedMap<K, T>& map_config) {
+  Visitor::visitField(map_config, "", "");
+}
+
 // Public interfaces to declare properties in declare_config.
 
 // argument-dependent-lookup (ADL) so definitions of 'declare_config()' can be found anywhere. Note that

config_utilities/include/config_utilities/internal/visitor_impl.hpp

@@ -269,7 +269,33 @@ void Visitor::visitField(std::vector<ConfigT>& config, const std::string& field_
 
 // Visit a map of subconfigs.
 template <typename K, typename ConfigT, typename std::enable_if<isConfig<ConfigT>(), bool>::type>
-void Visitor::visitField(std::map<K, ConfigT>& config, const std::string& field_name, const std::string& /* unit */) {
+void Visitor::visitField(std::map<K, ConfigT>& config, const std::string& field_name, const std::string& unit) {
+  Visitor& visitor = Visitor::instance();
+  if (visitor.mode == Visitor::Mode::kGetDefaults) {
+    return;
+  }
+
+  OrderedMap<K, ConfigT> intermediate;
+  // copy current config state if doing something else other than settings the config
+  if (visitor.mode != Visitor::Mode::kSet) {
+    intermediate.insert(intermediate.begin(), config.begin(), config.end());
+  }
+
+  // make use of more general named parsing
+  visitField<K, ConfigT>(intermediate, field_name, unit);
+
+  // assign parsed configs to actual map if we're setting the config
+  if (visitor.mode == Visitor::Mode::kSet) {
+    config.clear();
+    // note that we don't use insert here to guarantee that duplicates behave as expected (overriding with the last)
+    for (const auto& [key, value] : intermediate) {
+      config[key] = value;
+    }
+  }
+}
+
+template <typename K, typename ConfigT, typename std::enable_if<isConfig<ConfigT>(), bool>::type>
+void Visitor::visitField(OrderedMap<K, ConfigT>& config, const std::string& field_name, const std::string& /* unit */) {
   Visitor& visitor = Visitor::instance();
   if (visitor.mode == Visitor::Mode::kGetDefaults) {
     return;
@@ -281,10 +307,10 @@ void Visitor::visitField(std::map<K, ConfigT>& config, const std::string& field_
     const auto map_ns = visitor.name_space.empty() ? field_name : visitor.name_space + "/" + field_name;
     const auto nodes = getNodeMap(lookupNamespace(visitor.data.data, map_ns));
     for (auto&& [key, node] : nodes) {
-      auto iter = config.emplace(YAML::Node(key).template as<K>(), ConfigT()).first;
-      auto& sub_config = iter->second;
-      visitor.data.sub_configs.emplace_back(setValues(sub_config, node, false, "", field_name, false));
-      visitor.data.sub_configs.back().map_config_key = key;
+      auto& entry = config.emplace_back();
+      entry.first = key.template as<K>();
+      visitor.data.sub_configs.emplace_back(setValues(entry.second, node, false, "", field_name, false));
+      visitor.data.sub_configs.back().map_config_key = key.template as<std::string>();
     }
   }
 

config_utilities/include/config_utilities/internal/yaml_utils.h

@@ -78,7 +78,6 @@ std::vector<YAML::Node> getNodeArray(const YAML::Node& node);
  * @param node The node to convert.
  * @return The list of nodes. Nodes stored in this struct are references to the original data.
  */
-std::map<std::string, YAML::Node> getNodeMap(const YAML::Node& node);
-
+std::vector<std::pair<YAML::Node, YAML::Node>> getNodeMap(const YAML::Node& node);
 
 }  // namespace config::internal

config_utilities/src/yaml_utils.cpp

@@ -147,19 +147,22 @@ std::vector<YAML::Node> getNodeArray(const YAML::Node& node) {
   return result;
 }
 
-std::map<std::string, YAML::Node> getNodeMap(const YAML::Node& node) {
-  std::map<std::string, YAML::Node> result;
+std::vector<std::pair<YAML::Node, YAML::Node>> getNodeMap(const YAML::Node& node) {
+  std::vector<std::pair<YAML::Node, YAML::Node>> result;
   if (node.IsMap()) {
     for (const auto& kv_pair : node) {
-      result.emplace(kv_pair.first.as<std::string>(), kv_pair.second);
+      result.emplace_back(kv_pair);
     }
   } else if (node.IsSequence()) {
     size_t index = 0;
     for (const auto& sub_node : node) {
-      result.emplace(std::to_string(index), sub_node);
+      auto& new_pair = result.emplace_back();
+      new_pair.first = index;
+      new_pair.second = YAML::Clone(sub_node);
       ++index;
     }
   }
+
   return result;
 }
 

config_utilities/test/tests/config_maps.cpp

@@ -71,7 +71,7 @@ void declare_config(MapConfig& config) {
   check(config.f, GE, 0, "f");
 }
 
-void PrintTo(const MapConfig& conf, std::ostream* os) { *os << toString(conf); }
+void PrintTo(const MapConfig& conf, std::ostream* os) { *os << "\n" << toString(conf); }
 
 struct ConfigWithMaps {
   int i = 0;
@@ -118,6 +118,25 @@ TEST(ConfigMaps, FromYamlMap) {
   EXPECT_EQ(configs, expected);
 }
 
+TEST(ConfigMaps, FromYamlMapOrdered) {
+  const std::string yaml_map = R"(
+z:
+  s: "a"
+  f: 1
+x:
+  s: "b"
+  f: 2
+y:
+  s: "c"
+  f: 3
+)";
+  const auto node = YAML::Load(yaml_map);
+
+  const auto configs = fromYaml<OrderedMap<std::string, MapConfig>>(node);
+  OrderedMap<std::string, MapConfig> expected{{"z", {"a", 1}}, {"x", {"b", 2}}, {"y", {"c", 3}}};
+  EXPECT_EQ(configs, expected);
+}
+
 TEST(ConfigMaps, FromYamlSeq) {
   const std::string yaml_seq = R"(
 - s: "a"