Merge pull request #7004 from paritytech/cli-arguments-backwards-compatible

Make CLI arguments parsing more backwards compatible

Cargo.lock

@@ -1,17 +1,3 @@
-[root]
-name = "wasm"
-version = "0.1.0"
-dependencies = [
- "byteorder 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "ethcore-bigint 0.2.1",
- "ethcore-logger 1.9.0",
- "ethcore-util 1.9.0",
- "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
- "parity-wasm 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "vm 0.1.0",
- "wasm-utils 0.1.0 (git+https://github.com/paritytech/wasm-utils)",
-]
-
 [[package]]
 name = "adler32"
 version = "1.0.2"
@@ -3422,6 +3408,20 @@ name = "void"
 version = "1.0.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "wasm"
+version = "0.1.0"
+dependencies = [
+ "byteorder 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "ethcore-bigint 0.2.1",
+ "ethcore-logger 1.9.0",
+ "ethcore-util 1.9.0",
+ "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "parity-wasm 0.15.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "vm 0.1.0",
+ "wasm-utils 0.1.0 (git+https://github.com/paritytech/wasm-utils)",
+]
+
 [[package]]
 name = "wasm-utils"
 version = "0.1.0"

parity/cli/mod.rs

@@ -24,6 +24,7 @@ usage! {
 		// Subcommands must start with cmd_ and have '_' in place of '-'
 		// Sub-subcommands must start with the name of the subcommand
 		// Arguments must start with arg_
+		// Flags must start with flag_
 
 		CMD cmd_ui {
 			"Manage ui",
@@ -53,10 +54,6 @@ usage! {
 
 			CMD cmd_account_new {
 				"Create a new acount",
-
-				ARG arg_account_new_password: (Option<String>) = None,
-				"--password=[FILE]",
-				"Path to the password file",
 			}
 
 			CMD cmd_account_list {
@@ -81,10 +78,6 @@ usage! {
 			{
 				"Import wallet",
 
-				ARG arg_wallet_import_password: (Option<String>) = None,
-				"--password=[FILE]",
-				"Path to the password file",
-
 				ARG arg_wallet_import_path: (Option<String>) = None,
 				"<PATH>",
 				"Path to the wallet",
@@ -179,10 +172,6 @@ usage! {
 			{
 				"Sign",
 
-				ARG arg_signer_sign_password: (Option<String>) = None,
-				"--password=[FILE]",
-				"Path to the password file",
-
 				ARG arg_signer_sign_id: (Option<usize>) = None,
 				"[ID]",
 				"ID",
@@ -244,7 +233,7 @@ usage! {
 		}
 	}
 	{
-		// Flags and arguments
+		// Global flags and arguments
 		["Operating Options"]
 			FLAG flag_public_node: (bool) = false, or |c: &Config| otry!(c.parity).public_node.clone(),
 			"--public-node",
@@ -353,7 +342,6 @@ usage! {
 			ARG arg_password: (Vec<String>) = Vec::new(), or |c: &Config| otry!(c.account).password.clone(),
 			"--password=[FILE]...",
 			"Provide a file containing a password for unlocking an account. Leading and trailing whitespace is trimmed.",
-
 		["UI options"]
 			FLAG flag_force_ui: (bool) = false, or |c: &Config| otry!(c.ui).force.clone(),
 			"--force-ui",
@@ -840,6 +828,10 @@ usage! {
 			"Target size of the whisper message pool in megabytes.",
 
 		["Legacy options"]
+			FLAG flag_warp: (bool) = false, or |_| None,
+			"--warp",
+			"Does nothing; warp sync is enabled by default.",
+
 			FLAG flag_dapps_apis_all: (bool) = false, or |_| None,
 			"--dapps-apis-all",
 			"Dapps server is merged with RPC server. Use --jsonrpc-apis.",
@@ -1208,6 +1200,29 @@ mod tests {
 	use toml;
 	use clap::{ErrorKind as ClapErrorKind};
 
+	#[test]
+	fn should_accept_any_argument_order() {
+		let args = Args::parse(&["parity", "--no-warp", "account", "list"]).unwrap();
+		assert_eq!(args.flag_no_warp, true);
+
+		let args = Args::parse(&["parity", "account", "list", "--no-warp"]).unwrap();
+		assert_eq!(args.flag_no_warp, true);
+
+		let args = Args::parse(&["parity", "--chain=dev", "account", "list"]).unwrap();
+		assert_eq!(args.arg_chain, "dev");
+
+		let args = Args::parse(&["parity", "account", "list", "--chain=dev"]).unwrap();
+		assert_eq!(args.arg_chain, "dev");
+	}
+
+	#[test]
+	fn should_not_crash_on_warp() {
+		let args = Args::parse(&["parity", "--warp"]);
+		assert!(args.is_ok());
+
+		let args = Args::parse(&["parity", "account", "list", "--warp"]);
+		assert!(args.is_ok());
+	}
 
 	#[test]
 	fn should_reject_invalid_values() {
@@ -1380,9 +1395,6 @@ mod tests {
 			arg_restore_file: None,
 			arg_tools_hash_file: None,
 
-			arg_account_new_password: None,
-			arg_signer_sign_password: None,
-			arg_wallet_import_password: None,
 			arg_signer_sign_id: None,
 			arg_signer_reject_id: None,
 			arg_dapp_path: None,
@@ -1565,6 +1577,7 @@ mod tests {
 			arg_whisper_pool_size: 20,
 
 			// -- Legacy Options
+			flag_warp: false,
 			flag_geth: false,
 			flag_testnet: false,
 			flag_import_geth_keys: false,

parity/cli/usage.rs

@@ -153,7 +153,7 @@ macro_rules! usage {
 		use std::{fs, io, process};
 		use std::io::{Read, Write};
 		use util::version;
-		use clap::{Arg, App, SubCommand, AppSettings, Error as ClapError, ErrorKind as ClapErrorKind};
+		use clap::{Arg, App, SubCommand, AppSettings, ArgMatches as ClapArgMatches, Error as ClapError, ErrorKind as ClapErrorKind};
 		use helpers::replace_home;
 		use std::ffi::OsStr;
 		use std::collections::HashMap;
@@ -503,6 +503,36 @@ macro_rules! usage {
 				args
 			}
 
+			pub fn hydrate_with_globals(self: &mut Self, matches: &ClapArgMatches) -> Result<(), ClapError> {
+				$(
+					$(
+						self.$flag = self.$flag || matches.is_present(stringify!($flag));
+					)*
+					$(
+						if let some @ Some(_) = return_if_parse_error!(if_option!(
+							$($arg_type_tt)+,
+							THEN {
+								if_option_vec!(
+									$($arg_type_tt)+,
+									THEN { values_t!(matches, stringify!($arg), inner_option_vec_type!($($arg_type_tt)+)) }
+									ELSE { value_t!(matches, stringify!($arg), inner_option_type!($($arg_type_tt)+)) }
+								)
+							}
+							ELSE {
+								if_vec!(
+									$($arg_type_tt)+,
+									THEN { values_t!(matches, stringify!($arg), inner_vec_type!($($arg_type_tt)+)) }
+									ELSE { value_t!(matches, stringify!($arg), $($arg_type_tt)+) }
+								)
+							}
+						)) {
+							self.$arg = some;
+						}
+					)*
+				)*
+				Ok(())
+			}
+
 			#[allow(unused_variables)] // the submatches of arg-less subcommands aren't used
 			pub fn parse<S: AsRef<str>>(command: &[S]) -> Result<Self, ClapError> {
 
@@ -559,12 +589,14 @@ macro_rules! usage {
 								SubCommand::with_name(&underscore_to_hyphen!(&stringify!($subc)[4..]))
 								.about($subc_help)
 								.args(&subc_usages.get(stringify!($subc)).unwrap().iter().map(|u| Arg::from_usage(u).use_delimiter(false).allow_hyphen_values(true)).collect::<Vec<Arg>>())
+								.args(&usages.iter().map(|u| Arg::from_usage(u).use_delimiter(false).allow_hyphen_values(true)).collect::<Vec<Arg>>()) // accept global arguments at this position
 								$(
 									.setting(AppSettings::SubcommandRequired) // prevent from running `parity account`
 									.subcommand(
 										SubCommand::with_name(&underscore_to_hyphen!(&stringify!($subc_subc)[stringify!($subc).len()+1..]))
 										.about($subc_subc_help)
 										.args(&subc_usages.get(stringify!($subc_subc)).unwrap().iter().map(|u| Arg::from_usage(u).use_delimiter(false).allow_hyphen_values(true)).collect::<Vec<Arg>>())
+										.args(&usages.iter().map(|u| Arg::from_usage(u).use_delimiter(false).allow_hyphen_values(true)).collect::<Vec<Arg>>()) // accept global arguments at this position
 									)
 								)*
 							)
@@ -572,36 +604,16 @@ macro_rules! usage {
 						.get_matches_from_safe(command.iter().map(|x| OsStr::new(x.as_ref())))?;
 
 				let mut raw_args : RawArgs = Default::default();
-				$(
+
-					$(
+				raw_args.hydrate_with_globals(&matches)?;
-						raw_args.$flag = matches.is_present(stringify!($flag));
-					)*
-					$(
-						raw_args.$arg = return_if_parse_error!(if_option!(
-							$($arg_type_tt)+,
-							THEN {
-								if_option_vec!(
-									$($arg_type_tt)+,
-									THEN { values_t!(matches, stringify!($arg), inner_option_vec_type!($($arg_type_tt)+)) }
-									ELSE { value_t!(matches, stringify!($arg), inner_option_type!($($arg_type_tt)+)) }
-								)
-							}
-							ELSE {
-								if_vec!(
-									$($arg_type_tt)+,
-									THEN { values_t!(matches, stringify!($arg), inner_vec_type!($($arg_type_tt)+)) }
-									ELSE { value_t!(matches, stringify!($arg), $($arg_type_tt)+) }
-								)
-							}
-						));
-					)*
-				)*
 
 				// Subcommands
 				$(
 					if let Some(submatches) = matches.subcommand_matches(&underscore_to_hyphen!(&stringify!($subc)[4..])) {
 						raw_args.$subc = true;
 
+						// Globals
+						raw_args.hydrate_with_globals(&submatches)?;
 						// Subcommand flags
 						$(
 							raw_args.$subc_flag = submatches.is_present(&stringify!($subc_flag));
@@ -626,12 +638,13 @@ macro_rules! usage {
 										}
 							));
 						)*
-
 						// Sub-subcommands
 						$(
 							if let Some(subsubmatches) = submatches.subcommand_matches(&underscore_to_hyphen!(&stringify!($subc_subc)[stringify!($subc).len()+1..])) {
 								raw_args.$subc_subc = true;
 
+								// Globals
+								raw_args.hydrate_with_globals(&subsubmatches)?;
 								// Sub-subcommand flags
 								$(
 									raw_args.$subc_subc_flag = subsubmatches.is_present(&stringify!($subc_subc_flag));

parity/configuration.rs

@@ -143,7 +143,7 @@ impl Configuration {
 			if self.args.cmd_signer_new_token {
 				Cmd::SignerToken(ws_conf, ui_conf, logger_config.clone())
 			} else if self.args.cmd_signer_sign {
-				let pwfile = self.args.arg_signer_sign_password.map(|pwfile| {
+				let pwfile = self.args.arg_password.first().map(|pwfile| {
 					PathBuf::from(pwfile)
 				});
 				Cmd::SignerSign {
@@ -180,7 +180,7 @@ impl Configuration {
 					iterations: self.args.arg_keys_iterations,
 					path: dirs.keys,
 					spec: spec,
-					password_file: self.args.arg_account_new_password.clone(),
+					password_file: self.args.arg_password.first().map(|x| x.to_owned()),
 				};
 				AccountCmd::New(new_acc)
 			} else if self.args.cmd_account_list {
@@ -215,7 +215,7 @@ impl Configuration {
 				path: dirs.keys,
 				spec: spec,
 				wallet_path: self.args.arg_wallet_import_path.unwrap().clone(),
-				password_file: self.args.arg_wallet_import_password,
+				password_file: self.args.arg_password.first().map(|x| x.to_owned()),
 			};
 			Cmd::ImportPresaleWallet(presale_cmd)
 		} else if self.args.cmd_import {

parity/deprecated.rs

@@ -37,6 +37,10 @@ impl fmt::Display for Deprecated {
 pub fn find_deprecated(args: &Args) -> Vec<Deprecated> {
 	let mut result = vec![];
 
+	if args.flag_warp {
+		result.push(Deprecated::DoesNothing("--warp"));
+	}
+
 	if args.flag_jsonrpc {
 		result.push(Deprecated::DoesNothing("--jsonrpc"));
 	}
@@ -117,6 +121,7 @@ mod tests {
 		assert_eq!(find_deprecated(&Args::default()), vec![]);
 		assert_eq!(find_deprecated(&{
 			let mut args = Args::default();
+			args.flag_warp = true;
 			args.flag_jsonrpc = true;
 			args.flag_rpc = true;
 			args.flag_jsonrpc_off = true;
@@ -135,6 +140,7 @@ mod tests {
 			args.flag_dapps_apis_all = true;
 			args
 		}), vec![
+			Deprecated::DoesNothing("--warp"),
 			Deprecated::DoesNothing("--jsonrpc"),
 			Deprecated::DoesNothing("--rpc"),
 			Deprecated::Replaced("--jsonrpc-off", "--no-jsonrpc"),