From 5bc1410bd6a3fa6d1778476bc288f7c87b586dbc Mon Sep 17 00:00:00 2001 From: Inanna Malick Date: Sun, 13 Aug 2023 14:00:26 -0700 Subject: [PATCH] Quick sketch adding derived cmd line arg parsing to veilid-cli Quick sketch showing cmd line args via clap-derive (same underlying parser, but simplifies cmd line parsing logic and reduces surface area for potential bugs) --- Cargo.lock | 117 ++++++++++++++++++++++++++++++++++++- veilid-cli/Cargo.toml | 2 +- veilid-cli/src/main.rs | 85 +++++++++++---------------- veilid-cli/src/settings.rs | 6 +- 4 files changed, 152 insertions(+), 58 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c8247f0e..9082082b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -154,6 +154,55 @@ dependencies = [ "winapi", ] +[[package]] +name = "anstream" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ca84f3628370c59db74ee214b3263d58f9aadd9b4fe7e711fd87dc452b7f163" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is-terminal", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a30da5c5f2d5e72842e00bcb57657162cdabef0931f40e2deb9b4140440cecd" + +[[package]] +name = "anstyle-parse" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "938874ff5980b03a87c5524b3ae5b59cf99b1d6bc836848df7bc5ada9643c333" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ca11d4be1bab0c8bc8734a9aa7bf4ee8316d462a08c6ac5052f888fef5b494b" +dependencies = [ + "windows-sys 0.48.0", +] + +[[package]] +name = "anstyle-wincon" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c677ab05e09154296dd37acecd46420c17b9713e8366facafa8fc0885167cf4c" +dependencies = [ + "anstyle", + "windows-sys 0.48.0", +] + [[package]] name = "anyhow" version = "1.0.72" @@ -966,13 +1015,48 @@ checksum = "4ea181bf566f71cb9a5d17a59e1871af638180a18fb0035c92ae62b705207123" dependencies = [ "atty", "bitflags 1.3.2", - "clap_lex", + "clap_lex 0.2.4", "indexmap 1.9.3", "strsim 0.10.0", "termcolor", "textwrap 0.16.0", ] +[[package]] +name = "clap" +version = "4.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c27cdf28c0f604ba3f512b0c9a409f8de8513e4816705deb0498b627e7c3a3fd" +dependencies = [ + "clap_builder", + "clap_derive", + "once_cell", +] + +[[package]] +name = "clap_builder" +version = "4.3.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08a9f1ab5e9f01a9b81f202e8562eb9a10de70abf9eaeac1be465c28b75aa4aa" +dependencies = [ + "anstream", + "anstyle", + "clap_lex 0.5.0", + "strsim 0.10.0", +] + +[[package]] +name = "clap_derive" +version = "4.3.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "54a9bb5758fc5dfe728d1019941681eccaf0cf8a4189b692a0ee2f2ecf90a050" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 2.0.27", +] + [[package]] name = "clap_lex" version = "0.2.4" @@ -982,6 +1066,12 @@ dependencies = [ "os_str_bytes", ] +[[package]] +name = "clap_lex" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2da6da31387c7e4ef160ffab6d5e7f00c42626fe39aea70a7b0f1773f7dd6c1b" + [[package]] name = "clipboard-win" version = "4.5.0" @@ -1021,6 +1111,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d7b894f5411737b7867f4827955924d7c254fc9f4d91a6aad6b097804b1018b" +[[package]] +name = "colorchoice" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" + [[package]] name = "combine" version = "4.6.6" @@ -2552,6 +2648,17 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "28b29a3cd74f0f4598934efe3aeba42bae0eb4680554128851ebbecb02af14e6" +[[package]] +name = "is-terminal" +version = "0.4.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb0889898416213fab133e1d33a0e5858a48177452750691bde3666d0fdbaf8b" +dependencies = [ + "hermit-abi 0.3.2", + "rustix 0.38.4", + "windows-sys 0.48.0", +] + [[package]] name = "itertools" version = "0.10.5" @@ -5622,6 +5729,12 @@ version = "0.7.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09cc8ee72d2a9becf2f2febe0205bbed8fc6615b7cb429ad062dc7b7ddd036a9" +[[package]] +name = "utf8parse" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" + [[package]] name = "valuable" version = "0.1.0" @@ -5655,7 +5768,7 @@ dependencies = [ "async-tungstenite 0.8.0", "bugsalot", "cfg-if 1.0.0", - "clap 3.2.25", + "clap 4.3.21", "config", "crossbeam-channel", "cursive", diff --git a/veilid-cli/Cargo.toml b/veilid-cli/Cargo.toml index 6bebace4..7c046997 100644 --- a/veilid-cli/Cargo.toml +++ b/veilid-cli/Cargo.toml @@ -27,7 +27,7 @@ cursive_buffered_backend = { path = "../external/cursive_buffered_backend" } cursive_table_view = "0.14.0" arboard = "3.2.0" # cursive-tabs = "0.5.0" -clap = "^3" +clap = {version= "4", features = ["derive"]} directories = "^4" log = "^0" futures = "^0" diff --git a/veilid-cli/src/main.rs b/veilid-cli/src/main.rs index f51c58e3..fb436cff 100644 --- a/veilid-cli/src/main.rs +++ b/veilid-cli/src/main.rs @@ -4,11 +4,9 @@ use crate::tools::*; -use clap::{Arg, ColorChoice, Command}; +use clap::{Parser, ValueEnum}; use flexi_logger::*; -use std::ffi::OsStr; -use std::net::ToSocketAddrs; -use std::path::Path; +use std::{net::ToSocketAddrs, path::PathBuf}; mod client_api_connection; mod command_processor; @@ -17,62 +15,45 @@ mod settings; mod tools; mod ui; -fn parse_command_line(default_config_path: &OsStr) -> Result { - let matches = Command::new("veilid-cli") - .version("0.1") - .color(ColorChoice::Auto) - .about("Veilid Console Client") - .arg( - Arg::new("address") - .required(false) - .help("Address to connect to"), - ) - .arg( - Arg::new("debug") - .long("debug") - .help("Turn on debug logging"), - ) - .arg( - Arg::new("wait-for-debug") - .long("wait-for-debug") - .help("Wait for debugger to attach"), - ) - .arg( - Arg::new("trace") - .long("trace") - .conflicts_with("debug") - .help("Turn on trace logging"), - ) - .arg( - Arg::new("config-file") - .short('c') - .takes_value(true) - .value_name("FILE") - .default_value_os(default_config_path) - .allow_invalid_utf8(true) - .help("Specify a configuration file to use"), - ) - .get_matches(); +#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ValueEnum, Debug)] +enum LogLevel { + /// Turn on debug logging + Debug, + /// Turn on trace logging + Trace, +} - Ok(matches) +#[derive(Parser, Debug)] +#[command(author, version, about = "Veilid Console Client")] +struct CmdlineArgs { + /// Address to connect to + #[arg(long)] + address: Option, + /// Wait for debugger to attach + #[arg(long)] + wait_for_debug: bool, + /// Specify a configuration file to use + #[arg(short, long, value_name = "FILE", /*allow_invalid_utf8 = true*/)] + config_file: Option, + /// log level + #[arg(value_enum)] + log_level: Option, } fn main() -> Result<(), String> { // Get command line options let default_config_path = settings::Settings::get_default_config_path(); - let matches = parse_command_line(default_config_path.as_os_str())?; - if matches.occurrences_of("wait-for-debug") != 0 { + let args = CmdlineArgs::parse(); + + if args.wait_for_debug { use bugsalot::debugger; debugger::wait_until_attached(None).expect("state() not implemented on this platform"); } // Attempt to load configuration - let settings_path = if let Some(config_file) = matches.value_of_os("config-file") { - if Path::new(config_file).exists() { - Some(config_file) - } else { - None - } + let settings_path = args.config_file.unwrap_or(default_config_path); + let settings_path = if settings_path.exists() { + Some(settings_path.into_os_string()) } else { None }; @@ -81,11 +62,11 @@ fn main() -> Result<(), String> { .map_err(|e| format!("configuration is invalid: {}", e))?; // Set config from command line - if matches.occurrences_of("debug") != 0 { + if let Some(LogLevel::Debug) = args.log_level { settings.logging.level = settings::LogLevel::Debug; settings.logging.terminal.enabled = true; } - if matches.occurrences_of("trace") != 0 { + if let Some(LogLevel::Trace) = args.log_level { settings.logging.level = settings::LogLevel::Trace; settings.logging.terminal.enabled = true; } @@ -142,7 +123,7 @@ fn main() -> Result<(), String> { } } // Get client address - let server_addrs = if let Some(address_arg) = matches.value_of("address") { + let server_addrs = if let Some(address_arg) = args.address { address_arg .to_socket_addrs() .map_err(|e| format!("Invalid server address '{}'", e))? diff --git a/veilid-cli/src/settings.rs b/veilid-cli/src/settings.rs index 5cc9268f..a34b14ff 100644 --- a/veilid-cli/src/settings.rs +++ b/veilid-cli/src/settings.rs @@ -1,7 +1,7 @@ use directories::*; use serde_derive::*; -use std::ffi::OsStr; +use std::ffi::OsString; use std::net::{SocketAddr, ToSocketAddrs}; use std::path::{Path, PathBuf}; @@ -234,13 +234,13 @@ impl Settings { default_log_directory } - pub fn new(config_file: Option<&OsStr>) -> Result { + pub fn new(config_file: Option) -> Result { // Load the default config let mut cfg = load_default_config()?; // Merge in the config file if we have one if let Some(config_file) = config_file { - let config_file_path = Path::new(config_file); + let config_file_path = Path::new(&config_file); // If the user specifies a config file on the command line then it must exist cfg = load_config(cfg, config_file_path)?; }