From 48b25c14b154e29bb866fae3b8beeef5f4e9c2d4 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 31 Oct 2019 17:39:25 -0700 Subject: [PATCH] Use constants instead of literals in arg parser (#504) - Differentiate between `arg`s, which are flags and options, and `cmd`s, which are mutually exclusive subcommands - Replace string literals, like "EVALUATE", with constants, like `cmd::EVALUATE`, since they're slightly less error prone. - Remove `Config::evaluate`, and handle it like other subcommands --- src/common.rs | 2 +- src/config.rs | 153 ++++++++++++++++++++++++++++++++-------------- src/justfile.rs | 2 +- src/subcommand.rs | 7 ++- 4 files changed, 114 insertions(+), 50 deletions(-) diff --git a/src/common.rs b/src/common.rs index 1fa2da7..e9dea2e 100644 --- a/src/common.rs +++ b/src/common.rs @@ -11,7 +11,7 @@ pub(crate) use std::{ ops::{Range, RangeInclusive}, path::{Path, PathBuf}, process::{self, Command}, - str::Chars, + str::{self, Chars}, sync::{Mutex, MutexGuard}, usize, vec, }; diff --git a/src/config.rs b/src/config.rs index 17a6a04..391bc98 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,7 +7,6 @@ pub(crate) const DEFAULT_SHELL: &str = "sh"; pub(crate) struct Config<'a> { pub(crate) subcommand: Subcommand<'a>, pub(crate) dry_run: bool, - pub(crate) evaluate: bool, pub(crate) highlight: bool, pub(crate) overrides: BTreeMap<&'a str, &'a str>, pub(crate) quiet: bool, @@ -20,17 +19,29 @@ pub(crate) struct Config<'a> { pub(crate) invocation_directory: Result, } -mod arg { +mod cmd { pub(crate) const DUMP: &str = "DUMP"; - pub(crate) const COLOR: &str = "COLOR"; pub(crate) const EDIT: &str = "EDIT"; + pub(crate) const EVALUATE: &str = "EVALUATE"; pub(crate) const LIST: &str = "LIST"; pub(crate) const SHOW: &str = "SHOW"; pub(crate) const SUMMARY: &str = "SUMMARY"; +} + +mod arg { + pub(crate) const ARGUMENTS: &str = "ARGUMENTS"; + pub(crate) const COLOR: &str = "COLOR"; + pub(crate) const DRY_RUN: &str = "DRY-RUN"; + pub(crate) const HIGHLIGHT: &str = "HIGHLIGHT"; + pub(crate) const JUSTFILE: &str = "JUSTFILE"; + pub(crate) const QUIET: &str = "QUIET"; + pub(crate) const SET: &str = "SET"; + pub(crate) const SHELL: &str = "SHELL"; + pub(crate) const VERBOSE: &str = "VERBOSE"; pub(crate) const WORKING_DIRECTORY: &str = "WORKING-DIRECTORY"; - pub(crate) const COLOR_AUTO: &str = "auto"; pub(crate) const COLOR_ALWAYS: &str = "always"; + pub(crate) const COLOR_AUTO: &str = "auto"; pub(crate) const COLOR_NEVER: &str = "never"; pub(crate) const COLOR_VALUES: &[&str] = &[COLOR_AUTO, COLOR_ALWAYS, COLOR_NEVER]; } @@ -43,7 +54,7 @@ impl<'a> Config<'a> { .setting(AppSettings::ColoredHelp) .setting(AppSettings::TrailingVarArg) .arg( - Arg::with_name("ARGUMENTS") + Arg::with_name(arg::ARGUMENTS) .multiple(true) .help("The recipe(s) to run, defaults to the first recipe in the justfile"), ) @@ -56,54 +67,54 @@ impl<'a> Config<'a> { .help("Print colorful output"), ) .arg( - Arg::with_name("DRY-RUN") + Arg::with_name(arg::DRY_RUN) .long("dry-run") .help("Print what just would do without doing it") - .conflicts_with("QUIET"), + .conflicts_with(arg::QUIET), ) .arg( - Arg::with_name(arg::DUMP) + Arg::with_name(cmd::DUMP) .long("dump") .help("Print entire justfile"), ) .arg( - Arg::with_name(arg::EDIT) + Arg::with_name(cmd::EDIT) .short("e") .long("edit") .help("Open justfile with $EDITOR"), ) .arg( - Arg::with_name("EVALUATE") + Arg::with_name(cmd::EVALUATE) .long("evaluate") .help("Print evaluated variables"), ) .arg( - Arg::with_name("HIGHLIGHT") + Arg::with_name(arg::HIGHLIGHT) .long("highlight") .help("Highlight echoed recipe lines in bold"), ) .arg( - Arg::with_name("JUSTFILE") + Arg::with_name(arg::JUSTFILE) .short("f") .long("justfile") .takes_value(true) .help("Use as justfile."), ) .arg( - Arg::with_name(arg::LIST) + Arg::with_name(cmd::LIST) .short("l") .long("list") .help("List available recipes and their arguments"), ) .arg( - Arg::with_name("QUIET") + Arg::with_name(arg::QUIET) .short("q") .long("quiet") .help("Suppress all output") - .conflicts_with("DRY-RUN"), + .conflicts_with(arg::DRY_RUN), ) .arg( - Arg::with_name("SET") + Arg::with_name(arg::SET) .long("set") .takes_value(true) .number_of_values(2) @@ -112,14 +123,14 @@ impl<'a> Config<'a> { .help("Set to "), ) .arg( - Arg::with_name("SHELL") + Arg::with_name(arg::SHELL) .long("shell") .takes_value(true) .default_value(DEFAULT_SHELL) .help("Invoke to run recipes"), ) .arg( - Arg::with_name(arg::SHOW) + Arg::with_name(cmd::SHOW) .short("s") .long("show") .takes_value(true) @@ -127,12 +138,12 @@ impl<'a> Config<'a> { .help("Show information about "), ) .arg( - Arg::with_name(arg::SUMMARY) + Arg::with_name(cmd::SUMMARY) .long("summary") .help("List names of available recipes"), ) .arg( - Arg::with_name("VERBOSE") + Arg::with_name(arg::VERBOSE) .short("v") .long("verbose") .multiple(true) @@ -144,16 +155,16 @@ impl<'a> Config<'a> { .long("working-directory") .takes_value(true) .help("Use as working directory. --justfile must also be set") - .requires("JUSTFILE"), + .requires(arg::JUSTFILE), ) .group(ArgGroup::with_name("EARLY-EXIT").args(&[ - arg::DUMP, - arg::EDIT, - arg::LIST, - arg::SHOW, - arg::SUMMARY, - "ARGUMENTS", - "EVALUATE", + arg::ARGUMENTS, + cmd::DUMP, + cmd::EDIT, + cmd::EVALUATE, + cmd::LIST, + cmd::SHOW, + cmd::SUMMARY, ])); if cfg!(feature = "help4help2man") { @@ -189,7 +200,7 @@ impl<'a> Config<'a> { let invocation_directory = env::current_dir().map_err(|e| format!("Error getting current directory: {}", e)); - let verbosity = Verbosity::from_flag_occurrences(matches.occurrences_of("VERBOSE")); + let verbosity = Verbosity::from_flag_occurrences(matches.occurrences_of(arg::VERBOSE)); let color = Self::color_from_value( matches @@ -197,10 +208,10 @@ impl<'a> Config<'a> { .expect("`--color` had no value"), )?; - let set_count = matches.occurrences_of("SET"); + let set_count = matches.occurrences_of(arg::SET); let mut overrides = BTreeMap::new(); if set_count > 0 { - let mut values = matches.values_of("SET").unwrap(); + let mut values = matches.values_of(arg::SET).unwrap(); for _ in 0..set_count { overrides.insert(values.next().unwrap(), values.next().unwrap()); } @@ -211,7 +222,7 @@ impl<'a> Config<'a> { } let raw_arguments: Vec<&str> = matches - .values_of("ARGUMENTS") + .values_of(arg::ARGUMENTS) .map(Iterator::collect) .unwrap_or_default(); @@ -258,28 +269,29 @@ impl<'a> Config<'a> { }) .collect::>(); - let subcommand = if matches.is_present(arg::EDIT) { + let subcommand = if matches.is_present(cmd::EDIT) { Subcommand::Edit - } else if matches.is_present(arg::SUMMARY) { + } else if matches.is_present(cmd::SUMMARY) { Subcommand::Summary - } else if matches.is_present(arg::DUMP) { + } else if matches.is_present(cmd::DUMP) { Subcommand::Dump - } else if matches.is_present(arg::LIST) { + } else if matches.is_present(cmd::LIST) { Subcommand::List - } else if let Some(name) = matches.value_of(arg::SHOW) { + } else if matches.is_present(cmd::EVALUATE) { + Subcommand::Evaluate + } else if let Some(name) = matches.value_of(cmd::SHOW) { Subcommand::Show { name } } else { Subcommand::Run }; Ok(Config { - dry_run: matches.is_present("DRY-RUN"), - evaluate: matches.is_present("EVALUATE"), - highlight: matches.is_present("HIGHLIGHT"), - quiet: matches.is_present("QUIET"), - shell: matches.value_of("SHELL").unwrap(), - justfile: matches.value_of("JUSTFILE").map(Path::new), - working_directory: matches.value_of("WORKING-DIRECTORY").map(Path::new), + dry_run: matches.is_present(arg::DRY_RUN), + highlight: matches.is_present(arg::HIGHLIGHT), + quiet: matches.is_present(arg::QUIET), + shell: matches.value_of(arg::SHELL).unwrap(), + justfile: matches.value_of(arg::JUSTFILE).map(Path::new), + working_directory: matches.value_of(arg::WORKING_DIRECTORY).map(Path::new), invocation_directory, subcommand, verbosity, @@ -295,7 +307,6 @@ impl<'a> Default for Config<'a> { Config { subcommand: Subcommand::Run, dry_run: false, - evaluate: false, highlight: false, overrides: empty(), arguments: empty(), @@ -310,3 +321,55 @@ impl<'a> Default for Config<'a> { } } } + +#[cfg(test)] +mod tests { + use super::*; + + use pretty_assertions::assert_eq; + + // This test guards against unintended changes to the argument parser. We should have + // proper tests for all the flags, but this will do for now. + #[test] + fn help() { + const EXPECTED_HELP: &str = "just v0.4.4 +Casey Rodarmor +🤖 Just a command runner - https://github.com/casey/just + +USAGE: + just [FLAGS] [OPTIONS] [--] [ARGUMENTS]... + +FLAGS: + --dry-run Print what just would do without doing it + --dump Print entire justfile + -e, --edit Open justfile with $EDITOR + --evaluate Print evaluated variables + --highlight Highlight echoed recipe lines in bold + -l, --list List available recipes and their arguments + -q, --quiet Suppress all output + --summary List names of available recipes + -v, --verbose Use verbose output + +OPTIONS: + --color + Print colorful output [default: auto] [possible values: auto, always, never] + + -f, --justfile Use as justfile. + --set Set to + --shell Invoke to run recipes [default: sh] + -s, --show Show information about + -d, --working-directory + Use as working directory. --justfile must also be set + + +ARGS: + ... The recipe(s) to run, defaults to the first recipe in the justfile"; + + let app = Config::app().setting(AppSettings::ColorNever); + let mut buffer = Vec::new(); + app.write_help(&mut buffer).unwrap(); + let help = str::from_utf8(&buffer).unwrap(); + + assert_eq!(help, EXPECTED_HELP); + } +} diff --git a/src/justfile.rs b/src/justfile.rs index 0bb404b..cdb59b2 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -69,7 +69,7 @@ impl<'a> Justfile<'a> { config.dry_run, )?; - if config.evaluate { + if config.subcommand == Subcommand::Evaluate { let mut width = 0; for name in scope.keys() { width = cmp::max(name.len(), width); diff --git a/src/subcommand.rs b/src/subcommand.rs index 33f9655..4593bcb 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -1,9 +1,10 @@ #[derive(PartialEq)] pub(crate) enum Subcommand<'a> { - Edit, - Summary, Dump, + Edit, + Evaluate, List, - Show { name: &'a str }, Run, + Show { name: &'a str }, + Summary, }