Repository: jacobdeichert/mask (language: rust)

Issue:
Support optional (non-required) positional arguments
Required arguments use the round brackets like `(required_arg)` so optional args can probably use square brackets `[optional_arg]`.

```md
## say [phrase]

~~~sh
if [[ $phrase != "" ]]; then
    echo "$phrase"
else
    echo "😶"
fi
~~~
```


Discussion / hints:
I'll take this one sometime this week.
I would suggest `(arg_name?)` where the question denotes optional, much like in TypeScript types.
Hmmm, i like that idea. That would make it very easy to tell what's optional and what's not.
it will be good if we can provide default value
I would suggest following traditional shell syntax for brackets: <required> [optional] 
> I would suggest following traditional shell syntax for brackets: [optional]

Yeah, agree that sticking with traditional shell syntax is better – going to go with [optional] instead of (optional?) 👍 
> it will be good if we can provide default value

Maybe like this: `[optional=default_value]`  ?

Still thinking about this... named flags don't have support for a default value yet either. If we add it here, we should add to named flags as well.
Could also do traditional shell syntax for mandatory args: `<required>` as opposed to current `(required)`

> Could also do traditional shell syntax for mandatory args: `<required>` as opposed to current `(required)`


Yeah, I originally had that but then changed it in #16 because a few markdown renderers I noticed were interpreting `<required>` args as html :/

I might have to take another look at that and see which renderers (like GitHub, or VS Code markdown preview) break... can't remember off the top of my head. 
> Maybe like this: [optional=default_value]?

> If we add it here, we should add to named flags as well.

These both make sense. Just make sure to only set default values for optional args not required ones!



Fix patch:
diff --git a/mask-parser/src/maskfile.rs b/mask-parser/src/maskfile.rs
index c5f0f28..cf35293 100644
--- a/mask-parser/src/maskfile.rs
+++ b/mask-parser/src/maskfile.rs
@@ -22,6 +22,7 @@ pub struct Command {
     pub script: Option<Script>,
     pub subcommands: Vec<Command>,
     pub required_args: Vec<RequiredArg>,
+    pub optional_args: Vec<OptionalArg>,
     pub named_flags: Vec<NamedFlag>,
 }
 
@@ -34,6 +35,7 @@ impl Command {
             script: Some(Script::new()),
             subcommands: vec![],
             required_args: vec![],
+            optional_args: vec![],
             named_flags: vec![],
         }
     }
@@ -99,6 +101,23 @@ impl RequiredArg {
     }
 }
 
