From 5ea5f588a964e46fefcac1efabb7f241c719f7ac Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 10:12:55 +0200 Subject: [PATCH 1/9] Fix panics due to unwrap on Option::None --- bin/src/main.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index 25d47ce3..12af035e 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -170,7 +170,11 @@ fn main() { match matches.subcommand() { (subcommand, Some(scmd)) => { - let subcommand_args : Vec<&str> = scmd.values_of("").unwrap().collect(); + debug!("Calling with subcommand: {}", subcommand); + let subcommand_args : Vec<&str> = match scmd.values_of("") { + Some(values) => values.collect(), + None => Vec::new() + }; debug!("Calling 'imag-{}' with args: {:?}", subcommand, subcommand_args); match Command::new(format!("imag-{}", subcommand)) From e6d48cb31ab6eee22ae5db3e8ace8f882835303f Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 10:16:36 +0200 Subject: [PATCH 2/9] Fix exit codes --- bin/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index 12af035e..885f0163 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -189,7 +189,7 @@ fn main() { if !exit_status.success() { debug!("{} exited with non-zero exit code: {:?}", subcommand, exit_status); println!("{} exited with non-zero exit code", subcommand); - exit(exit_status.code().unwrap_or(42)); + exit(exit_status.code().unwrap_or(1)); } debug!("Successful exit!"); }, @@ -208,7 +208,7 @@ fn main() { }, _ => { println!("Error spawning: {:?}", e); - exit(1337); + exit(1); } } } From 7023d1f2027b1402ca837efc626029e2cb1d687f Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 10:31:00 +0200 Subject: [PATCH 3/9] Add check if given subcommand is supported --- bin/src/main.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bin/src/main.rs b/bin/src/main.rs index 885f0163..de383068 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -175,6 +175,13 @@ fn main() { Some(values) => values.collect(), None => Vec::new() }; + + if !get_commands().contains(&String::from(subcommand)) { + println!("No such command: 'imag-{}'", subcommand); + println!("See 'imag --help' for available subcommands"); + exit(2); + } + debug!("Calling 'imag-{}' with args: {:?}", subcommand, subcommand_args); match Command::new(format!("imag-{}", subcommand)) @@ -198,6 +205,8 @@ fn main() { debug!("Error calling the subcommand"); match e.kind() { ErrorKind::NotFound => { + // With the check above, this absolutely should not happen. + // Keeping it to be safe println!("No such command: 'imag-{}'", subcommand); println!("See 'imag --help' for available subcommands"); exit(2); From c828bed0e15cb7b545403c49a1c3b56c53f26488 Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 10:37:16 +0200 Subject: [PATCH 4/9] Fix panic! when reaching unreachable!, because imag doesnt do anything without an argument or subcommand --- bin/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index de383068..b708b861 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -114,7 +114,7 @@ fn main() { let commands = get_commands(); let helptext = help_text(commands); let app = Runtime::get_default_cli_builder(appname, version, about) - .settings(&[AppSettings::AllowExternalSubcommands]) + .settings(&[AppSettings::AllowExternalSubcommands, AppSettings::ArgRequiredElseHelp]) .arg(Arg::with_name("version") .long("version") .takes_value(false) From d69b8498e9e82072da70430143b2141f1f85e015 Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 11:07:02 +0200 Subject: [PATCH 5/9] Add some comments --- bin/src/main.rs | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index b708b861..4f6f84d7 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -20,6 +20,8 @@ use clap::{Arg, AppSettings, SubCommand}; use libimagrt::runtime::Runtime; use libimagerror::trace::trace_error; +/// Returns the helptext, putting the Strings in cmds as possible +/// subcommands into it fn help_text(cmds: Vec) -> String { let text = format!(r#" @@ -60,7 +62,7 @@ fn help_text(cmds: Vec) -> String { text } - +/// Returns the list of imag-* executables found in $PATH fn get_commands() -> Vec { let path = env::var("PATH"); if path.is_err() { @@ -108,11 +110,12 @@ fn get_commands() -> Vec { fn main() { + // Initialize the Runtime and build the CLI let appname = "imag"; let version = &version!(); let about = "imag - the PIM suite for the commandline"; let commands = get_commands(); - let helptext = help_text(commands); + let helptext = help_text(commands.clone()); let app = Runtime::get_default_cli_builder(appname, version, about) .settings(&[AppSettings::AllowExternalSubcommands, AppSettings::ArgRequiredElseHelp]) .arg(Arg::with_name("version") @@ -139,6 +142,8 @@ fn main() { debug!("matches: {:?}", matches); + // Begin checking for arguments + if matches.is_present("version") { debug!("Showing version"); println!("imag {}", &version!()[..]); @@ -168,15 +173,19 @@ fn main() { } } + // Matches any subcommand given match matches.subcommand() { (subcommand, Some(scmd)) => { - debug!("Calling with subcommand: {}", subcommand); + // Get all given arguments and further subcommands to pass to + // the imag-<> binary + // Providing no arguments is OK, and is therefore ignored here let subcommand_args : Vec<&str> = match scmd.values_of("") { Some(values) => values.collect(), None => Vec::new() }; - if !get_commands().contains(&String::from(subcommand)) { + // Typos happen, so check if the given subcommand is one found in $PATH + if !commands.clone().contains(&String::from(subcommand)) { println!("No such command: 'imag-{}'", subcommand); println!("See 'imag --help' for available subcommands"); exit(2); @@ -184,6 +193,7 @@ fn main() { debug!("Calling 'imag-{}' with args: {:?}", subcommand, subcommand_args); + // Create a Command, and pass it the gathered arguments match Command::new(format!("imag-{}", subcommand)) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) @@ -223,9 +233,8 @@ fn main() { } } }, - // clap ensures we have valid input by exiting if not. - // The above case is a catch-all for subcommands, - // so nothing else needs to be expexted. - _ => unreachable!(), + // Calling for example 'imag --versions' will lead here, as this option does not exit. + // There's nothing to do in such a case + _ => {}, } } From 12f9175700ce9da71de1c37fa3607b9440bc5828 Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 11:14:08 +0200 Subject: [PATCH 6/9] Fix --versions --- bin/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index 4f6f84d7..f8259d52 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -156,7 +156,7 @@ fn main() { for command in get_commands().iter() { result.push(crossbeam::scope(|scope| { scope.spawn(|| { - let v = Command::new(command).arg("--version").output(); + let v = Command::new(format!("imag-{}",command)).arg("--version").output(); match v { Ok(v) => match String::from_utf8(v.stdout) { Ok(s) => format!("{} -> {}", command, s), From e813ab9e3a58c67e7322dd4c74d8b49ac162a569 Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 11:24:36 +0200 Subject: [PATCH 7/9] Pretty output of --versions --- bin/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index f8259d52..70aaf5d0 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -159,7 +159,7 @@ fn main() { let v = Command::new(format!("imag-{}",command)).arg("--version").output(); match v { Ok(v) => match String::from_utf8(v.stdout) { - Ok(s) => format!("{} -> {}", command, s), + Ok(s) => format!("{:10} -> {}", command, s), Err(e) => format!("Failed calling {} -> {:?}", command, e), }, Err(e) => format!("Failed calling {} -> {:?}", command, e), @@ -169,7 +169,7 @@ fn main() { } for versionstring in result.into_iter().map(|handle| handle.join()) { - println!("{}", versionstring); + print!("{}", versionstring); } } From e2d3e5597bbfb64b337de207eb0af67922ab62f4 Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 12:53:38 +0200 Subject: [PATCH 8/9] Fix differing amount of newlines from subprocesses influences output --- bin/src/main.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index 70aaf5d0..afe37cbb 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -169,7 +169,8 @@ fn main() { } for versionstring in result.into_iter().map(|handle| handle.join()) { - print!("{}", versionstring); + // The amount of newlines may differ depending on the subprocess + println!("{}", versionstring.trim()); } } From 6fe440880070e53bc7c1a68d2941cbc108909691 Mon Sep 17 00:00:00 2001 From: Mario Krehl Date: Wed, 7 Sep 2016 12:57:34 +0200 Subject: [PATCH 9/9] Reduce clone()-ing of commands to necessary places --- bin/src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bin/src/main.rs b/bin/src/main.rs index afe37cbb..71d56820 100644 --- a/bin/src/main.rs +++ b/bin/src/main.rs @@ -153,7 +153,7 @@ fn main() { if matches.is_present("versions") { debug!("Showing versions"); let mut result = vec![]; - for command in get_commands().iter() { + for command in commands.iter() { result.push(crossbeam::scope(|scope| { scope.spawn(|| { let v = Command::new(format!("imag-{}",command)).arg("--version").output(); @@ -186,7 +186,7 @@ fn main() { }; // Typos happen, so check if the given subcommand is one found in $PATH - if !commands.clone().contains(&String::from(subcommand)) { + if !commands.contains(&String::from(subcommand)) { println!("No such command: 'imag-{}'", subcommand); println!("See 'imag --help' for available subcommands"); exit(2);