From 778392f729f2ab4360fe3747c930abf4007f3798 Mon Sep 17 00:00:00 2001 From: stephen Date: Thu, 2 Jan 2025 00:42:54 +0000 Subject: [PATCH] better error handling using Result instead of panic --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/lib.rs | 82 ++++++++++++++++++++++++++++++----------------------- src/main.rs | 16 +++++++---- src/sway.rs | 9 ++++-- 5 files changed, 66 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 928004c..8d03b43 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -192,7 +192,7 @@ dependencies = [ [[package]] name = "swayout" -version = "1.2.1" +version = "1.2.2" dependencies = [ "kdl", "serde", diff --git a/Cargo.toml b/Cargo.toml index f472479..353b746 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "swayout" -version = "1.2.1" +version = "1.2.2" edition = "2021" description = "A tool to configure sway outputs" authors = ["Stephen Byrne"] diff --git a/src/lib.rs b/src/lib.rs index b429725..b1f1778 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -135,7 +135,7 @@ pub fn print_layout_names() { /// or the name of an output. /// Return true of any outputs were enabled. /// Return false if there were no outputs to enable and thus nothing was done. -pub fn apply_layout(layout_name: &String) -> bool { +pub fn apply_layout(layout_name: &String) -> Result<(), String> { let config = get_config(); let outputs = get_outputs(); let monitor_states = get_monitor_states(&config, &outputs); @@ -151,33 +151,42 @@ pub fn apply_layout(layout_name: &String) -> bool { .find(|layout| &layout.name == layout_name) { // The layout is defined in the config. - layout - .outputs - .iter() - .for_each(|(monitor_name, output_config)| { - let monitor_state = get_monitor_state_by_name(&monitor_states, &monitor_name) - .unwrap_or_else(||panic!("Cannot find monitor '{}' for layout '{}'", monitor_name, layout_name)); + for (monitor_name, output_config) in &layout.outputs { + // there is probably a syntactic sugar for this? + let monitor_state = if let Some(monitor_state) = + get_monitor_state_by_name(&monitor_states, &monitor_name) + { + monitor_state + } else { + return Err(format!( + "Cannot find monitor '{}' for layout '{}'", + monitor_name, layout_name + )); + }; - if monitor_state.is_lid_closed { - panic!("Monitor '{}' is closed", monitor_name); - } + if monitor_state.is_lid_closed { + return Err(format!("Monitor '{}' is closed.", monitor_name)); + } - // find the output for the monitor to get the output name. - let output_opt = outputs - .iter() - .find(|output| monitor_matches_output(monitor_state.monitor, output)); - if let Some(output) = output_opt { - output_config_map.insert(&output.name, &output_config); - } else { - panic!("Missing output for monitor '{monitor_name}'"); - } - }); + // find the output for the monitor to get the output name. + let output_opt = outputs + .iter() + .find(|output| monitor_matches_output(monitor_state.monitor, output)); + if let Some(output) = output_opt { + output_config_map.insert(&output.name, &output_config); + } else { + return Err(format!("Missing output for monitor '{}'.", monitor_name)); + } + } } else { // The layout is not defined in the rules. // See if it is a monitor name... if let Some(monitor_state) = get_monitor_state_by_name(&monitor_states, &layout_name) { if monitor_state.is_lid_closed { - panic!("Monitor '{}' is closed", &monitor_state.monitor.name); + return Err(format!( + "Monitor '{}' is closed.", + &monitor_state.monitor.name + )); } // It is a monitor name. Find the matching output... @@ -187,29 +196,34 @@ pub fn apply_layout(layout_name: &String) -> bool { { output_config_map.insert(&output.name, &output.output_config); } else { - panic!("Could not find output for monitor '{layout_name}'") + return Err(format!( + "Could not find output for monitor '{}'.", + layout_name + )); } } else { // See if it is an output name... if let Some(output) = outputs.iter().find(|output| &output.name == layout_name) { output_config_map.insert(&output.name, &output.output_config); } else { - panic!("Could not find layout, monitor, or output '{layout_name}'") + return Err(format!( + "Could not find layout, monitor, or output '{}.'", + layout_name + )); } } } - if ! output_config_map.is_empty() { - apply_outputs(&outputs, &output_config_map); - true - } else { - false + if output_config_map.is_empty() { + return Err("No outputs would be enabled. Do nothing.".to_string()); } + + apply_outputs(&outputs, &output_config_map) } /// Apply the first automatic layout for which all outputs are available. /// Return the name of the layout applied or None if no automatic layout was available. -pub fn apply_automatic() -> Option { +pub fn apply_automatic() -> Result { let outputs = get_outputs(); let config = get_config(); @@ -219,12 +233,10 @@ pub fn apply_automatic() -> Option { .iter() .find(|layout| layout.automatic) { - if apply_layout(&layout.name) { - Some(String::from(&layout.name)) - } else { - None - } + + apply_layout(&layout.name)?; + Ok(String::from(&layout.name)) } else { - None + Err("No automatic layouts available.".to_string()) } } diff --git a/src/main.rs b/src/main.rs index 11da524..c0c2133 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,14 +8,18 @@ fn main() { } else if args.len() == 2 { let arg = &args[1]; if arg == "--automatic" { - if let Some(layout_name) = swayout::apply_automatic() { - println!("{}", layout_name); - } else { - eprintln!("no automatic layout available"); - process::exit(2) + match swayout::apply_automatic() { + Ok(layout_name) => { + println!("{}", layout_name); + } + Err(e) => { + eprintln!("{}", e); + process::exit(2) + } } } else { - if ! swayout::apply_layout(&args[1]) { + if let Err(e) = swayout::apply_layout(&args[1]) { + eprintln!("{}", e); process::exit(3) } } diff --git a/src/sway.rs b/src/sway.rs index f81bc75..2502be2 100644 --- a/src/sway.rs +++ b/src/sway.rs @@ -67,7 +67,10 @@ fn get_mode(output: &Value) -> String { } /// Apply the specified outputs. Enable all outputs in [outputs], disable others. -pub fn apply_outputs(all_outputs: &Vec, outputs: &HashMap<&String, &OutputConfig>) { +pub fn apply_outputs( + all_outputs: &Vec, + outputs: &HashMap<&String, &OutputConfig>, +) -> Result<(), String> { // set enabled outputs first, then set disabled outputs. // That way if some work before an error, you have at least one output enabled. @@ -85,7 +88,7 @@ pub fn apply_outputs(all_outputs: &Vec, outputs: &HashMap<&String, &Outp }); if enabled.is_empty() { - panic!("No enabled outputs!"); + return Err("No enabled outputs!".to_string()); } let mut cmd = Command::new("swaymsg"); @@ -117,4 +120,6 @@ pub fn apply_outputs(all_outputs: &Vec, outputs: &HashMap<&String, &Outp .for_each(|arg| print!("{} ", arg.to_str().unwrap())); cmd.output().expect("swaymsg output failed"); + + Ok(()) }