+#[derive(Debug, Serialize, Clone)]
+pub struct OptionalArg {
+    pub name: String,
+    /// Used within mask. TODO: store in a different place within mask instead of here.
+    #[serde(skip)]
+    pub val: String,
+}
+
+impl OptionalArg {
+    pub fn new(name: String) -> Self {
+        Self {
+            name,
+            val: "".to_string(),
+        }
+    }
+}
+
 #[derive(Debug, Serialize, Clone)]
 pub struct NamedFlag {
     pub name: String,
diff --git a/mask-parser/src/parser.rs b/mask-parser/src/parser.rs
index 9971f2d..dffb50a 100644
--- a/mask-parser/src/parser.rs
+++ b/mask-parser/src/parser.rs
@@ -56,9 +56,11 @@ pub fn parse(maskfile_contents: String) -> Maskfile {
             }
             End(tag) => match tag {
                 Tag::Header(_) => {
-                    let (name, required_args) = parse_command_name_and_required_args(text.clone());
+                    let (name, required_args, optional_args) =
+                        parse_command_name_required_and_optional_args(text.clone());
                     current_command.name = name;
                     current_command.required_args = required_args;
+                    current_command.optional_args = optional_args;
                 }
                 Tag::BlockQuote => {
                     current_command.description = text.clone();
@@ -230,23 +232,37 @@ fn treeify_commands(commands: Vec<Command>) -> Vec<Command> {
     command_tree
 }
 
-fn parse_command_name_and_required_args(text: String) -> (String, Vec<RequiredArg>) {
-    // Find any required arguments. They look like this: (required_arg_name)
-    let name_and_args: Vec<&str> = text.split(|c| c == '(' || c == ')').collect();
-    let (name, args) = name_and_args.split_at(1);
-    let name = name.join(" ").trim().to_string();
-    let mut required_args: Vec<RequiredArg> = vec![];
-
-    if !args.is_empty() {
-        let args = args.join("");
-        let args: Vec<&str> = args.split(" ").collect();
-        required_args = args
-            .iter()
-            .map(|a| RequiredArg::new(a.to_string()))
-            .collect();
-    }
-
-    (name, required_args)
+fn parse_command_name_required_and_optional_args(
+    text: String,
+) -> (String, Vec<RequiredArg>, Vec<OptionalArg>) {
+    // Checks if any args are present and if not, return early
+    let split_idx = match text.find(|c| c == '(' || c == '[') {
+        Some(idx) => idx,
+        None => return (text.trim().to_string(), vec![], vec![]),
+    };
+
+    let (name, args) = text.split_at(split_idx);
+    let name = name.trim().to_string();
+
+    // Collects (required_args)
+    let required_args = args
+        .split(|c| c == '(' || c == ')')
+        .filter_map(|arg| match arg.trim() {
+            a if !a.is_empty() && !a.contains('[') => Some(RequiredArg::new(a.trim().to_string())),
+            _ => None,
+        })
+        .collect();
+
+    // Collects [optional_args]
+    let optional_args = args
+        .split(|c| c == '[' || c == ']')
+        .filter_map(|arg| match arg.trim() {
+            a if !a.is_empty() && !a.contains('(') => Some(OptionalArg::new(a.trim().to_string())),
+            _ => None,
+        })
+        .collect();
+
+    (name, required_args, optional_args)
 }
 
 #[cfg(test)]
@@ -285,6 +301,18 @@ echo hey
 ## no_script
 
 This command has no source/script.
+
+## multi (required) [optional]
+
+> Example with optional args
+
+~~~bash
+if ! [ -z "$optional" ]; then
+ echo "This is optional - $optional"
+fi
+
+echo "This is required - $required"
+~~~
 "#;
 
 #[cfg(test)]
@@ -327,6 +355,7 @@ mod parse {
                                 "name": "port"
                             }
                         ],
+                        "optional_args": [],
                         "named_flags": [verbose_flag],
                     },
                     {
@@ -343,6 +372,7 @@ mod parse {
                                 "name": "name"
                             }
                         ],
+                        "optional_args": [],
                         "named_flags": [verbose_flag],
                     },
                     {
@@ -360,12 +390,27 @@ mod parse {
                                     "source": "echo hey\n",
                                 },
                                 "subcommands": [],
+                                "optional_args": [],
                                 "required_args": [],
                                 "named_flags": [verbose_flag],
                             }
                         ],
                         "required_args": [],
+                        "optional_args": [],
                         "named_flags": [],
+                    },
+                    {
+                        "level": 2,
+                        "name": "multi",
+                        "description": "Example with optional args",
+                        "script": {
+                            "executor": "bash",
+                            "source": "if ! [ -z \"$optional\" ]; then\n echo \"This is optional - $optional\"\nfi\n\necho \"This is required - $required\"\n",
+                        },
+                        "subcommands": [],
+                        "required_args": [{ "name": "required" }],
+                        "optional_args": [{ "name": "optional" }],
+                        "named_flags": [verbose_flag],
                     }
                 ]
             }),
diff --git a/mask/src/executor.rs b/mask/src/executor.rs
index 10d26b6..c58e52a 100644
--- a/mask/src/executor.rs
+++ b/mask/src/executor.rs
@@ -120,6 +120,11 @@ fn add_flag_variables(mut child: process::Command, cmd: &Command) -> process::Co
         child.env(arg.name.clone(), arg.val.clone());
     }
 
