From 91db3535f802770082d44f6238acbf4bdfdd770a Mon Sep 17 00:00:00 2001 From: Kirill Pimenov Date: Thu, 9 Nov 2017 14:42:17 +0100 Subject: [PATCH] Merge pull request #7004 from paritytech/cli-arguments-backwards-compatible Make CLI arguments parsing more backwards compatible --- Cargo.lock | 28 +++++++++--------- parity/cli/mod.rs | 47 ++++++++++++++++++----------- parity/cli/usage.rs | 65 ++++++++++++++++++++++++----------------- parity/configuration.rs | 6 ++-- parity/deprecated.rs | 6 ++++ 5 files changed, 92 insertions(+), 60 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1b5b86280..b72d6bb4f 100644 --- a/Cargo.lock +++ b/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.1.3", - "ethcore-logger 1.8.0", - "ethcore-util 1.8.2", - "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" @@ -3580,6 +3566,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.1.3", + "ethcore-logger 1.8.0", + "ethcore-util 1.8.2", + "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" diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index 774256d02..3f5099b61 100644 --- a/parity/cli/mod.rs +++ b/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) = 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) = None, - "--password=[FILE]", - "Path to the password file", - ARG arg_wallet_import_path: (Option) = None, "", "Path to the wallet", @@ -179,10 +172,6 @@ usage! { { "Sign", - ARG arg_signer_sign_password: (Option) = None, - "--password=[FILE]", - "Path to the password file", - ARG arg_signer_sign_id: (Option) = 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) = 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", @@ -836,6 +824,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.", @@ -1203,6 +1195,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() { @@ -1375,9 +1390,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, @@ -1559,6 +1571,7 @@ mod tests { arg_whisper_pool_size: 20, // -- Legacy Options + flag_warp: false, flag_geth: false, flag_testnet: false, flag_import_geth_keys: false, diff --git a/parity/cli/usage.rs b/parity/cli/usage.rs index 83b878ee1..71ac01295 100644 --- a/parity/cli/usage.rs +++ b/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>(command: &[S]) -> Result { @@ -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::>()) + .args(&usages.iter().map(|u| Arg::from_usage(u).use_delimiter(false).allow_hyphen_values(true)).collect::>()) // 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::>()) + .args(&usages.iter().map(|u| Arg::from_usage(u).use_delimiter(false).allow_hyphen_values(true)).collect::>()) // 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.$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)+) } - ) - } - )); - )* - )* + + raw_args.hydrate_with_globals(&matches)?; // 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)); diff --git a/parity/configuration.rs b/parity/configuration.rs index 83b71f05d..dc390d5e7 100644 --- a/parity/configuration.rs +++ b/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 { diff --git a/parity/deprecated.rs b/parity/deprecated.rs index d80ea3357..b41475d9d 100644 --- a/parity/deprecated.rs +++ b/parity/deprecated.rs @@ -37,6 +37,10 @@ impl fmt::Display for Deprecated { pub fn find_deprecated(args: &Args) -> Vec { 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"),