Fix handling of CSV files with empty columns and rows (#149)

* Fix #147

* Add localization test project, fix empty rows in general

* Fill out 'normal' CSV in localization test project

* Update Changelog

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## [Unreleased]
 * Changed `rojo build` to use buffered I/O, which can make it up to 2x faster in some cases.
     * Building [*Road Not Taken*](https://github.com/LPGhatguy/roads) to an `rbxlx` file dropped from 150ms to 70ms on my machine
+* Fixed `LocalizationTable` instances being made from `csv` files not supporting empty rows or columns. ([#149](https://github.com/LPGhatguy/rojo/pull/149))
 
 ## [0.5.0 Alpha 8](https://github.com/LPGhatguy/rojo/releases/tag/v0.5.0-alpha.8) (March 29, 2019)
 * Added support for a bunch of new types when dealing with XML model/place files:

server/src/rbx_snapshot.rs

@@ -483,6 +483,7 @@ fn snapshot_csv_file<'source>(
         .deserialize()
         // TODO: Propagate error upward instead of panicking
         .map(|result| result.expect("Malformed localization table found!"))
+        .filter(|entry: &LocalizationEntryCsv| !(entry.key.is_empty() && entry.source.is_empty()))
         .map(LocalizationEntryCsv::to_json)
         .collect();
 
@@ -519,12 +520,32 @@ struct LocalizationEntryCsv {
 
 impl LocalizationEntryCsv {
     fn to_json(self) -> LocalizationEntryJson {
+        fn none_if_empty(value: String) -> Option<String> {
+            if value.is_empty() {
+                None
+            } else {
+                Some(value)
+            }
+        }
+
+        let key = none_if_empty(self.key);
+        let context = none_if_empty(self.context);
+        let example = none_if_empty(self.example);
+        let source = none_if_empty(self.source);
+
+        let mut values = HashMap::with_capacity(self.values.len());
+        for (key, value) in self.values.into_iter() {
+            if !key.is_empty() {
+                values.insert(key, value);
+            }
+        }
+
         LocalizationEntryJson {
-            key: self.key,
-            context: self.context,
-            example: self.example,
-            source: self.source,
-            values: self.values,
+            key,
+            context,
+            example,
+            source,
+            values,
         }
     }
 }
@@ -532,10 +553,18 @@ impl LocalizationEntryCsv {
 #[derive(Debug, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 struct LocalizationEntryJson {
-    key: String,
-    context: String,
-    example: String,
-    source: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    key: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    context: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    example: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    source: Option<String>,
+
     values: HashMap<String, String>,
 }
 

server/tests/snapshot_snapshots.rs

@@ -33,6 +33,7 @@ macro_rules! generate_snapshot_tests {
 
 generate_snapshot_tests!(
     empty,
+    localization,
     multi_partition_game,
     nested_partitions,
     single_partition_game,

test-projects/localization/default.project.json

@@ -0,0 +1,6 @@
+{
+  "name": "localization",
+  "tree": {
+    "$path": "src"
+  }
+}

test-projects/localization/expected-snapshot.json

@@ -0,0 +1,53 @@
+{
+  "name": "localization",
+  "class_name": "Folder",
+  "properties": {},
+  "children": [
+    {
+      "name": "empty-column-bug-147",
+      "class_name": "LocalizationTable",
+      "properties": {
+        "Contents": {
+          "Type": "String",
+          "Value": "[{\"key\":\"Language.Name\",\"source\":\"English\",\"values\":{}},{\"key\":\"Language.Region\",\"source\":\"United States\",\"values\":{}},{\"key\":\"Label.Thickness\",\"source\":\"Thickness\",\"values\":{}},{\"key\":\"Label.Opacity\",\"source\":\"Opacity\",\"values\":{}},{\"key\":\"Toolbar.Undo\",\"source\":\"Undo\",\"values\":{}},{\"key\":\"Toolbar.Redo\",\"source\":\"Redo\",\"values\":{}},{\"key\":\"Toolbar.Camera\",\"source\":\"Top-down camera\",\"values\":{}},{\"key\":\"Toolbar.Saves\",\"source\":\"Saved drawings\",\"values\":{}},{\"key\":\"Toolbar.Preferences\",\"source\":\"Settings\",\"values\":{}},{\"key\":\"Toolbar.Mode.Vector\",\"source\":\"Vector mode\",\"values\":{}},{\"key\":\"Toolbar.Mode.Pixel\",\"source\":\"Pixel mode\",\"values\":{}}]"
+        }
+      },
+      "children": [],
+      "metadata": {
+        "ignore_unknown_instances": false,
+        "source_path": "src/empty-column-bug-147.csv",
+        "project_definition": null
+      }
+    },
+    {
+      "name": "normal",
+      "class_name": "LocalizationTable",
+      "properties": {
+        "Contents": {
+          "Type": "String",
+          "Value": "[{\"key\":\"Ack\",\"example\":\"An exclamation of despair\",\"source\":\"Ack!\",\"values\":{\"es\":\"¡Ay!\"}}]"
+        }
+      },
+      "children": [],
+      "metadata": {
+        "ignore_unknown_instances": false,
+        "source_path": "src/normal.csv",
+        "project_definition": null
+      }
+    }
+  ],
+  "metadata": {
+    "ignore_unknown_instances": false,
+    "source_path": "src",
+    "project_definition": [
+      "localization",
+      {
+        "class_name": null,
+        "children": {},
+        "properties": {},
+        "ignore_unknown_instances": null,
+        "path": "src"
+      }
+    ]
+  }
+}

test-projects/localization/src/empty-column-bug-147.csv

@@ -0,0 +1,17 @@
+,Key,Source,Context,Example
+,,,,
+Metadata,Language.Name,English,,
+,Language.Region,United States,,
+,,,,
+Options,Label.Thickness,Thickness,,
+,Label.Opacity,Opacity,,
+,,,,
+Toolbar,Toolbar.Undo,Undo,,
+,Toolbar.Redo,Redo,,
+,,,,
+,Toolbar.Camera,Top-down camera,,
+,Toolbar.Saves,Saved drawings,,
+,Toolbar.Preferences,Settings,,
+,,,,
+,Toolbar.Mode.Vector,Vector mode,,
+,Toolbar.Mode.Pixel,Pixel mode,,

test-projects/localization/src/normal.csv

@@ -0,0 +1,2 @@
+Key,Source,Context,Example,es
+Ack,Ack!,,An exclamation of despair,¡Ay!