Pass args from clap to roc/roc run child processes

This works much better than the previous "arg index"
approach, especially with subcommands like `roc run`

cli/src/lib.rs

@@ -97,8 +97,7 @@ pub fn build_app<'a>() -> Command<'a> {
 
     let roc_file_to_run = Arg::new(ROC_FILE)
         .help("The .roc file of an app to run")
-        .allow_invalid_utf8(true)
+        .allow_invalid_utf8(true);
-        .required(true);
 
     let args_for_app = Arg::new(ARGS_FOR_APP)
         .help("Arguments to pass into the app being run")
@@ -159,7 +158,7 @@ pub fn build_app<'a>() -> Command<'a> {
             .arg(flag_linker.clone())
             .arg(flag_precompiled.clone())
             .arg(flag_valgrind.clone())
-            .arg(roc_file_to_run.clone())
+            .arg(roc_file_to_run.clone().required(true))
             .arg(args_for_app.clone())
         )
         .subcommand(Command::new(CMD_FORMAT)
@@ -209,7 +208,7 @@ pub fn build_app<'a>() -> Command<'a> {
         .arg(flag_linker)
         .arg(flag_precompiled)
         .arg(flag_valgrind)
-        .arg(roc_file_to_run)
+        .arg(roc_file_to_run.required(false))
         .arg(args_for_app);
 
     if cfg!(feature = "editor") {
@@ -236,8 +235,8 @@ pub fn docs(files: Vec<PathBuf>) {
 #[derive(Debug, PartialEq, Eq)]
 pub enum BuildConfig {
     BuildOnly,
-    BuildAndRun { roc_file_arg_index: usize },
+    BuildAndRun,
-    BuildAndRunIfNoErrors { roc_file_arg_index: usize },
+    BuildAndRunIfNoErrors,
 }
 
 pub enum FormatMode {
@@ -372,7 +371,7 @@ pub fn build(
                     // Return a nonzero exit code if there were problems
                     Ok(problems.exit_code())
                 }
-                BuildAndRun { roc_file_arg_index } => {
+                BuildAndRun => {
                     if problems.errors > 0 || problems.warnings > 0 {
                         println!(
                             "\x1B[{}m{}\x1B[39m {} and \x1B[{}m{}\x1B[39m {} found in {} ms.\n\nRunning program anyway…\n\n\x1B[36m{}\x1B[39m",
@@ -403,15 +402,11 @@ pub fn build(
                         );
                     }
 
-                    roc_run(
+                    let args = matches.values_of_os(ARGS_FOR_APP).unwrap_or_default();
-                        arena,
+
-                        &original_cwd,
+                    roc_run(arena, &original_cwd, triple, args, &binary_path)
-                        triple,
-                        roc_file_arg_index,
-                        &binary_path,
-                    )
                 }
-                BuildAndRunIfNoErrors { roc_file_arg_index } => {
+                BuildAndRunIfNoErrors => {
                     if problems.errors == 0 {
                         if problems.warnings > 0 {
                             println!(
@@ -427,13 +422,9 @@ pub fn build(
                             );
                         }
 
-                        roc_run(
+                        let args = matches.values_of_os(ARGS_FOR_APP).unwrap_or_default();
-                            arena,
+
-                            &original_cwd,
+                        roc_run(arena, &original_cwd, triple, args, &binary_path)
-                            triple,
-                            roc_file_arg_index,
-                            &binary_path,
-                        )
                     } else {
                         println!(
                             "\x1B[{}m{}\x1B[39m {} and \x1B[{}m{}\x1B[39m {} found in {} ms.\n\nYou can run the program anyway with: \x1B[32mroc run {}\x1B[39m",
@@ -479,16 +470,13 @@ pub fn build(
     }
 }
 
-#[cfg(target_family = "unix")]
+fn roc_run<'a, I: IntoIterator<Item = &'a OsStr>>(
-fn roc_run(
     arena: Bump, // This should be passed an owned value, not a reference, so we can usefully mem::forget it!
     cwd: &Path,
     triple: Triple,
-    roc_file_arg_index: usize,
+    args: I,
     binary_path: &Path,
 ) -> io::Result<i32> {
-    use std::os::unix::process::CommandExt;
-
     match triple.architecture {
         Architecture::Wasm32 => {
             // If possible, report the generated executable name relative to the current dir.
@@ -500,19 +488,44 @@ fn roc_run(
             // since the process is about to exit anyway.
             std::mem::forget(arena);
 
-            let args = std::env::args()
+            if cfg!(target_family = "unix") {
-                .skip(roc_file_arg_index)
+                use std::os::unix::ffi::OsStrExt;
-                .collect::<Vec<_>>();
 
-            run_with_wasmer(generated_filename, &args);
+                run_with_wasmer(
-            return Ok(0);
+                    generated_filename,
+                    args.into_iter().map(|os_str| os_str.as_bytes()),
+                );
+            } else {
+                run_with_wasmer(
+                    generated_filename,
+                    args.into_iter().map(|os_str| {
+                        os_str.to_str().expect(
+                            "Roc does not currently support passing non-UTF8 arguments to Wasmer.",
+                        )
+                    }),
+                );
             }
-        _ => std::process::Command::new(&binary_path),
-    };
 
-    if let Architecture::Wasm32 = triple.architecture {
+            Ok(0)
-        cmd.arg(binary_path);
         }
+        _ => {
+            if cfg!(target_family = "unix") {
+                roc_run_unix(cwd, args, binary_path)
+            } else {
+                roc_run_non_unix(arena, cwd, args, binary_path)
+            }
+        }
+    }
+}
+
+fn roc_run_unix<I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
+    cwd: &Path,
+    args: I,
+    binary_path: &Path,
+) -> io::Result<i32> {
+    use std::os::unix::process::CommandExt;
+
+    let mut cmd = std::process::Command::new(&binary_path);
 
     // Forward all the arguments after the .roc file argument
     // to the new process. This way, you can do things like:
@@ -521,11 +534,9 @@ fn roc_run(
     //
     // ...and have it so that app.roc will receive only `foo`,
     // `bar`, and `baz` as its arguments.
-    for (index, arg) in std::env::args().enumerate() {
+    for arg in args {
-        if index > roc_file_arg_index {
         cmd.arg(arg);
     }
-    }
 
     // This is much faster than spawning a subprocess if we're on a UNIX system!
     let err = cmd.current_dir(cwd).exec();
@@ -536,29 +547,36 @@ fn roc_run(
     Err(err)
 }
 
-#[cfg(not(target_family = "unix"))]
+fn roc_run_non_unix<I: IntoIterator<Item = S>, S: AsRef<OsStr>>(
-fn roc_run(cmd: &mut Command) -> io::Result<i32> {
+    _arena: Bump, // This should be passed an owned value, not a reference, so we can usefully mem::forget it!
-    // Run the compiled app
+    _cwd: &Path,
-    let exit_status = cmd
+    _args: I,
-                        .spawn()
+    _binary_path: &Path,
-                        .unwrap_or_else(|err| panic!("Failed to run app after building it: {:?}", err))
+) -> io::Result<i32> {
-                        .wait()
+    todo!("TODO support running roc programs on non-UNIX targets");
-                        .expect("TODO gracefully handle block_on failing when `roc` spawns a subprocess for the compiled app");
+    // let mut cmd = std::process::Command::new(&binary_path);
 
-    // `roc [FILE]` exits with the same status code as the app it ran.
+    // // Run the compiled app
-    //
+    // let exit_status = cmd
-    // If you want to know whether there were compilation problems
+    //     .spawn()
-    // via status code, use either `roc build` or `roc check` instead!
+    //     .unwrap_or_else(|err| panic!("Failed to run app after building it: {:?}", err))
-    match exit_status.code() {
+    //     .wait()
-        Some(code) => Ok(code),
+    //     .expect("TODO gracefully handle block_on failing when `roc` spawns a subprocess for the compiled app");
-        None => {
+
-            todo!("TODO gracefully handle the `roc [FILE]` subprocess terminating with a signal.");
+    // // `roc [FILE]` exits with the same status code as the app it ran.
-        }
+    // //
-    }
+    // // If you want to know whether there were compilation problems
+    // // via status code, use either `roc build` or `roc check` instead!
+    // match exit_status.code() {
+    //     Some(code) => Ok(code),
+    //     None => {
+    //         todo!("TODO gracefully handle the `roc [FILE]` subprocess terminating with a signal.");
+    //     }
+    // }
 }
 
 #[cfg(feature = "run-wasm32")]
-fn run_with_wasmer(wasm_path: &std::path::Path, args: &[String]) {
+fn run_with_wasmer<I: Iterator<Item = S>, S: AsRef<[u8]>>(wasm_path: &std::path::Path, args: I) {
     use wasmer::{Instance, Module, Store};
 
     let store = Store::default();
@@ -589,8 +607,8 @@ fn run_with_wasmer(wasm_path: &std::path::Path, args: &[String]) {
 }
 
 #[cfg(not(feature = "run-wasm32"))]
-fn run_with_wasmer(_wasm_path: &std::path::Path, _args: &[String]) {
+fn run_with_wasmer<I: Iterator<Item = S>, S: AsRef<[u8]>>(_wasm_path: &std::path::Path, _args: I) {
-    println!("Running wasm files not support");
+    println!("Running wasm files is not supported on this target.");
 }
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq)]

cli/src/main.rs

@@ -26,37 +26,34 @@ fn main() -> io::Result<()> {
     let matches = build_app().get_matches();
 
     let exit_code = match matches.subcommand() {
-        None => match matches.index_of(ROC_FILE) {
+        None => {
-            Some(arg_index) => build(
+            if matches.is_present(ROC_FILE) {
+                build(
                     &matches,
-                BuildConfig::BuildAndRunIfNoErrors {
+                    BuildConfig::BuildAndRunIfNoErrors,
-                    roc_file_arg_index: arg_index + 1,
-                },
                     Triple::host(),
                     LinkType::Executable,
-            ),
+                )
-
+            } else {
-            None => {
                 launch_editor(None)?;
 
                 Ok(0)
             }
-        },
+        }
-        Some((CMD_RUN, matches)) => match matches.index_of(ROC_FILE) {
+        Some((CMD_RUN, matches)) => {
-            Some(arg_index) => build(
+            if matches.is_present(ROC_FILE) {
+                build(
                     matches,
-                BuildConfig::BuildAndRun {
+                    BuildConfig::BuildAndRun,
-                    roc_file_arg_index: arg_index + 1,
-                },
                     Triple::host(),
                     LinkType::Executable,
-            ),
+                )
-            None => {
+            } else {
                 eprintln!("What .roc file do you want to run? Specify it at the end of the `roc run` command.");
 
                 Ok(1)
             }
-        },
+        }
         Some((CMD_BUILD, matches)) => {
             let target: Target = matches.value_of_t(FLAG_TARGET).unwrap_or_default();