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

@@ -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,
 };

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<PathBuf, String>,
 }
 
-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 <JUSTFILE> 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 <VARIABLE> to <VALUE>"),
       )
       .arg(
-        Arg::with_name("SHELL")
+        Arg::with_name(arg::SHELL)
           .long("shell")
           .takes_value(true)
           .default_value(DEFAULT_SHELL)
           .help("Invoke <SHELL> 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 <RECIPE>"),
       )
       .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 <WORKING-DIRECTORY> as working directory. --justfile must also be set")
-          .requires("JUSTFILE"),
+          .requires(arg::JUSTFILE),
       )
       .group(ArgGroup::with_name("EARLY-EXIT").args(&[
-        arg::DUMP,
+        arg::ARGUMENTS,
-        arg::EDIT,
+        cmd::DUMP,
-        arg::LIST,
+        cmd::EDIT,
-        arg::SHOW,
+        cmd::EVALUATE,
-        arg::SUMMARY,
+        cmd::LIST,
-        "ARGUMENTS",
+        cmd::SHOW,
-        "EVALUATE",
+        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::<Vec<&str>>();
 
-    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"),
+      dry_run: matches.is_present(arg::DRY_RUN),
-      evaluate: matches.is_present("EVALUATE"),
+      highlight: matches.is_present(arg::HIGHLIGHT),
-      highlight: matches.is_present("HIGHLIGHT"),
+      quiet: matches.is_present(arg::QUIET),
-      quiet: matches.is_present("QUIET"),
+      shell: matches.value_of(arg::SHELL).unwrap(),
-      shell: matches.value_of("SHELL").unwrap(),
+      justfile: matches.value_of(arg::JUSTFILE).map(Path::new),
-      justfile: matches.value_of("JUSTFILE").map(Path::new),
+      working_directory: matches.value_of(arg::WORKING_DIRECTORY).map(Path::new),
-      working_directory: matches.value_of("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 <casey@rodarmor.com>
+🤖 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 <COLOR>
+            Print colorful output [default: auto]  [possible values: auto, always, never]
+
+    -f, --justfile <JUSTFILE>                      Use <JUSTFILE> as justfile.
+        --set <VARIABLE> <VALUE>                   Set <VARIABLE> to <VALUE>
+        --shell <SHELL>                            Invoke <SHELL> to run recipes [default: sh]
+    -s, --show <RECIPE>                            Show information about <RECIPE>
+    -d, --working-directory <WORKING-DIRECTORY>
+            Use <WORKING-DIRECTORY> as working directory. --justfile must also be set
+
+
+ARGS:
+    <ARGUMENTS>...    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);
+  }
+}

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);

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,
 }