+    // Add all optional args
+    for opt_arg in &cmd.optional_args {
+        child.env(opt_arg.name.clone(), opt_arg.val.clone());
+    }
+
     // Add all named flags as environment variables if they have a value
     for flag in &cmd.named_flags {
         if flag.val != "" {
diff --git a/mask/src/main.rs b/mask/src/main.rs
index cb6f6ea..613d23b 100644
--- a/mask/src/main.rs
+++ b/mask/src/main.rs
@@ -132,6 +132,12 @@ fn build_subcommands<'a, 'b>(
             subcmd = subcmd.arg(arg);
         }
 
+        // Add all optional arguments
+        for o in &c.optional_args {
+            let arg = Arg::with_name(&o.name);
+            subcmd = subcmd.arg(arg);
+        }
+
         // Add all named flags
         for f in &c.named_flags {
             let arg = Arg::with_name(&f.name)
@@ -174,6 +180,14 @@ fn get_command_options(mut cmd: Command, matches: &ArgMatches) -> Command {
         arg.val = matches.value_of(arg.name.clone()).unwrap().to_string();
     }
 
+    // Check optional args
+    for opt_arg in &mut cmd.optional_args {
+        opt_arg.val = matches
+            .value_of(opt_arg.name.clone())
+            .unwrap_or("")
+            .to_string();
+    }
+
     // Check all named flags
     for flag in &mut cmd.named_flags {
         flag.val = if flag.takes_value {


Test patch:
diff --git a/mask/tests/arguments_and_flags_test.rs b/mask/tests/arguments_and_flags_test.rs
index d3bbb74..e78d1a9 100644
--- a/mask/tests/arguments_and_flags_test.rs
+++ b/mask/tests/arguments_and_flags_test.rs
@@ -510,3 +510,68 @@ Write-Output "This shouldn't render"
             .failure();
     }
 }
+
+mod optional_args {
+    use predicates::boolean::PredicateBooleanExt;
+
+    use super::*;
+
+    #[test]
+    fn runs_with_optional_args() {
+        let (_temp, maskfile_path) = common::maskfile(
+            r#"
+## with_opt (required) [optional]
+
+~~~bash
+echo "$required" "$optional"
+~~~
+
+~~~powershell
+param(
+    $req = $env:required,
+    $opt = $env:optional
+)
+
+Write-Output "$req $opt"
+~~~
+"#,
+        );
+
+        common::run_mask(&maskfile_path)
+            .cli("with_opt")
+            .arg("I am required")
+            .arg("I am optional")
+            .assert()
+            .stdout(contains("I am required I am optional"))
+            .success();
+    }
+
+    #[test]
+    fn does_not_fail_when_optional_arg_is_not_present() {
+        let (_temp, maskfile_path) = common::maskfile(
+            r#"
+## with_opt (required) [optional]
+
+~~~bash
+echo "$required" "$optional"
+~~~
+
+~~~powershell
+param(
+    $req = $env:required,
+    $opt = $env:optional
+)
+
+Write-Output "$req $opt"
+~~~
+"#,
+        );
+
+        common::run_mask(&maskfile_path)
+            .cli("with_opt")
+            .arg("I am required")
+            .assert()
+            .stdout(contains("I am optional").not())
+            .success();
+    }
+}
diff --git a/mask/tests/introspect_test.rs b/mask/tests/introspect_test.rs
index 7c2a88f..a343b70 100644
--- a/mask/tests/introspect_test.rs
+++ b/mask/tests/introspect_test.rs
@@ -44,6 +44,7 @@ echo something
                 },
                 "subcommands": [],
                 "required_args": [],
+                "optional_args": [],
                 "named_flags": [verbose_flag],
             }
         ]