Rework --generator Command Line Format

slicec/src/slice_options.rs

@@ -23,7 +23,7 @@ pub struct SliceOptions {
     ///   Ex: '--generator /path/to/my/generator'
     ///
     /// Each code-generator can be provided with arbitrary string arguments using the following syntax:
-    ///   '/path/to/my/generator;arg1=value1;arg2 = value2;arg3;...'
+    ///   '/path/to/my/generator,arg1=value1,arg2 = value2,arg3,...'
     /// Leading and trailing whitespace is stripped from arguments and their values. Argument values are optional.
     #[arg(short = 'G', long = "generator", num_args = 1, action = Append, value_name = "GENERATOR", value_parser = plugin_parser, verbatim_doc_comment)]
     pub generators: Vec<Plugin>,
@@ -62,7 +62,6 @@ This AST is encoded with Slice, and then output, to be consumed by other tools."
 
 fn plugin_parser<'a>(s: &str) -> Result<Plugin, &'a str> {
     // Helper enum to track what element the parser is currently parsing.
-    #[derive(PartialEq, Eq)]
     enum State {
         Path,
         Key,
@@ -82,22 +81,14 @@ fn plugin_parser<'a>(s: &str) -> Result<Plugin, &'a str> {
     let mut char_iter = s.chars().peekable();
     while let Some(c) = char_iter.next() {
         match c {
-            // The next character after this is being escaped, add it directly to the buffer without parsing.
-            '\\' => match char_iter.next() {
-                Some(escaped_char) => string_buffer.push(escaped_char),
-                None => return Err("unterminated escape sequence (for a literal '\\' character, use '\\\\')"),
-            },
-
-            ';' => {
-                if string_buffer.is_empty() {
-                    match state {
-                        State::Path => return Err("missing plugin path (ex: 'PATH;KEY=VALUE')"),
-                        State::Key => return Err("missing argument key (ex: 'PATH;KEY=VALUE')"),
-                        State::Value => return Err("missing argument value (ex: 'PATH;KEY=VALUE' or 'PATH;KEY')"),
-                    }
-                }
+            // If the current character is a backslash, and the next character is either ',' or '=',
+            // this is an escaping backslash. We eat the backslash, and write the next character as-is.
+            '\\' if matches!(char_iter.peek(), Some(',' | '=')) => {
+                string_buffer.push(char_iter.next().unwrap());
+            }
 
-                // We only handle ';' if there's more characters after it. If it's a trailing ';' we ignore it.
+            ',' => {
+                // We only handle ',' if there's more characters after it. If it's a trailing ',' we ignore it.
                 if char_iter.peek().is_some() {
                     // Add a '(key=value)' argument pair, and re-target `string_buffer` to point at the key's buffer.
                     plugin_args.push(Default::default());
@@ -109,13 +100,6 @@ fn plugin_parser<'a>(s: &str) -> Result<Plugin, &'a str> {
             '=' => match state {
                 State::Path => string_buffer.push('='), // '=' has no special meaning in the plugin path.
                 State::Key => {
-                    if string_buffer.is_empty() {
-                        return Err("missing argument key (ex: 'PATH;KEY=VALUE')");
-                    }
-                    if matches!(char_iter.peek(), None | Some(';')) {
-                        return Err("missing argument value (ex: 'PATH;KEY=VALUE' or 'PATH;KEY')");
-                    }
-
                     // Re-target `string_buffer` to point at the argument value's buffer (instead of the key).
                     string_buffer = &mut plugin_args.last_mut().unwrap().1;
                     state = State::Value;
@@ -131,11 +115,21 @@ fn plugin_parser<'a>(s: &str) -> Result<Plugin, &'a str> {
 
     // Trim any leading/trailing whitespace from the path and arguments.
     let path = plugin_path.trim().to_owned();
-    let args = plugin_args
+    let args: Vec<_> = plugin_args
         .into_iter()
         .map(|(key, value)| (key.trim().to_owned(), value.trim().to_owned()))
         .collect();
 
+    // Validate that the plugin path, and any argument keys are non-empty (empty argument values are fine).
+    if path.is_empty() {
+        return Err("missing plugin path (ex: 'PATH,KEY=VALUE')");
+    }
+    for arg in &args {
+        if arg.0.is_empty() {
+            return Err("missing argument key (ex: 'PATH,KEY=VALUE')");
+        }
+    }
+
     Ok(Plugin { path, args })
 }
 

slicec/tests/command_line_tests.rs

@@ -10,10 +10,11 @@ use test_case::test_case;
 
 mod test_helpers;
 
-#[test]
-fn plugins_can_have_no_arguments() {
+#[test_case("foo"; "without trailing separator")]
+#[test_case("foo,"; "with trailing separator")]
+fn plugins_can_have_no_arguments(value: &str) {
     // Arrange
-    let input = ["", "--generator", "foo"];
+    let input = ["", "--generator", value];
 
     // Act
     let result = SliceOptions::try_parse_from(input);
@@ -45,9 +46,9 @@ fn equals_sign_has_no_special_meaning_in_plugin_path() {
 }
 
 #[test]
-fn path_characters_can_be_escaped() {
+fn backslash_escapes_special_characters_in_path() {
     // Arrange
-    let input = ["", "--generator", "C:\\\\my\\;folder\\=cool/\\a"];
+    let input = ["", "--generator", "C:\\my\\,folder\\=cool/\\a"];
 
     // Act
     let result = SliceOptions::try_parse_from(input);
@@ -57,14 +58,55 @@ fn path_characters_can_be_escaped() {
     assert_eq!(parsed_options.generators.len(), 1);
 
     let generator_plugin = &parsed_options.generators[0];
-    assert_eq!(generator_plugin.path, "C:\\my;folder=cool/a");
+    assert_eq!(generator_plugin.path, "C:\\my,folder=cool/\\a");
     assert!(generator_plugin.args.is_empty());
 }
 
 #[test]
-fn plugins_can_have_a_single_argument() {
+fn backslash_escapes_special_characters_in_arguments() {
+    // Arrange
+    let input = ["", "--generator", "C:\\plugin,my\\=name\\,=my\\=value\\,,"];
+
+    // Act
+    let result = SliceOptions::try_parse_from(input);
+
+    // Assert
+    let parsed_options = result.unwrap();
+    assert_eq!(parsed_options.generators.len(), 1);
+
+    let generator_plugin = &parsed_options.generators[0];
+    assert_eq!(generator_plugin.path, "C:\\plugin");
+    assert_eq!(generator_plugin.args.len(), 1);
+
+    assert_eq!(generator_plugin.args[0].0, "my=name,");
+    assert_eq!(generator_plugin.args[0].1, "my=value,");
+}
+
+#[test]
+fn backslash_does_not_escape_normal_characters() {
+    // Arrange
+    let input = ["", "--generator", "C:\\Users\\Me\\_plugin,na\\me=\\value\\"];
+
+    // Act
+    let result = SliceOptions::try_parse_from(input);
+
+    // Assert
+    let parsed_options = result.unwrap();
+    assert_eq!(parsed_options.generators.len(), 1);
+
+    let generator_plugin = &parsed_options.generators[0];
+    assert_eq!(generator_plugin.path, "C:\\Users\\Me\\_plugin");
+    assert_eq!(generator_plugin.args.len(), 1);
+
+    assert_eq!(generator_plugin.args[0].0, "na\\me");
+    assert_eq!(generator_plugin.args[0].1, "\\value\\");
+}
+
+#[test_case("C:\\my_plugin,arg1=val1"; "without trailing separator")]
+#[test_case("C:\\my_plugin,arg1=val1,"; "with trailing separator")]
+fn plugins_can_have_a_single_argument(value: &str) {
     // Arrange
-    let input = ["", "--generator", "C:\\\\my_plugin;arg1=val1"];
+    let input = ["", "--generator", value];
 
     // Act
     let result = SliceOptions::try_parse_from(input);
@@ -84,7 +126,7 @@ fn plugins_can_have_a_single_argument() {
 #[test]
 fn plugins_arguments_have_whitespace_trimmed() {
     // Arrange
-    let input = ["", "--generator", "/path/to/plugin;	arg1  = val1 	 "];
+    let input = ["", "--generator", "/path/to/plugin,	arg1  = val1 	 "];
 
     // Act
     let result = SliceOptions::try_parse_from(input);
@@ -101,10 +143,12 @@ fn plugins_arguments_have_whitespace_trimmed() {
     assert_eq!(generator_plugin.args[0].1, "val1");
 }
 
-#[test]
-fn plugins_arguments_can_omit_a_value() {
+#[test_case("../plugin/path,foo,bar"; "without trailing equals")]
+#[test_case("../plugin/path,foo=,bar="; "with trailing equals")]
+#[test_case("../plugin/path,foo,bar=,"; "with trailing equals and separator")]
+fn plugins_arguments_can_omit_a_value(value: &str) {
     // Arrange
-    let input = ["", "--generator", "../plugin/path;foo;bar"];
+    let input = ["", "--generator", value];
 
     // Act
     let result = SliceOptions::try_parse_from(input);
@@ -126,7 +170,7 @@ fn plugins_arguments_can_omit_a_value() {
 #[test]
 fn plugins_can_have_multiple_arguments() {
     // Arrange
-    let input = ["", "--generator", "foo;arg1=val1;arg2;arg3 = val3"];
+    let input = ["", "--generator", "foo,arg1=val1,arg2, arg3 = val3 "];
 
     // Act
     let result = SliceOptions::try_parse_from(input);
@@ -147,9 +191,10 @@ fn plugins_can_have_multiple_arguments() {
     assert_eq!(generator_plugin.args[2].1, "val3");
 }
 
-#[test_case("foo;arg="; "without trailing argument")]
-#[test_case("foo;arg1=;arg2"; "with trailing argument")]
-fn argument_value_must_appear_after_equals(generator_arg: &str) {
+#[test_case("    "; "without trailing equals")]
+#[test_case("   ,"; "with trailing separator")]
+#[test_case(","; "only trailing separator")]
+fn plugin_path_must_be_present(generator_arg: &str) {
     // Arrange
     let input = ["", "--generator", generator_arg];
 
@@ -161,15 +206,12 @@ fn argument_value_must_appear_after_equals(generator_arg: &str) {
     assert_eq!(parsing_error.kind(), ErrorKind::ValueValidation);
 
     let error_message = parsing_error.source().unwrap().to_string();
-    assert_eq!(
-        error_message,
-        "missing argument value (ex: 'PATH;KEY=VALUE' or 'PATH;KEY')",
-    );
+    assert_eq!(error_message, "missing plugin path (ex: 'PATH,KEY=VALUE')");
 }
 
-#[test_case("foo;="; "trailing equals")]
-#[test_case("foo;=val"; "trailing equals with value")]
-#[test_case("foo;;"; "trailing semicolon")]
+#[test_case("foo,="; "trailing equals")]
+#[test_case("foo,=val"; "trailing equals with value")]
+#[test_case("foo,,"; "trailing separator")]
 fn argument_key_must_be_present(generator_arg: &str) {
     // Arrange
     let input = ["", "--generator", generator_arg];
@@ -182,13 +224,13 @@ fn argument_key_must_be_present(generator_arg: &str) {
     assert_eq!(parsing_error.kind(), ErrorKind::ValueValidation);
 
     let error_message = parsing_error.source().unwrap().to_string();
-    assert_eq!(error_message, "missing argument key (ex: 'PATH;KEY=VALUE')");
+    assert_eq!(error_message, "missing argument key (ex: 'PATH,KEY=VALUE')");
 }
 
 #[test]
 fn double_equals_is_disallowed() {
     // Arrange
-    let input = ["", "--generator", "/paths/are=/=okay;arg1=key=1"];
+    let input = ["", "--generator", "/paths/are=/=okay,arg1=key=1"];
 
     // Act
     let result = SliceOptions::try_parse_from(input);