Extend MiniYaml parser with new features:

- Add support for escaping '#' inside values
- Add support for escaping leading and trailing whitespace

And when discardCommentsAndWhitespace is set to false:
- Add proper support for comments
- Persist empty lines

Whitespace and comment support requires an explicit opt-in because
they produce MiniYamlNodes with null keys.  Supporting these through
the entire game engine would require changing all yaml enumerations
to explicitly check and account for these keys with no benefit.

Comments and whitespace are now treated as real nodes during parsing,
which means that the yaml parser will throw errors if they have
incorrect indentation, even if these nodes will be discarded.

OpenRA.Game/ExternalMods.cs

@@ -128,7 +128,7 @@ namespace OpenRA
 				try
 				{
 					Directory.CreateDirectory(metadataPath);
-					File.WriteAllLines(Path.Combine(metadataPath, key + ".yaml"), yaml.ToLines(false).ToArray());
+					File.WriteAllLines(Path.Combine(metadataPath, key + ".yaml"), yaml.ToLines().ToArray());
 				}
 				catch (Exception e)
 				{

OpenRA.Game/Exts.cs

@@ -375,6 +375,10 @@ namespace OpenRA
 				var key = keySelector(item);
 				var element = elementSelector(item);
 
+				// Discard elements with null keys
+				if (!typeof(TKey).IsValueType && key == null)
+					continue;
+
 				// Check for a key conflict:
 				if (d.ContainsKey(key))
 				{

OpenRA.Game/Map/TileSet.cs

@@ -202,7 +202,8 @@ namespace OpenRA
 
 		public TileSet(IReadOnlyFileSystem fileSystem, string filepath)
 		{
-			var yaml = MiniYaml.DictFromStream(fileSystem.Open(filepath), filepath);
+			var yaml = MiniYaml.FromStream(fileSystem.Open(filepath), filepath)
+				.ToDictionary(x => x.Key, x => x.Value);
 
 			// General info
 			FieldLoader.Load(this, yaml["General"]);

OpenRA.Game/MiniYaml.cs

@@ -23,24 +23,19 @@ namespace OpenRA
 	{
 		public static void WriteToFile(this MiniYamlNodes y, string filename)
 		{
-			File.WriteAllLines(filename, y.ToLines(true).Select(x => x.TrimEnd()).ToArray());
+			File.WriteAllLines(filename, y.ToLines().Select(x => x.TrimEnd()).ToArray());
 		}
 
 		public static string WriteToString(this MiniYamlNodes y)
 		{
-			return y.ToLines(true).Select(x => x.TrimEnd()).JoinWith("\n");
+			return y.ToLines().JoinWith("\n");
 		}
 
-		public static IEnumerable<string> ToLines(this MiniYamlNodes y, bool lowest)
+		public static IEnumerable<string> ToLines(this MiniYamlNodes y)
 		{
 			foreach (var kv in y)
-			{
+				foreach (var line in kv.Value.ToLines(kv.Key, kv.Comment))
-				foreach (var line in kv.Value.ToLines(kv.Key))
 					yield return line;
-
-				if (lowest)
-					yield return "";
-			}
 		}
 	}
 
@@ -55,27 +50,32 @@ namespace OpenRA
 		public SourceLocation Location;
 		public string Key;
 		public MiniYaml Value;
+		public string Comment;
 
-		public MiniYamlNode(string k, MiniYaml v)
+		public MiniYamlNode(string k, MiniYaml v, string c = null)
 		{
 			Key = k;
 			Value = v;
+			Comment = c;
 		}
 
-		public MiniYamlNode(string k, MiniYaml v, SourceLocation loc)
+		public MiniYamlNode(string k, MiniYaml v, string c, SourceLocation loc)
-			: this(k, v)
+			: this(k, v, c)
 		{
 			Location = loc;
 		}
 
-		public MiniYamlNode(string k, string v)
+		public MiniYamlNode(string k, string v, string c = null)
-			: this(k, v, null) { }
+			: this(k, v, c, null) { }
 
 		public MiniYamlNode(string k, string v, List<MiniYamlNode> n)
-			: this(k, new MiniYaml(v, n)) { }
+			: this(k, new MiniYaml(v, n), null) { }
 
-		public MiniYamlNode(string k, string v, List<MiniYamlNode> n, SourceLocation loc)
+		public MiniYamlNode(string k, string v, string c, List<MiniYamlNode> n)
-			: this(k, new MiniYaml(v, n), loc) { }
+			: this(k, new MiniYaml(v, n), c) { }
+
+		public MiniYamlNode(string k, string v, string c, List<MiniYamlNode> n, SourceLocation loc)
+			: this(k, new MiniYaml(v, n), c, loc) { }
 
 		public override string ToString()
 		{
@@ -146,7 +146,7 @@ namespace OpenRA
 			return nd.ContainsKey(s) ? nd[s].Nodes : new List<MiniYamlNode>();
 		}
 
-		static List<MiniYamlNode> FromLines(IEnumerable<string> lines, string filename)
+		static List<MiniYamlNode> FromLines(IEnumerable<string> lines, string filename, bool discardCommentsAndWhitespace)
 		{
 			var levels = new List<List<MiniYamlNode>>();
 			levels.Add(new List<MiniYamlNode>());
@@ -157,99 +157,134 @@ namespace OpenRA
 				var line = ll;
 				++lineNo;
 
-				var commentIndex = line.IndexOf('#');
+				var keyStart = 0;
-				if (commentIndex != -1)
-					line = line.Substring(0, commentIndex).TrimEnd(' ', '\t');
-
-				if (line.Length == 0)
-					continue;
-
-				var charPosition = 0;
 				var level = 0;
 				var spaces = 0;
 				var textStart = false;
-				var currChar = line[charPosition];
-
-				while (!(currChar == '\n' || currChar == '\r') && charPosition < line.Length && !textStart)
-				{
-					currChar = line[charPosition];
-					switch (currChar)
-					{
-						case ' ':
-							spaces++;
-							if (spaces >= SpacesPerLevel)
-							{
-								spaces = 0;
-								level++;
-							}
-
-							charPosition++;
-							break;
-						case '\t':
-							level++;
-							charPosition++;
-							break;
-						default:
-							textStart = true;
-							break;
-					}
-				}
-
-				var realText = line.Substring(charPosition);
-				if (realText.Length == 0)
-					continue;
 
+				string key = null;
+				string value = null;
+				string comment = null;
 				var location = new MiniYamlNode.SourceLocation { Filename = filename, Line = lineNo };
 
-				if (levels.Count <= level)
+				if (line.Length > 0)
-					throw new YamlException("Bad indent in miniyaml at {0}".F(location));
+				{
+					var currChar = line[keyStart];
 
-				while (levels.Count > level + 1)
+					while (!(currChar == '\n' || currChar == '\r') && keyStart < line.Length && !textStart)
-					levels.RemoveAt(levels.Count - 1);
+					{
+						currChar = line[keyStart];
+						switch (currChar)
+						{
+							case ' ':
+								spaces++;
+								if (spaces >= SpacesPerLevel)
+								{
+									spaces = 0;
+									level++;
+								}
 
-				var d = new List<MiniYamlNode>();
+								keyStart++;
-				var rhs = SplitAtColon(ref realText);
+								break;
-				levels[level].Add(new MiniYamlNode(realText, rhs, d, location));
+							case '\t':
+								level++;
+								keyStart++;
+								break;
+							default:
+								textStart = true;
+								break;
+						}
+					}
 
-				levels.Add(d);
+					if (levels.Count <= level)
+						throw new YamlException("Bad indent in miniyaml at {0}".F(location));
+
+					while (levels.Count > level + 1)
+						levels.RemoveAt(levels.Count - 1);
+
+					// Extract key, value, comment from line as `<key>: <value>#<comment>`
+					// The # character is allowed in the value if escaped (\#).
+					// Leading and trailing whitespace is always trimmed from keys.
+					// Leading and trailing whitespace is trimmed from values unless they
+					// are marked with leading or trailing backslashes
+					var keyLength = line.Length - keyStart;
+					var valueStart = -1;
+					var valueLength = 0;
+					var commentStart = -1;
+					for (var i = 0; i < line.Length; i++)
+					{
+						if (valueStart < 0 && line[i] == ':')
+						{
+							valueStart = i + 1;
+							keyLength = i - keyStart;
+							valueLength = line.Length - i - 1;
+						}
+
+						if (commentStart < 0 && line[i] == '#' && (i == 0 || line[i - 1] != '\\'))
+						{
+							commentStart = i + 1;
+							if (commentStart < keyLength)
+								keyLength = i - keyStart;
+							else
+								valueLength = i - valueStart;
+
+							break;
+						}
+					}
+
+					if (keyLength > 0)
+						key = line.Substring(keyStart, keyLength).Trim();
+
+					if (valueStart >= 0)
+					{
+						var trimmed = line.Substring(valueStart, valueLength).Trim();
+						if (trimmed.Length > 0)
+							value = trimmed;
+					}
+
+					if (commentStart >= 0 && !discardCommentsAndWhitespace)
+						comment = line.Substring(commentStart);
+
+					// Remove leading/trailing whitespace guards
+					if (value != null && value.Length > 1)
+					{
+						var trimLeading = value[0] == '\\' && (value[1] == ' ' || value[1] == '\t') ? 1 : 0;
+						var trimTrailing = value[value.Length - 1] == '\\' && (value[value.Length - 2] == ' ' || value[value.Length - 2] == '\t') ? 1 : 0;
+						if (trimLeading + trimTrailing > 0)
+							value = value.Substring(trimLeading, value.Length - trimLeading - trimTrailing);
+					}
+
+					// Remove escape characters from #
+					if (value != null && value.IndexOf('#') != -1)
+						value = value.Replace("\\#", "#");
+				}
+
+				if (key != null || !discardCommentsAndWhitespace)
+				{
+					var nodes = new List<MiniYamlNode>();
+					levels[level].Add(new MiniYamlNode(key, value, comment, nodes, location));
+
+					levels.Add(nodes);
+				}
 			}
 
 			return levels[0];
 		}
 
-		static string SplitAtColon(ref string realText)
+		public static List<MiniYamlNode> FromFile(string path, bool discardCommentsAndWhitespace = true)
 		{
-			var colon = realText.IndexOf(':');
+			return FromLines(File.ReadAllLines(path), path, discardCommentsAndWhitespace);
-			if (colon == -1)
-				return null;
-
-			var ret = realText.Substring(colon + 1).Trim();
-			if (ret.Length == 0)
-				ret = null;
-
-			realText = realText.Substring(0, colon).Trim();
-			return ret;
 		}
 
-		public static Dictionary<string, MiniYaml> DictFromStream(Stream stream, string fileName = "<no filename available>")
+		public static List<MiniYamlNode> FromStream(Stream s, string fileName = "<no filename available>", bool discardCommentsAndWhitespace = true)
-		{
-			return FromStream(stream, fileName).ToDictionary(x => x.Key, x => x.Value);
-		}
-
-		public static List<MiniYamlNode> FromFile(string path)
-		{
-			return FromLines(File.ReadAllLines(path), path);
-		}
-
-		public static List<MiniYamlNode> FromStream(Stream s, string fileName = "<no filename available>")
 		{
 			using (var reader = new StreamReader(s))
-				return FromString(reader.ReadToEnd(), fileName);
+				return FromString(reader.ReadToEnd(), fileName, discardCommentsAndWhitespace);
 		}
 
-		public static List<MiniYamlNode> FromString(string text, string fileName = "<no filename available>")
+		public static List<MiniYamlNode> FromString(string text, string fileName = "<no filename available>", bool discardCommentsAndWhitespace = true)
 		{
-			return FromLines(text.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None), fileName);
+			return FromLines(text.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None), fileName, discardCommentsAndWhitespace);
 		}
 
 		public static List<MiniYamlNode> Merge(IEnumerable<List<MiniYamlNode>> sources)
