From df3169033228da65f6d30a7c0d7ab053ca281bc4 Mon Sep 17 00:00:00 2001 From: Paul Chote Date: Mon, 7 May 2018 17:49:21 +0000 Subject: [PATCH] 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 | 2 +- OpenRA.Game/Exts.cs | 4 + OpenRA.Game/Map/TileSet.cs | 3 +- OpenRA.Game/MiniYaml.cs | 234 ++++++++++++++---------- OpenRA.Test/OpenRA.Game/MiniYamlTest.cs | 77 ++++++++ 5 files changed, 222 insertions(+), 98 deletions(-) diff --git a/OpenRA.Game/ExternalMods.cs b/OpenRA.Game/ExternalMods.cs index e317295560..bbcc4c7ad3 100644 --- a/OpenRA.Game/ExternalMods.cs +++ b/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) { diff --git a/OpenRA.Game/Exts.cs b/OpenRA.Game/Exts.cs index 998e73c6d4..b371f2f8a3 100644 --- a/OpenRA.Game/Exts.cs +++ b/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)) { diff --git a/OpenRA.Game/Map/TileSet.cs b/OpenRA.Game/Map/TileSet.cs index 05b692d434..0c82254392 100644 --- a/OpenRA.Game/Map/TileSet.cs +++ b/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"]); diff --git a/OpenRA.Game/MiniYaml.cs b/OpenRA.Game/MiniYaml.cs index 0b3414a662..8b71d1c6b5 100644 --- a/OpenRA.Game/MiniYaml.cs +++ b/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 ToLines(this MiniYamlNodes y, bool lowest) + public static IEnumerable ToLines(this MiniYamlNodes y) { foreach (var kv in y) - { - foreach (var line in kv.Value.ToLines(kv.Key)) + foreach (var line in kv.Value.ToLines(kv.Key, kv.Comment)) 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) - : this(k, v) + public MiniYamlNode(string k, MiniYaml v, string c, SourceLocation loc) + : this(k, v, c) { Location = loc; } - public MiniYamlNode(string k, string v) - : this(k, v, null) { } + public MiniYamlNode(string k, string v, string c = null) + : this(k, v, c, null) { } public MiniYamlNode(string k, string v, List n) - : this(k, new MiniYaml(v, n)) { } + : this(k, new MiniYaml(v, n), null) { } - public MiniYamlNode(string k, string v, List n, SourceLocation loc) - : this(k, new MiniYaml(v, n), loc) { } + public MiniYamlNode(string k, string v, string c, List n) + : this(k, new MiniYaml(v, n), c) { } + + public MiniYamlNode(string k, string v, string c, List 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(); } - static List FromLines(IEnumerable lines, string filename) + static List FromLines(IEnumerable lines, string filename, bool discardCommentsAndWhitespace) { var levels = new List>(); levels.Add(new List()); @@ -157,99 +157,134 @@ namespace OpenRA var line = ll; ++lineNo; - var commentIndex = line.IndexOf('#'); - if (commentIndex != -1) - line = line.Substring(0, commentIndex).TrimEnd(' ', '\t'); - - if (line.Length == 0) - continue; - - var charPosition = 0; + var keyStart = 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) - throw new YamlException("Bad indent in miniyaml at {0}".F(location)); + if (line.Length > 0) + { + var currChar = line[keyStart]; - while (levels.Count > level + 1) - levels.RemoveAt(levels.Count - 1); + while (!(currChar == '\n' || currChar == '\r') && keyStart < line.Length && !textStart) + { + currChar = line[keyStart]; + switch (currChar) + { + case ' ': + spaces++; + if (spaces >= SpacesPerLevel) + { + spaces = 0; + level++; + } - var d = new List(); - var rhs = SplitAtColon(ref realText); - levels[level].Add(new MiniYamlNode(realText, rhs, d, location)); + keyStart++; + break; + 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 `: #` + // 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(); + 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 FromFile(string path, bool discardCommentsAndWhitespace = true) { - var colon = realText.IndexOf(':'); - 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; + return FromLines(File.ReadAllLines(path), path, discardCommentsAndWhitespace); } - public static Dictionary DictFromStream(Stream stream, string fileName = "") - { - return FromStream(stream, fileName).ToDictionary(x => x.Key, x => x.Value); - } - - public static List FromFile(string path) - { - return FromLines(File.ReadAllLines(path), path); - } - - public static List FromStream(Stream s, string fileName = "") + public static List FromStream(Stream s, string fileName = "", bool discardCommentsAndWhitespace = true) { using (var reader = new StreamReader(s)) - return FromString(reader.ReadToEnd(), fileName); + return FromString(reader.ReadToEnd(), fileName, discardCommentsAndWhitespace); } - public static List FromString(string text, string fileName = "") + public static List FromString(string text, string fileName = "", 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 Merge(IEnumerable> 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(); @@ -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 ToLines(string name) + public IEnumerable 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); } } diff --git a/OpenRA.Test/OpenRA.Game/MiniYamlTest.cs b/OpenRA.Test/OpenRA.Game/MiniYamlTest.cs index a04588ee6c..325a36e7a2 100644 --- a/OpenRA.Test/OpenRA.Game/MiniYamlTest.cs +++ b/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); + } } }