Skip to content

Commit

Permalink
Reform positional argument parsing (#523)
Browse files Browse the repository at this point in the history
This diff makes positional argument parsing much cleaner, along with
adding a bunch of tests. Just's positional argument parsing is rather,
complex, so hopefully this reform allows it to both be correct and stay
correct.

User-visible changes:

- `just ..` is now accepted, with the same effect as `just ../`

- `just .` is also accepted, with the same effect as `just`

- It is now an error to pass arguments or overrides to subcommands
  that do not accept them, namely `--dump`, `--edit`, `--list`,
  `--show`, and `--summary`. It is also an error to pass arguments to
  `--evaluate`, although `--evaluate` does of course still accept
  overrides.

  (This is a breaking change, but hopefully worth it, as it will allow us
  to add arguments to subcommands which did not previously take
  them, if we so desire.)

- Subcommands which do not accept arguments may now accept a
  single search-directory argument, so `just --list ../` and
  `just --dump foo/` are now accepted, with the former starting the
  search for the justfile to list in the parent directory, and the latter
  starting the search for the justfile to dump in `foo`.
  • Loading branch information
casey authored Nov 11, 2019
1 parent aefdcea commit 177516b
Show file tree
Hide file tree
Showing 15 changed files with 956 additions and 451 deletions.
70 changes: 33 additions & 37 deletions src/assignment_evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pub(crate) struct AssignmentEvaluator<'a: 'b, 'b> {
pub(crate) evaluated: BTreeMap<&'a str, (bool, String)>,
pub(crate) scope: &'b BTreeMap<&'a str, (bool, String)>,
pub(crate) working_directory: &'b Path,
pub(crate) overrides: &'b BTreeMap<String, String>,
}

impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
Expand All @@ -15,10 +16,12 @@ impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
working_directory: &'b Path,
dotenv: &'b BTreeMap<String, String>,
assignments: &BTreeMap<&'a str, Assignment<'a>>,
overrides: &BTreeMap<String, String>,
) -> RunResult<'a, BTreeMap<&'a str, (bool, String)>> {
let mut evaluator = AssignmentEvaluator {
evaluated: empty(),
scope: &empty(),
overrides,
config,
assignments,
working_directory,
Expand Down Expand Up @@ -55,7 +58,7 @@ impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
}

if let Some(assignment) = self.assignments.get(name) {
if let Some(value) = self.config.overrides.get(name) {
if let Some(value) = self.overrides.get(name) {
self
.evaluated
.insert(name, (assignment.export, value.to_string()));
Expand Down Expand Up @@ -159,47 +162,40 @@ impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
#[cfg(test)]
mod tests {
use super::*;
use crate::testing::{compile, config};

#[test]
fn backtick_code() {
let justfile = compile("a:\n echo {{`f() { return 100; }; f`}}");
let config = config(&["a"]);
let dir = env::current_dir().unwrap();
match justfile.run(&config, &dir).unwrap_err() {
RuntimeError::Backtick {
token,
output_error: OutputError::Code(code),
} => {
assert_eq!(code, 100);
assert_eq!(token.lexeme(), "`f() { return 100; }; f`");
}
other => panic!("expected a code run error, but got: {}", other),

run_error! {
name: backtick_code,
src: "
a:
echo {{`f() { return 100; }; f`}}
",
args: ["a"],
error: RuntimeError::Backtick {
token,
output_error: OutputError::Code(code),
},
check: {
assert_eq!(code, 100);
assert_eq!(token.lexeme(), "`f() { return 100; }; f`");
}
}

#[test]
fn export_assignment_backtick() {
let text = r#"
export exported_variable = "A"
b = `echo $exported_variable`
recipe:
echo {{b}}
"#;

let justfile = compile(text);
let config = config(&["--quiet", "recipe"]);
let dir = env::current_dir().unwrap();

match justfile.run(&config, &dir).unwrap_err() {
RuntimeError::Backtick {
run_error! {
name: export_assignment_backtick,
src: r#"
export exported_variable = "A"
b = `echo $exported_variable`
recipe:
echo {{b}}
"#,
args: ["--quiet", "recipe"],
error: RuntimeError::Backtick {
token,
output_error: OutputError::Code(_),
} => {
assert_eq!(token.lexeme(), "`echo $exported_variable`");
}
other => panic!("expected a backtick code errror, but got: {}", other),
},
check: {
assert_eq!(token.lexeme(), "`echo $exported_variable`");
}
}
}
7 changes: 4 additions & 3 deletions src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,10 @@ pub(crate) use crate::{
function_context::FunctionContext, functions::Functions, interrupt_guard::InterruptGuard,
interrupt_handler::InterruptHandler, item::Item, justfile::Justfile, lexer::Lexer, line::Line,
list::List, load_error::LoadError, module::Module, name::Name, output_error::OutputError,
parameter::Parameter, parser::Parser, platform::Platform, position::Position, recipe::Recipe,
recipe_context::RecipeContext, recipe_resolver::RecipeResolver, runtime_error::RuntimeError,
search::Search, search_config::SearchConfig, search_error::SearchError, shebang::Shebang,
parameter::Parameter, parser::Parser, platform::Platform, position::Position,
positional::Positional, recipe::Recipe, recipe_context::RecipeContext,
recipe_resolver::RecipeResolver, runtime_error::RuntimeError, search::Search,
search_config::SearchConfig, search_error::SearchError, shebang::Shebang,
show_whitespace::ShowWhitespace, state::State, string_literal::StringLiteral,
subcommand::Subcommand, table::Table, token::Token, token_kind::TokenKind, use_color::UseColor,
variables::Variables, verbosity::Verbosity, warning::Warning,
Expand Down
Loading

0 comments on commit 177516b

Please sign in to comment.