@@ -260,6 +295,7 @@ namespace OpenRA
 			var tree = sources.Where(s => s != null)
 				.Select(MergeSelfPartial)
 				.Aggregate(MergePartial)
+				.Where(n => n.Key != null)
 				.ToDictionary(n => n.Key, n => n.Value);
 
 			var resolved = new Dictionary<string, MiniYaml>();
@@ -283,7 +319,7 @@ namespace OpenRA
 			var existingNode = existingNodes.FirstOrDefault(n => n.Key == overrideNode.Key);
 			if (existingNode != null)
 			{
-				existingNode.Value = MiniYaml.MergePartial(existingNode.Value, overrideNode.Value);
+				existingNode.Value = MergePartial(existingNode.Value, overrideNode.Value);
 				existingNode.Value.Nodes = ResolveInherits(existingNode.Key, existingNode.Value, tree, inherited);
 			}
 			else
@@ -395,20 +431,26 @@ namespace OpenRA
 				overrideDict.TryGetValue(key, out overrideNode);
 
 				var loc = overrideNode == null ? default(MiniYamlNode.SourceLocation) : overrideNode.Location;
+				var comment = (overrideNode ?? existingNode).Comment;
 				var merged = (existingNode == null || overrideNode == null) ? overrideNode ?? existingNode :
