Skip to content

Commit

Permalink
Gargantuan refactor
Browse files Browse the repository at this point in the history
- Instead of changing the current directory with `env::set_current_dir`
  to be implicitly inherited by subprocesses, we now use
  `Command::current_dir` to set it explicitly. This feels much better,
  since we aren't dependent on the implicit state of the process's
  current directory.

- Subcommand execution is much improved.

- Added a ton of tests for config parsing, config execution, working
  dir, and search dir.

- Error messages are improved. Many more will be colored.

- The Config is now onwed, instead of borrowing from the arguments and
  the `clap::ArgMatches` object. This is a huge ergonomic improvement,
  especially in tests, and I don't think anyone will notice.

- `--edit` now uses `$VISUAL`, `$EDITOR`, or `vim`, in that order,
  matching git, which I think is what most people will expect.

- Added a cute `tmptree!{}` macro, for creating temporary directories
  populated with directories and files for tests.

- Admitted that grammer is LL(k) and I don't know what `k` is.
  • Loading branch information
casey committed Nov 10, 2019
1 parent 8279361 commit dc6c264
Show file tree
Hide file tree
Showing 42 changed files with 1,930 additions and 897 deletions.
76 changes: 76 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ itertools = "0.8"
lazy_static = "1"
libc = "0.2"
log = "0.4.4"
snafu = "0.6"
target = "1"
tempfile = "3"
unicode-width = "0.1"
Expand All @@ -37,6 +38,7 @@ features = ["termination"]
[dev-dependencies]
executable-path = "1"
pretty_assertions = "0.6"
which = "3"

# Until github.com/rust-lang/cargo/pull/7333 makes it into stable,
# this version-less dev-dependency will interfere with publishing
Expand Down
5 changes: 2 additions & 3 deletions GRAMMAR.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@ justfile grammar
================

Justfiles are processed by a mildly context-sensitive tokenizer
and a recursive descent parser. The grammar is mostly LL(1),
although an extra token of lookahead is used to distinguish between
export assignments and recipes with parameters.
and a recursive descent parser. The grammar is LL(k), for an
unknown but hopefully reasonable value of k.

tokens
------
Expand Down
2 changes: 1 addition & 1 deletion justfile
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ check:
cargo check

watch +COMMAND='test':
cargo watch --clear --exec "{{COMMAND}}"
cargo watch --clear --exec build --exec "{{COMMAND}}"

man:
cargo build --features help4help2man
Expand Down
58 changes: 26 additions & 32 deletions src/assignment_evaluator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,36 +2,27 @@ use crate::common::*;

pub(crate) struct AssignmentEvaluator<'a: 'b, 'b> {
pub(crate) assignments: &'b BTreeMap<&'a str, Assignment<'a>>,
pub(crate) invocation_directory: &'b Result<PathBuf, String>,
pub(crate) config: &'a Config,
pub(crate) dotenv: &'b BTreeMap<String, String>,
pub(crate) dry_run: bool,
pub(crate) evaluated: BTreeMap<&'a str, (bool, String)>,
pub(crate) overrides: &'b BTreeMap<&'b str, &'b str>,
pub(crate) quiet: bool,
pub(crate) scope: &'b BTreeMap<&'a str, (bool, String)>,
pub(crate) shell: &'b str,
pub(crate) working_directory: &'b Path,
}

impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
pub(crate) fn evaluate_assignments(
assignments: &BTreeMap<&'a str, Assignment<'a>>,
invocation_directory: &Result<PathBuf, String>,
config: &'a Config,
working_directory: &'b Path,
dotenv: &'b BTreeMap<String, String>,
overrides: &BTreeMap<&str, &str>,
quiet: bool,
shell: &'a str,
dry_run: bool,
assignments: &BTreeMap<&'a str, Assignment<'a>>,
) -> RunResult<'a, BTreeMap<&'a str, (bool, String)>> {
let mut evaluator = AssignmentEvaluator {
evaluated: empty(),
scope: &empty(),
config,
assignments,
invocation_directory,
working_directory,
dotenv,
dry_run,
overrides,
quiet,
shell,
};

for name in assignments.keys() {
Expand Down Expand Up @@ -64,7 +55,7 @@ impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
}

if let Some(assignment) = self.assignments.get(name) {
if let Some(value) = self.overrides.get(name) {
if let Some(value) = self.config.overrides.get(name) {
self
.evaluated
.insert(name, (assignment.export, value.to_string()));
Expand Down Expand Up @@ -113,14 +104,15 @@ impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
.map(|argument| self.evaluate_expression(argument, arguments))
.collect::<Result<Vec<String>, RuntimeError>>()?;
let context = FunctionContext {
invocation_directory: &self.invocation_directory,
invocation_directory: &self.config.invocation_directory,
working_directory: &self.working_directory,
dotenv: self.dotenv,
};
Function::evaluate(*function, &context, &call_arguments)
}
Expression::StringLiteral { string_literal } => Ok(string_literal.cooked.to_string()),
Expression::Backtick { contents, token } => {
if self.dry_run {
if self.config.dry_run {
Ok(format!("`{}`", contents))
} else {
Ok(self.run_backtick(self.dotenv, contents, token)?)
Expand All @@ -139,23 +131,25 @@ impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
raw: &str,
token: &Token<'a>,
) -> RunResult<'a, String> {
let mut cmd = Command::new(self.shell);
let mut cmd = Command::new(&self.config.shell);

cmd.current_dir(self.working_directory);

cmd.arg("-cu").arg(raw);

cmd.export_environment_variables(self.scope, dotenv)?;

cmd.stdin(process::Stdio::inherit());

cmd.stderr(if self.quiet {
cmd.stderr(if self.config.quiet {
process::Stdio::null()
} else {
process::Stdio::inherit()
});

InterruptHandler::guard(|| {
output(cmd).map_err(|output_error| RuntimeError::Backtick {
token: token.clone(),
token: *token,
output_error,
})
})
Expand All @@ -165,14 +159,14 @@ impl<'a, 'b> AssignmentEvaluator<'a, 'b> {
#[cfg(test)]
mod tests {
use super::*;
use crate::testing::compile;
use crate::testing::{compile, config};

#[test]
fn backtick_code() {
match compile("a:\n echo {{`f() { return 100; }; f`}}")
.run(&["a"], &Default::default())
.unwrap_err()
{
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),
Expand All @@ -193,12 +187,12 @@ b = `echo $exported_variable`
recipe:
echo {{b}}
"#;
let config = Config {
quiet: true,
..Default::default()
};

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

match justfile.run(&config, &dir).unwrap_err() {
RuntimeError::Backtick {
token,
output_error: OutputError::Code(_),
Expand Down
2 changes: 1 addition & 1 deletion src/assignment_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ impl<'a: 'b, 'b> AssignmentResolver<'a, 'b> {
let token = self.assignments[variable].name.token();
self.stack.push(variable);
return Err(token.error(CircularVariableDependency {
variable: variable,
variable,
circle: self.stack.clone(),
}));
} else if self.assignments.contains_key(variable) {
Expand Down
4 changes: 2 additions & 2 deletions src/color.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ansi_term::Color::*;
use ansi_term::{ANSIGenericString, Prefix, Style, Suffix};
use atty::Stream;

#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug, PartialEq)]
pub(crate) struct Color {
use_color: UseColor,
atty: bool,
Expand Down Expand Up @@ -128,7 +128,7 @@ impl Color {
impl Default for Color {
fn default() -> Color {
Color {
use_color: UseColor::Never,
use_color: UseColor::Auto,
atty: false,
style: Style::new(),
}
Expand Down
Loading

0 comments on commit dc6c264

Please sign in to comment.