From 8279361b39c9b91e4e9f184301ea5b1025470aca Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 7 Nov 2019 13:52:22 -0800 Subject: [PATCH] Move subcommand execution into Subcommand (#514) Moves the code which executes subcommands into Subcommand:run, delegating to separate functions for each subcommand. This reduces the disgustingness of `run::run` a bit, and paves the way for future refactoring and cleanup. --- src/common.rs | 2 - src/config.rs | 4 +- src/run.rs | 197 +---------------------------------------- src/subcommand.rs | 218 +++++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 221 insertions(+), 200 deletions(-) diff --git a/src/common.rs b/src/common.rs index 8d5653c..003d4c6 100644 --- a/src/common.rs +++ b/src/common.rs @@ -3,9 +3,7 @@ pub(crate) use std::{ borrow::Cow, cmp, collections::{BTreeMap, BTreeSet}, - convert::AsRef, env, - ffi::OsStr, fmt::{self, Display, Formatter}, fs, io::{self, Write}, diff --git a/src/config.rs b/src/config.rs index 6af8a7b..50ba564 100644 --- a/src/config.rs +++ b/src/config.rs @@ -291,7 +291,7 @@ impl<'a> Config<'a> { } else if let Some(name) = matches.value_of(cmd::SHOW) { Subcommand::Show { name } } else { - Subcommand::Run + Subcommand::Execute }; Ok(Config { @@ -315,7 +315,7 @@ impl<'a> Config<'a> { impl<'a> Default for Config<'a> { fn default() -> Config<'static> { Config { - subcommand: Subcommand::Run, + subcommand: Subcommand::Execute, dry_run: false, highlight: false, overrides: empty(), diff --git a/src/run.rs b/src/run.rs index 87d5435..21dbde5 100644 --- a/src/run.rs +++ b/src/run.rs @@ -1,35 +1,5 @@ use crate::common::*; -use crate::interrupt_handler::InterruptHandler; -use unicode_width::UnicodeWidthStr; - -fn edit>(path: P) -> Result<(), i32> { - let editor = match env::var_os("EDITOR") { - None => { - eprintln!("Error getting EDITOR environment variable"); - return Err(EXIT_FAILURE); - } - Some(editor) => editor, - }; - - let error = Command::new(editor).arg(path).status(); - - match error { - Ok(status) => { - if status.success() { - Ok(()) - } else { - eprintln!("Editor failed: {}", status); - Err(status.code().unwrap_or(EXIT_FAILURE)) - } - } - Err(error) => { - eprintln!("Failed to invoke editor: {}", error); - Err(EXIT_FAILURE) - } - } -} - pub fn run() -> Result<(), i32> { #[cfg(windows)] ansi_term::enable_ansi_support().ok(); @@ -92,7 +62,7 @@ pub fn run() -> Result<(), i32> { let text; if let (Some(justfile), Some(directory)) = (justfile, working_directory) { if config.subcommand == Subcommand::Edit { - return edit(justfile); + return Subcommand::edit(justfile); } text = fs::read_to_string(justfile) @@ -113,7 +83,7 @@ pub fn run() -> Result<(), i32> { match search::justfile(¤t_dir) { Ok(name) => { if config.subcommand == Subcommand::Edit { - return edit(name); + return Subcommand::edit(&name); } text = match fs::read_to_string(&name) { Err(error) => { @@ -161,166 +131,5 @@ pub fn run() -> Result<(), i32> { } } - if config.subcommand == Subcommand::Summary { - if justfile.count() == 0 { - eprintln!("Justfile contains no recipes."); - } else { - let summary = justfile - .recipes - .iter() - .filter(|&(_, recipe)| !recipe.private) - .map(|(name, _)| name) - .cloned() - .collect::>() - .join(" "); - println!("{}", summary); - } - return Ok(()); - } - - if config.subcommand == Subcommand::Dump { - println!("{}", justfile); - return Ok(()); - } - - if config.subcommand == Subcommand::List { - // Construct a target to alias map. - let mut recipe_aliases: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); - for alias in justfile.aliases.values() { - if alias.is_private() { - continue; - } - - if !recipe_aliases.contains_key(alias.target.lexeme()) { - recipe_aliases.insert(alias.target.lexeme(), vec![alias.name.lexeme()]); - } else { - let aliases = recipe_aliases.get_mut(alias.target.lexeme()).unwrap(); - aliases.push(alias.name.lexeme()); - } - } - - let mut line_widths: BTreeMap<&str, usize> = BTreeMap::new(); - - for (name, recipe) in &justfile.recipes { - if recipe.private { - continue; - } - - for name in iter::once(name).chain(recipe_aliases.get(name).unwrap_or(&Vec::new())) { - let mut line_width = UnicodeWidthStr::width(*name); - - for parameter in &recipe.parameters { - line_width += UnicodeWidthStr::width(format!(" {}", parameter).as_str()); - } - - if line_width <= 30 { - line_widths.insert(name, line_width); - } - } - } - - let max_line_width = cmp::min(line_widths.values().cloned().max().unwrap_or(0), 30); - - let doc_color = config.color.stdout().doc(); - println!("Available recipes:"); - - for (name, recipe) in &justfile.recipes { - if recipe.private { - continue; - } - - let alias_doc = format!("alias for `{}`", recipe.name); - - for (i, name) in iter::once(name) - .chain(recipe_aliases.get(name).unwrap_or(&Vec::new())) - .enumerate() - { - print!(" {}", name); - for parameter in &recipe.parameters { - if config.color.stdout().active() { - print!(" {:#}", parameter); - } else { - print!(" {}", parameter); - } - } - - // Declaring this outside of the nested loops will probably be more efficient, but - // it creates all sorts of lifetime issues with variables inside the loops. - // If this is inlined like the docs say, it shouldn't make any difference. - let print_doc = |doc| { - print!( - " {:padding$}{} {}", - "", - doc_color.paint("#"), - doc_color.paint(doc), - padding = max_line_width - .saturating_sub(line_widths.get(name).cloned().unwrap_or(max_line_width)) - ); - }; - - match (i, recipe.doc) { - (0, Some(doc)) => print_doc(doc), - (0, None) => (), - _ => print_doc(&alias_doc), - } - println!(); - } - } - - return Ok(()); - } - - if let Subcommand::Show { name } = config.subcommand { - if let Some(alias) = justfile.get_alias(name) { - let recipe = justfile.get_recipe(alias.target.lexeme()).unwrap(); - println!("{}", alias); - println!("{}", recipe); - return Ok(()); - } - if let Some(recipe) = justfile.get_recipe(name) { - println!("{}", recipe); - return Ok(()); - } else { - eprintln!("Justfile does not contain recipe `{}`.", name); - if let Some(suggestion) = justfile.suggest(name) { - eprintln!("Did you mean `{}`?", suggestion); - } - return Err(EXIT_FAILURE); - } - } - - let arguments = if !config.arguments.is_empty() { - config.arguments.clone() - } else if let Some(recipe) = justfile.first() { - let min_arguments = recipe.min_arguments(); - if min_arguments > 0 { - die!( - "Recipe `{}` cannot be used as default recipe since it requires at least {} {}.", - recipe.name, - min_arguments, - Count("argument", min_arguments), - ); - } - vec![recipe.name()] - } else { - die!("Justfile contains no recipes."); - }; - - if let Err(error) = InterruptHandler::install() { - warn!("Failed to set CTRL-C handler: {}", error) - } - - if let Err(run_error) = justfile.run(&arguments, &config) { - if !config.quiet { - if config.color.stderr().active() { - eprintln!("{:#}", run_error); - } else { - eprintln!("{}", run_error); - } - } - - return Err(run_error.code().unwrap_or(EXIT_FAILURE)); - } - - Ok(()) + config.subcommand.run(&config, justfile) } diff --git a/src/subcommand.rs b/src/subcommand.rs index 4593bcb..ae9ed35 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -1,10 +1,224 @@ -#[derive(PartialEq)] +use crate::common::*; + +use unicode_width::UnicodeWidthStr; + +#[derive(PartialEq, Clone, Copy)] pub(crate) enum Subcommand<'a> { Dump, Edit, Evaluate, + Execute, List, - Run, Show { name: &'a str }, Summary, } + +impl<'a> Subcommand<'a> { + pub(crate) fn run(self, config: &Config, justfile: Justfile) -> Result<(), i32> { + use Subcommand::*; + + match self { + Dump => Self::dump(justfile), + Edit => { + eprintln!("Internal error: Subcommand::run unexpectadly invoked on Edit variant!"); + Err(EXIT_FAILURE) + } + Execute | Evaluate => Self::execute(config, justfile), + List => Self::list(config, justfile), + Show { name } => Self::show(justfile, name), + Summary => Self::summary(justfile), + } + } + + fn dump(justfile: Justfile) -> Result<(), i32> { + println!("{}", justfile); + Ok(()) + } + + pub(crate) fn edit(path: &Path) -> Result<(), i32> { + let editor = match env::var_os("EDITOR") { + None => { + eprintln!("Error getting EDITOR environment variable"); + return Err(EXIT_FAILURE); + } + Some(editor) => editor, + }; + + let error = Command::new(editor).arg(path).status(); + + match error { + Ok(status) => { + if status.success() { + Ok(()) + } else { + eprintln!("Editor failed: {}", status); + Err(status.code().unwrap_or(EXIT_FAILURE)) + } + } + Err(error) => { + eprintln!("Failed to invoke editor: {}", error); + Err(EXIT_FAILURE) + } + } + } + + fn list(config: &Config, justfile: Justfile) -> Result<(), i32> { + // Construct a target to alias map. + let mut recipe_aliases: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); + for alias in justfile.aliases.values() { + if alias.is_private() { + continue; + } + + if !recipe_aliases.contains_key(alias.target.lexeme()) { + recipe_aliases.insert(alias.target.lexeme(), vec![alias.name.lexeme()]); + } else { + let aliases = recipe_aliases.get_mut(alias.target.lexeme()).unwrap(); + aliases.push(alias.name.lexeme()); + } + } + + let mut line_widths: BTreeMap<&str, usize> = BTreeMap::new(); + + for (name, recipe) in &justfile.recipes { + if recipe.private { + continue; + } + + for name in iter::once(name).chain(recipe_aliases.get(name).unwrap_or(&Vec::new())) { + let mut line_width = UnicodeWidthStr::width(*name); + + for parameter in &recipe.parameters { + line_width += UnicodeWidthStr::width(format!(" {}", parameter).as_str()); + } + + if line_width <= 30 { + line_widths.insert(name, line_width); + } + } + } + + let max_line_width = cmp::min(line_widths.values().cloned().max().unwrap_or(0), 30); + + let doc_color = config.color.stdout().doc(); + println!("Available recipes:"); + + for (name, recipe) in &justfile.recipes { + if recipe.private { + continue; + } + + let alias_doc = format!("alias for `{}`", recipe.name); + + for (i, name) in iter::once(name) + .chain(recipe_aliases.get(name).unwrap_or(&Vec::new())) + .enumerate() + { + print!(" {}", name); + for parameter in &recipe.parameters { + if config.color.stdout().active() { + print!(" {:#}", parameter); + } else { + print!(" {}", parameter); + } + } + + // Declaring this outside of the nested loops will probably be more efficient, but + // it creates all sorts of lifetime issues with variables inside the loops. + // If this is inlined like the docs say, it shouldn't make any difference. + let print_doc = |doc| { + print!( + " {:padding$}{} {}", + "", + doc_color.paint("#"), + doc_color.paint(doc), + padding = max_line_width + .saturating_sub(line_widths.get(name).cloned().unwrap_or(max_line_width)) + ); + }; + + match (i, recipe.doc) { + (0, Some(doc)) => print_doc(doc), + (0, None) => (), + _ => print_doc(&alias_doc), + } + println!(); + } + } + + Ok(()) + } + + fn execute(config: &Config, justfile: Justfile) -> Result<(), i32> { + let arguments = if !config.arguments.is_empty() { + config.arguments.clone() + } else if let Some(recipe) = justfile.first() { + let min_arguments = recipe.min_arguments(); + if min_arguments > 0 { + die!( + "Recipe `{}` cannot be used as default recipe since it requires at least {} {}.", + recipe.name, + min_arguments, + Count("argument", min_arguments), + ); + } + vec![recipe.name()] + } else { + die!("Justfile contains no recipes."); + }; + + if let Err(error) = InterruptHandler::install() { + warn!("Failed to set CTRL-C handler: {}", error) + } + + if let Err(run_error) = justfile.run(&arguments, &config) { + if !config.quiet { + if config.color.stderr().active() { + eprintln!("{:#}", run_error); + } else { + eprintln!("{}", run_error); + } + } + + return Err(run_error.code().unwrap_or(EXIT_FAILURE)); + } + + Ok(()) + } + + fn show(justfile: Justfile, name: &str) -> Result<(), i32> { + if let Some(alias) = justfile.get_alias(name) { + let recipe = justfile.get_recipe(alias.target.lexeme()).unwrap(); + println!("{}", alias); + println!("{}", recipe); + return Ok(()); + } + if let Some(recipe) = justfile.get_recipe(name) { + println!("{}", recipe); + return Ok(()); + } else { + eprintln!("Justfile does not contain recipe `{}`.", name); + if let Some(suggestion) = justfile.suggest(name) { + eprintln!("Did you mean `{}`?", suggestion); + } + return Err(EXIT_FAILURE); + } + } + + fn summary(justfile: Justfile) -> Result<(), i32> { + if justfile.count() == 0 { + eprintln!("Justfile contains no recipes."); + } else { + let summary = justfile + .recipes + .iter() + .filter(|&(_, recipe)| !recipe.private) + .map(|(name, _)| name) + .cloned() + .collect::>() + .join(" "); + println!("{}", summary); + } + Ok(()) + } +}