-					new MiniYamlNode(key, MergePartial(existingNode.Value, overrideNode.Value), loc);
+					new MiniYamlNode(key, MergePartial(existingNode.Value, overrideNode.Value), comment, loc);
 				ret.Add(merged);
 			}
 
 			return ret;
 		}
 
-		public IEnumerable<string> ToLines(string name)
+		public IEnumerable<string> ToLines(string key, string comment = null)
 		{
-			yield return name + ": " + Value;
+			var hasKey = !string.IsNullOrEmpty(key);
+			var hasValue = !string.IsNullOrEmpty(Value);
+			var hasComment = !string.IsNullOrEmpty(comment);
+			yield return (hasKey ? key + ":" : "")
+				+ (hasValue ? " " + Value.Replace("#", "\\#") : "")
+				+ (hasComment ? (hasKey || hasValue ? " " : "") + "#" + comment : "");
 
 			if (Nodes != null)
-				foreach (var line in Nodes.ToLines(false))
+				foreach (var line in Nodes.ToLines())
 					yield return "\t" + line;
 		}
 
@@ -420,11 +462,11 @@ namespace OpenRA
 				files = files.Append(mapFiles);
 			}
 
-			var yaml = files.Select(s => MiniYaml.FromStream(fileSystem.Open(s), s));
+			var yaml = files.Select(s => FromStream(fileSystem.Open(s), s));
 			if (mapRules != null && mapRules.Nodes.Any())
 				yaml = yaml.Append(mapRules.Nodes);
 
