chore: Move CliCommand::run -> CliArgs::run; do not mutate the configuration

This way CliArgs::run has access to all command line parameters.
Avoided mutating the CliArgs (or rather CliCommand) structure here,
because doing so is simply bad style. There is no good reasoning for
why this function should mutate CliCommand, except for a bit of
convenience.
This commit is contained in:
Karolin Varner
2024-08-03 15:32:15 +02:00
parent 1ab457ed37
commit 3cc3b6009f
3 changed files with 17 additions and 17 deletions

View File

@@ -149,14 +149,14 @@ pub enum CliCommand {
Man, Man,
} }
impl CliCommand { impl CliArgs {
/// runs the command specified via CLI /// runs the command specified via CLI
/// ///
/// ## TODO /// ## TODO
/// - This method consumes the [`CliCommand`] value. It might be wise to use a reference... /// - This method consumes the [`CliCommand`] value. It might be wise to use a reference...
pub fn run(self, test_helpers: Option<AppServerTest>) -> anyhow::Result<()> { pub fn run(self, test_helpers: Option<AppServerTest>) -> anyhow::Result<()> {
use CliCommand::*; use CliCommand::*;
match self { match &self.command {
Man => { Man => {
let man_cmd = std::process::Command::new("man") let man_cmd = std::process::Command::new("man")
.args(["1", "rosenpass"]) .args(["1", "rosenpass"])
@@ -168,7 +168,7 @@ impl CliCommand {
} }
GenConfig { config_file, force } => { GenConfig { config_file, force } => {
ensure!( ensure!(
force || !config_file.exists(), *force || !config_file.exists(),
"config file {config_file:?} already exists" "config file {config_file:?} already exists"
); );
@@ -183,9 +183,9 @@ impl CliCommand {
let mut secret_key: Option<PathBuf> = None; let mut secret_key: Option<PathBuf> = None;
// Manual arg parsing, since clap wants to prefix flags with "--" // Manual arg parsing, since clap wants to prefix flags with "--"
let mut args = args.into_iter(); let mut args = args.iter();
loop { loop {
match (args.next().as_deref(), args.next()) { match (args.next().map(|x| x.as_str()), args.next()) {
(Some("private-key"), Some(opt)) | (Some("secret-key"), Some(opt)) => { (Some("private-key"), Some(opt)) | (Some("secret-key"), Some(opt)) => {
secret_key = Some(opt.into()); secret_key = Some(opt.into());
} }
@@ -227,7 +227,7 @@ impl CliCommand {
(config.public_key, config.secret_key) (config.public_key, config.secret_key)
} }
(_, Some(pkf), Some(skf)) => (pkf, skf), (_, Some(pkf), Some(skf)) => (pkf.clone(), skf.clone()),
_ => { _ => {
bail!("either a config-file or both public-key and secret-key file are required") bail!("either a config-file or both public-key and secret-key file are required")
} }
@@ -266,16 +266,17 @@ impl CliCommand {
Exchange { Exchange {
first_arg, first_arg,
mut rest_of_args, rest_of_args,
config_file, config_file,
} => { } => {
rest_of_args.insert(0, first_arg); let mut rest_of_args = rest_of_args.clone();
rest_of_args.insert(0, first_arg.clone());
let args = rest_of_args; let args = rest_of_args;
let mut config = config::Rosenpass::parse_args(args)?; let mut config = config::Rosenpass::parse_args(args)?;
if let Some(p) = config_file { if let Some(p) = config_file {
config.store(&p)?; config.store(p)?;
config.config_file_path = p; config.config_file_path.clone_from(p);
} }
config.validate()?; config.validate()?;
Self::event_loop(config, test_helpers)?; Self::event_loop(config, test_helpers)?;
@@ -283,7 +284,7 @@ impl CliCommand {
Validate { config_files } => { Validate { config_files } => {
for file in config_files { for file in config_files {
match config::Rosenpass::load(&file) { match config::Rosenpass::load(file) {
Ok(config) => { Ok(config) => {
eprintln!("{file:?} is valid TOML and conforms to the expected schema"); eprintln!("{file:?} is valid TOML and conforms to the expected schema");
match config.validate() { match config.validate() {
@@ -305,6 +306,7 @@ impl CliCommand {
test_helpers: Option<AppServerTest>, test_helpers: Option<AppServerTest>,
) -> anyhow::Result<()> { ) -> anyhow::Result<()> {
const MAX_PSK_SIZE: usize = 1000; const MAX_PSK_SIZE: usize = 1000;
// load own keys // load own keys
let sk = SSk::load(&config.secret_key)?; let sk = SSk::load(&config.secret_key)?;
let pk = SPk::load(&config.public_key)?; let pk = SPk::load(&config.public_key)?;
@@ -313,7 +315,7 @@ impl CliCommand {
let mut srv = std::boxed::Box::<AppServer>::new(AppServer::new( let mut srv = std::boxed::Box::<AppServer>::new(AppServer::new(
sk, sk,
pk, pk,
config.listen, config.listen.clone(),
config.verbosity, config.verbosity,
test_helpers, test_helpers,
)?); )?);

View File

@@ -34,7 +34,7 @@ pub fn main() {
// error!("error dummy"); // error!("error dummy");
} }
match args.command.run(None) { match args.run(None) {
Ok(_) => {} Ok(_) => {}
Err(e) => { Err(e) => {
error!("{e:?}"); error!("{e:?}");

View File

@@ -104,8 +104,7 @@ fn run_server_client_exchange(
.unwrap(); .unwrap();
std::thread::spawn(move || { std::thread::spawn(move || {
cli.command cli.run(Some(
.run(Some(
server_test_builder server_test_builder
.termination_handler(Some(server_terminate_rx)) .termination_handler(Some(server_terminate_rx))
.build() .build()
@@ -122,8 +121,7 @@ fn run_server_client_exchange(
.unwrap(); .unwrap();
std::thread::spawn(move || { std::thread::spawn(move || {
cli.command cli.run(Some(
.run(Some(
client_test_builder client_test_builder
.termination_handler(Some(client_terminate_rx)) .termination_handler(Some(client_terminate_rx))
.build() .build()