-			return MiniYaml.Merge(yaml);
+			return Merge(yaml);
 		}
 	}
 

OpenRA.Test/OpenRA.Game/MiniYamlTest.cs

@@ -194,5 +194,82 @@ Test:
 			Assert.That(mergeNode.Value, Is.EqualTo("override"), "Merge node has incorrect value.");
 			Assert.That(mergeNode.Nodes[0].Value.Value, Is.EqualTo("override"), "Merge node Child value should be 'override', but is not");
 		}
+
+		[TestCase(TestName = "Comments are correctly separated from values")]
+		public void TestEscapedHashInValues()
+		{
+			var trailingWhitespace = @"key: value # comment";
+			Assert.AreEqual("value", MiniYaml.FromString(trailingWhitespace, "trailingWhitespace")[0].Value.Value);
+
+			var noWhitespace = @"key:value# comment";
+			Assert.AreEqual("value", MiniYaml.FromString(noWhitespace, "noWhitespace")[0].Value.Value);
+
+			var escapedHashInValue = @"key: before \# after # comment";
+			Assert.AreEqual("before # after", MiniYaml.FromString(escapedHashInValue, "escapedHashInValue")[0].Value.Value);
+
+			var emptyValue = @"key:# comment";
+			Assert.AreEqual(null, MiniYaml.FromString(emptyValue, "emptyValue")[0].Value.Value);
+		}
+
+		[TestCase(TestName = "Leading and trailing whitespace can be guarded using a backslash")]
+		public void TestGuardedWhitespace()
+		{
+			var testYaml = @"key:   \      test value    \   ";
+			var nodes = MiniYaml.FromString(testYaml, "testYaml");
+			Assert.AreEqual("      test value    ", nodes[0].Value.Value);
+		}
+
+		[TestCase(TestName = "Comments should count toward line numbers")]
+		public void CommentsShouldCountTowardLineNumbers()
+		{
+			var yaml = @"
+TestA:
+	Nothing:
+
+# Comment
+TestB:
+	Nothing:
+";
+
+			var resultDiscard = MiniYaml.FromString(yaml).First(n => n.Key == "TestB");
+			Assert.That(resultDiscard.Location.Line, Is.EqualTo(6), "Node TestB should report its location as line 6, but is not (discarding comments)");
+
+			var resultKeep = MiniYaml.FromString(yaml, discardCommentsAndWhitespace: false).First(n => n.Key == "TestB");
+			Assert.That(resultDiscard.Location.Line, Is.EqualTo(6), "Node TestB should report its location as line 6, but is not (parsing comments)");
+		}
+
+		[TestCase(TestName = "Comments should survive a round trip intact")]
+		public void CommentsSurviveRoundTrip()
+		{
+			var yaml = @"
+# Top level comment node
+Parent: # comment without value
+	# Indented comment node
+	First: value containing a \# character
+	Second: value # node with inline comment
+";
+
+			var result = MiniYaml.FromString(yaml, discardCommentsAndWhitespace: false).WriteToString();
+			Assert.AreEqual(yaml, result);
+		}
+
+		[TestCase(TestName = "Comments should be be removed when discardCommentsAndWhitespace is false")]
+		public void CommentsShouldntSurviveRoundTrip()
+		{
+			var yaml = @"
+# Top level comment node
+Parent: # comment without value
+	# Indented comment node
+	First: value containing a \# character
+	Second: value # node with inline comment
+";
+
+			var strippedYaml = @"Parent:
+	First: value containing a \# character
+	Second: value";
+
+			var result = MiniYaml.FromString(yaml).WriteToString();
+			Assert.AreEqual(strippedYaml, result);
+		}
 	}
 }