From 3c1098ac94cc5764570538fee388546aeeb42e20 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sun, 1 Jan 2023 16:52:09 -0800 Subject: [PATCH 01/28] Support includes --- src/error.rs | 22 ++++++ src/loader.rs | 187 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 208 insertions(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 15e42b94de..8c557a401e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -87,6 +87,14 @@ pub(crate) enum Error<'src> { InitExists { justfile: PathBuf, }, + Include { + path: PathBuf, + io_error: io::Error, + }, + IncludeRecursive { + cur_path: PathBuf, + recursively_included_path: PathBuf, + }, Internal { message: String, }, @@ -485,6 +493,20 @@ impl<'src> ColorDisplay for Error<'src> { InitExists { justfile } => { write!(f, "Justfile `{}` already exists", justfile.display())?; } + Include { path, io_error } => { + write!( + f, + "Failed to read included justfile at `{}`: {}", + path.display(), + io_error + )?; + }, + IncludeRecursive { cur_path, recursively_included_path } => { + write!( + f, + "Justfile at {} tries to recursively include {}", cur_path.display(), recursively_included_path.display() + )?; + }, Internal { message } => { write!( f, diff --git a/src/loader.rs b/src/loader.rs index 95fa7c34dd..ab74031522 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -1,4 +1,9 @@ use super::*; +use std::collections::HashSet; + +// This regex defines the syntax for a Justfile include statement as `!include ` +// occurring at the start of a line, and as the only contents of that line +const INCLUDE_REGEX: &str = "^!include\\s+(.+)$"; pub(crate) struct Loader { arena: Arena, @@ -12,10 +17,190 @@ impl Loader { } pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { + let src = self.perform_load(path, HashSet::new())?; + Ok(self.arena.alloc(src)) + } + + fn perform_load(&self, path: &Path, seen_paths: HashSet) -> RunResult { let src = fs::read_to_string(path).map_err(|io_error| Error::Load { path: path.to_owned(), io_error, })?; - Ok(self.arena.alloc(src)) + + self.process_includes(src, path, seen_paths) + } + + /// Given the original contents of a Justfile (with include directives), load all the included + /// paths to produce one String with the contents of all the files concatenate!d. Note that + /// this does not do any parsing yet (i.e. nothing stops a justfile from including a file + /// that is not a valid justfile), and that (currently) line numbers in error messages + /// will reference lines in this concatenated String rather than probably-more-useful + /// information about the original file an error came from. + fn process_includes( + &self, + original: String, + current_justfile_path: &Path, + seen_paths: HashSet, + ) -> RunResult { + let has_final_newline = original.ends_with('\n'); + + let include_regexp = Regex::new(INCLUDE_REGEX).unwrap(); + + //NOTE this string-processing code can be made a bit cleaner once the Rust std lib Intersperse + //API is stabilized (https://doc.rust-lang.org/std/iter/struct.Intersperse.html) + + let mut buf = String::new(); + let mut lines = original.lines().peekable(); + + while let Some(line) = lines.next() { + match include_regexp.captures(line) { + Some(captures) => { + let path_match = captures.get(1).unwrap(); + let include_path = Path::new(path_match.as_str()); + let included_contents = + self.process_single_include(current_justfile_path, include_path, &seen_paths)?; + buf.push_str(&included_contents); + } + None => { + buf.push_str(line); + } + }; + if lines.peek().is_some() { + buf.push('\n'); + } + } + if has_final_newline { + buf.push('\n'); + } + + Ok(buf) + } + + fn process_single_include( + &self, + cur_path: &Path, + include_path: &Path, + seen_paths: &HashSet, + ) -> RunResult { + let canonical_path = if include_path.is_relative() { + let current_dir = cur_path.parent().ok_or(Error::Internal { + message: format!( + "Justfile path `{}` has no parent directory", + include_path.display() + ), + })?; + + current_dir + .join(include_path) + .canonicalize() + .map_err(|io_error| Error::Include { + path: include_path.to_owned(), + io_error, + })? + } else { + include_path.to_owned() + }; + + if seen_paths.contains(&canonical_path) { + return Err(Error::IncludeRecursive { + cur_path: cur_path.to_owned(), + recursively_included_path: canonical_path, + }); + } + + let mut seen_paths = seen_paths.clone(); + seen_paths.insert(cur_path.to_owned()); + + self.perform_load(&canonical_path, seen_paths) + } +} + +#[cfg(test)] +mod tests { + use super::{Error, Loader}; + use temptree::temptree; + + #[test] + fn include_justfile() { + let justfile_a = r#" +# A comment at the top of the file +!include ./justfile_b + +some_recipe: recipe_b + echo "some recipe" +"#; + + let justfile_b = r#"!include ./subdir/justfile_c + +recipe_b: recipe_c + echo "recipe b" +"#; + + let justfile_c = r#"recipe_c: + echo "recipe c" +"#; + + let tmp = temptree! { + justfile: justfile_a, + justfile_b: justfile_b, + subdir: { + justfile_c: justfile_c + } + }; + + let full_concatenated_output = r#" +# A comment at the top of the file +recipe_c: + echo "recipe c" + + +recipe_b: recipe_c + echo "recipe b" + + +some_recipe: recipe_b + echo "some recipe" +"#; + + let loader = Loader::new(); + + let justfile_a_path = tmp.path().join("justfile"); + let loader_output = loader.load(&justfile_a_path).unwrap(); + + assert_eq!(loader_output, full_concatenated_output); + } + + #[test] + fn recursive_includes_fail() { + let justfile_a = r#" +# A comment at the top of the file +!include ./subdir/justfile_b + +some_recipe: recipe_b + echo "some recipe" +"#; + + let justfile_b = r#" +!include ../justfile + +recipe_b: + echo "recipe b" +"#; + let tmp = temptree! { + justfile: justfile_a, + subdir: { + justfile_b: justfile_b + } + }; + + let loader = Loader::new(); + + let justfile_a_path = tmp.path().join("justfile"); + let loader_output = loader.load(&justfile_a_path).unwrap_err(); + + assert_matches!(loader_output, Error::IncludeRecursive { cur_path, recursively_included_path } + if cur_path == tmp.path().join("subdir").join("justfile_b") && + recursively_included_path == tmp.path().join("justfile") + ); } } From f7eb810cbf0165c7091cfdc79cf2d84608a1d4cb Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sun, 1 Jan 2023 22:59:51 -0800 Subject: [PATCH 02/28] Debug commit --- src/loader.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/loader.rs b/src/loader.rs index ab74031522..343b9e48ae 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -22,6 +22,11 @@ impl Loader { } fn perform_load(&self, path: &Path, seen_paths: HashSet) -> RunResult { + println!( + "Perform load on {}, seen_paths {:?}", + path.display(), + seen_paths + ); let src = fs::read_to_string(path).map_err(|io_error| Error::Load { path: path.to_owned(), io_error, From 797b61f039a0b490dd8674f373a8d0f42fc82be1 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sun, 1 Jan 2023 23:09:10 -0800 Subject: [PATCH 03/28] Revert "Debug commit" This reverts commit bb0120cad4feeef4e497244fce4fa8e639a1590a. --- src/loader.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/loader.rs b/src/loader.rs index 343b9e48ae..ab74031522 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -22,11 +22,6 @@ impl Loader { } fn perform_load(&self, path: &Path, seen_paths: HashSet) -> RunResult { - println!( - "Perform load on {}, seen_paths {:?}", - path.display(), - seen_paths - ); let src = fs::read_to_string(path).map_err(|io_error| Error::Load { path: path.to_owned(), io_error, From 751fcbda587083f45d6a266fcbeb21f15dbef696 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sun, 1 Jan 2023 23:27:44 -0800 Subject: [PATCH 04/28] Canonicalize in all cases --- src/loader.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/loader.rs b/src/loader.rs index ab74031522..ea0cce0ec9 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -89,18 +89,18 @@ impl Loader { include_path.display() ), })?; - - current_dir - .join(include_path) - .canonicalize() - .map_err(|io_error| Error::Include { - path: include_path.to_owned(), - io_error, - })? + current_dir.join(include_path) } else { include_path.to_owned() }; + let canonical_path = canonical_path + .canonicalize() + .map_err(|io_error| Error::Include { + path: include_path.to_owned(), + io_error, + })?; + if seen_paths.contains(&canonical_path) { return Err(Error::IncludeRecursive { cur_path: cur_path.to_owned(), @@ -109,7 +109,12 @@ impl Loader { } let mut seen_paths = seen_paths.clone(); - seen_paths.insert(cur_path.to_owned()); + + let cur_path_canonical = cur_path.canonicalize().map_err(|io_error| Error::Include { + path: include_path.to_owned(), + io_error, + })?; + seen_paths.insert(cur_path_canonical); self.perform_load(&canonical_path, seen_paths) } @@ -199,8 +204,8 @@ recipe_b: let loader_output = loader.load(&justfile_a_path).unwrap_err(); assert_matches!(loader_output, Error::IncludeRecursive { cur_path, recursively_included_path } - if cur_path == tmp.path().join("subdir").join("justfile_b") && - recursively_included_path == tmp.path().join("justfile") + if cur_path == tmp.path().join("subdir").join("justfile_b").canonicalize().unwrap() && + recursively_included_path == tmp.path().join("justfile").canonicalize().unwrap() ); } } From f42a7c0c8e1e5960e692bf8277dbb4ea4eae0980 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Mon, 2 Jan 2023 00:15:25 -0800 Subject: [PATCH 05/28] Invalid includes --- src/error.rs | 10 ++++++++ src/loader.rs | 63 +++++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 7 deletions(-) diff --git a/src/error.rs b/src/error.rs index 8c557a401e..7acb0c1edd 100644 --- a/src/error.rs +++ b/src/error.rs @@ -98,6 +98,9 @@ pub(crate) enum Error<'src> { Internal { message: String, }, + InvalidInclude { + line_number: usize, + }, Io { recipe: &'src str, io_error: io::Error, @@ -515,6 +518,13 @@ impl<'src> ColorDisplay for Error<'src> { message )?; } + InvalidInclude { line_number } => { + write!( + f, + "!include statement at line {} occurs after the first non-blank, non-comment line", + line_number + )?; + } Io { recipe, io_error } => { match io_error.kind() { io::ErrorKind::NotFound => write!( diff --git a/src/loader.rs b/src/loader.rs index ea0cce0ec9..2def3714c1 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -31,11 +31,17 @@ impl Loader { } /// Given the original contents of a Justfile (with include directives), load all the included - /// paths to produce one String with the contents of all the files concatenate!d. Note that - /// this does not do any parsing yet (i.e. nothing stops a justfile from including a file - /// that is not a valid justfile), and that (currently) line numbers in error messages - /// will reference lines in this concatenated String rather than probably-more-useful - /// information about the original file an error came from. + /// paths to produce one String with the contents of all the files concatenated. + /// + /// Note that this does not do any parsing yet (i.e. nothing stops a justfile from including a + /// file that is not a valid justfile), and that (currently) line numbers in error messages will + /// reference lines in this concatenated String rather than probably-more-useful information + /// about the original file an error came from. + /// + /// !include directives are allowed to appear in a justfile only before the first non-blank, + /// non-comment line. This is because this processing step occurs before parsing, and we don't + /// want to accidentally process the string "!include" when it happens to occur within a string + /// or in another post-parsing syntactic context where it shouldn't be. fn process_includes( &self, original: String, @@ -50,10 +56,17 @@ impl Loader { //API is stabilized (https://doc.rust-lang.org/std/iter/struct.Intersperse.html) let mut buf = String::new(); - let mut lines = original.lines().peekable(); + let mut lines = original.lines().enumerate().peekable(); - while let Some(line) = lines.next() { + let mut seen_first_contentful_line = false; + + while let Some((line_num, line)) = lines.next() { match include_regexp.captures(line) { + Some(_) if seen_first_contentful_line => { + return Err(Error::InvalidInclude { + line_number: line_num + 1, + }); + } Some(captures) => { let path_match = captures.get(1).unwrap(); let include_path = Path::new(path_match.as_str()); @@ -62,6 +75,9 @@ impl Loader { buf.push_str(&included_contents); } None => { + if !(line.is_empty() || line.starts_with('#')) { + seen_first_contentful_line = true; + } buf.push_str(line); } }; @@ -208,4 +224,37 @@ recipe_b: recursively_included_path == tmp.path().join("justfile").canonicalize().unwrap() ); } + + #[test] + fn invalid_includes() { + let justfile_a = r#" +# A comment at the top of the file +!include ./subdir/justfile_b + +some_recipe: recipe_b + echo "some recipe" +"#; + + let justfile_b = r#" + +alias b := recipe_b +!include ../justfile + +recipe_b: + echo "recipe b" +"#; + let tmp = temptree! { + justfile: justfile_a, + subdir: { + justfile_b: justfile_b + } + }; + + let loader = Loader::new(); + + let justfile_a_path = tmp.path().join("justfile"); + let loader_output = loader.load(&justfile_a_path).unwrap_err(); + + assert_matches!(loader_output, Error::InvalidInclude { line_number } if line_number == 4); + } } From 01dbf227f29988e1a14e23c4aa92bf9a47193b3a Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Mon, 2 Jan 2023 01:03:16 -0800 Subject: [PATCH 06/28] Use lexiclean instead of canonicalize --- src/error.rs | 12 ------------ src/loader.rs | 20 +++++--------------- 2 files changed, 5 insertions(+), 27 deletions(-) diff --git a/src/error.rs b/src/error.rs index 7acb0c1edd..6a99818f70 100644 --- a/src/error.rs +++ b/src/error.rs @@ -87,10 +87,6 @@ pub(crate) enum Error<'src> { InitExists { justfile: PathBuf, }, - Include { - path: PathBuf, - io_error: io::Error, - }, IncludeRecursive { cur_path: PathBuf, recursively_included_path: PathBuf, @@ -496,14 +492,6 @@ impl<'src> ColorDisplay for Error<'src> { InitExists { justfile } => { write!(f, "Justfile `{}` already exists", justfile.display())?; } - Include { path, io_error } => { - write!( - f, - "Failed to read included justfile at `{}`: {}", - path.display(), - io_error - )?; - }, IncludeRecursive { cur_path, recursively_included_path } => { write!( f, diff --git a/src/loader.rs b/src/loader.rs index 2def3714c1..f6d86cc001 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -110,12 +110,7 @@ impl Loader { include_path.to_owned() }; - let canonical_path = canonical_path - .canonicalize() - .map_err(|io_error| Error::Include { - path: include_path.to_owned(), - io_error, - })?; + let canonical_path = canonical_path.lexiclean(); if seen_paths.contains(&canonical_path) { return Err(Error::IncludeRecursive { @@ -125,12 +120,7 @@ impl Loader { } let mut seen_paths = seen_paths.clone(); - - let cur_path_canonical = cur_path.canonicalize().map_err(|io_error| Error::Include { - path: include_path.to_owned(), - io_error, - })?; - seen_paths.insert(cur_path_canonical); + seen_paths.insert(cur_path.lexiclean()); self.perform_load(&canonical_path, seen_paths) } @@ -138,7 +128,7 @@ impl Loader { #[cfg(test)] mod tests { - use super::{Error, Loader}; + use super::{Error, Lexiclean, Loader}; use temptree::temptree; #[test] @@ -220,8 +210,8 @@ recipe_b: let loader_output = loader.load(&justfile_a_path).unwrap_err(); assert_matches!(loader_output, Error::IncludeRecursive { cur_path, recursively_included_path } - if cur_path == tmp.path().join("subdir").join("justfile_b").canonicalize().unwrap() && - recursively_included_path == tmp.path().join("justfile").canonicalize().unwrap() + if cur_path == tmp.path().join("subdir").join("justfile_b").lexiclean() && + recursively_included_path == tmp.path().join("justfile").lexiclean() ); } From 8728f388975f86ea24060a07b8031ed81c3e7bc4 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Tue, 3 Jan 2023 02:25:58 -0800 Subject: [PATCH 07/28] Gate includes behind unstable option --- src/config.rs | 8 ++++++++ src/loader.rs | 16 ++++++++++++---- src/subcommand.rs | 7 ++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/config.rs b/src/config.rs index 8c31b4efbc..f953bf2147 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,6 +19,7 @@ pub(crate) struct Config { pub(crate) dry_run: bool, pub(crate) dump_format: DumpFormat, pub(crate) highlight: bool, + pub(crate) includes: bool, pub(crate) invocation_directory: PathBuf, pub(crate) list_heading: String, pub(crate) list_prefix: String, @@ -89,6 +90,7 @@ mod arg { pub(crate) const DRY_RUN: &str = "DRY-RUN"; pub(crate) const DUMP_FORMAT: &str = "DUMP-FORMAT"; pub(crate) const HIGHLIGHT: &str = "HIGHLIGHT"; + pub(crate) const INCLUDES: &str = "INCLUDES"; pub(crate) const JUSTFILE: &str = "JUSTFILE"; pub(crate) const LIST_HEADING: &str = "LIST-HEADING"; pub(crate) const LIST_PREFIX: &str = "LIST-PREFIX"; @@ -162,6 +164,11 @@ impl Config { .help("Highlight echoed recipe lines in bold") .overrides_with(arg::NO_HIGHLIGHT), ) + .arg( + Arg::with_name(arg::INCLUDES) + .long("includes") + .help("Enable the !include directive to include the contents of other justfiles") + ) .arg( Arg::with_name(arg::LIST_HEADING) .long("list-heading") @@ -572,6 +579,7 @@ impl Config { dry_run: matches.is_present(arg::DRY_RUN), dump_format: Self::dump_format_from_matches(matches)?, highlight: !matches.is_present(arg::NO_HIGHLIGHT), + includes: matches.is_present(arg::INCLUDES), shell: matches.value_of(arg::SHELL).map(str::to_owned), load_dotenv: !matches.is_present(arg::NO_DOTENV), shell_command: matches.is_present(arg::SHELL_COMMAND), diff --git a/src/loader.rs b/src/loader.rs index f6d86cc001..b04a553427 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -16,7 +16,15 @@ impl Loader { } } - pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { + pub(crate) fn load_without_includes<'src>(&'src self, path: &Path) -> RunResult<&'src str> { + let src = fs::read_to_string(path).map_err(|io_error| Error::Load { + path: path.to_owned(), + io_error, + })?; + Ok(self.arena.alloc(src)) + } + + pub(crate) fn load_with_includes<'src>(&'src self, path: &Path) -> RunResult<&'src str> { let src = self.perform_load(path, HashSet::new())?; Ok(self.arena.alloc(src)) } @@ -176,7 +184,7 @@ some_recipe: recipe_b let loader = Loader::new(); let justfile_a_path = tmp.path().join("justfile"); - let loader_output = loader.load(&justfile_a_path).unwrap(); + let loader_output = loader.load_with_includes(&justfile_a_path).unwrap(); assert_eq!(loader_output, full_concatenated_output); } @@ -207,7 +215,7 @@ recipe_b: let loader = Loader::new(); let justfile_a_path = tmp.path().join("justfile"); - let loader_output = loader.load(&justfile_a_path).unwrap_err(); + let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err(); assert_matches!(loader_output, Error::IncludeRecursive { cur_path, recursively_included_path } if cur_path == tmp.path().join("subdir").join("justfile_b").lexiclean() && @@ -243,7 +251,7 @@ recipe_b: let loader = Loader::new(); let justfile_a_path = tmp.path().join("justfile"); - let loader_output = loader.load(&justfile_a_path).unwrap_err(); + let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err(); assert_matches!(loader_output, Error::InvalidInclude { line_number } if line_number == 4); } diff --git a/src/subcommand.rs b/src/subcommand.rs index 1c2ff857f6..77c1b12be7 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -177,7 +177,12 @@ impl Subcommand { loader: &'src Loader, search: &Search, ) -> Result<(&'src str, Ast<'src>, Justfile<'src>), Error<'src>> { - let src = loader.load(&search.justfile)?; + let src = if config.includes { + config.require_unstable("--includes is currently an unstable option")?; + loader.load_with_includes(&search.justfile)? + } else { + loader.load_without_includes(&search.justfile)? + }; let tokens = Lexer::lex(src)?; let ast = Parser::parse(&tokens)?; From 988972ff2bfc2115e08e05e4a0e330b96a4a380e Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Tue, 3 Jan 2023 23:24:59 -0800 Subject: [PATCH 08/28] Completions --- completions/just.bash | 2 +- completions/just.elvish | 1 + completions/just.fish | 1 + completions/just.powershell | 1 + completions/just.zsh | 1 + 5 files changed, 5 insertions(+), 1 deletion(-) diff --git a/completions/just.bash b/completions/just.bash index 59d118acf6..12ad1efc87 100644 --- a/completions/just.bash +++ b/completions/just.bash @@ -20,7 +20,7 @@ _just() { case "${cmd}" in just) - opts=" -q -u -v -e -l -h -V -f -d -c -s --check --dry-run --highlight --no-dotenv --no-highlight --quiet --shell-command --clear-shell-args --unsorted --unstable --verbose --changelog --choose --dump --edit --evaluate --fmt --init --list --summary --variables --help --version --chooser --color --dump-format --list-heading --list-prefix --justfile --set --shell --shell-arg --working-directory --command --completions --show --dotenv-filename --dotenv-path ... " + opts=" -q -u -v -e -l -h -V -f -d -c -s --check --dry-run --highlight --includes --no-dotenv --no-highlight --quiet --shell-command --clear-shell-args --unsorted --unstable --verbose --changelog --choose --dump --edit --evaluate --fmt --init --list --summary --variables --help --version --chooser --color --dump-format --list-heading --list-prefix --justfile --set --shell --shell-arg --working-directory --command --completions --show --dotenv-filename --dotenv-path ... " if [[ ${cur} == -* ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 diff --git a/completions/just.elvish b/completions/just.elvish index ef2f326a66..adebc061cf 100644 --- a/completions/just.elvish +++ b/completions/just.elvish @@ -36,6 +36,7 @@ edit:completion:arg-completer[just] = [@words]{ cand --check 'Run `--fmt` in ''check'' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.' cand --dry-run 'Print what just would do without doing it' cand --highlight 'Highlight echoed recipe lines in bold' + cand --includes 'Enable the !include directive to include the contents of other justfiles' cand --no-dotenv 'Don''t load `.env` file' cand --no-highlight 'Don''t highlight echoed recipe lines in bold' cand -q 'Suppress all output' diff --git a/completions/just.fish b/completions/just.fish index 9b55069f8c..38ac3852fe 100644 --- a/completions/just.fish +++ b/completions/just.fish @@ -27,6 +27,7 @@ complete -c just -n "__fish_use_subcommand" -l dotenv-path -d 'Load environment complete -c just -n "__fish_use_subcommand" -l check -d 'Run `--fmt` in \'check\' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.' complete -c just -n "__fish_use_subcommand" -l dry-run -d 'Print what just would do without doing it' complete -c just -n "__fish_use_subcommand" -l highlight -d 'Highlight echoed recipe lines in bold' +complete -c just -n "__fish_use_subcommand" -l includes -d 'Enable the !include directive to include the contents of other justfiles' complete -c just -n "__fish_use_subcommand" -l no-dotenv -d 'Don\'t load `.env` file' complete -c just -n "__fish_use_subcommand" -l no-highlight -d 'Don\'t highlight echoed recipe lines in bold' complete -c just -n "__fish_use_subcommand" -s q -l quiet -d 'Suppress all output' diff --git a/completions/just.powershell b/completions/just.powershell index 02c55c10e6..0b90003750 100644 --- a/completions/just.powershell +++ b/completions/just.powershell @@ -41,6 +41,7 @@ Register-ArgumentCompleter -Native -CommandName 'just' -ScriptBlock { [CompletionResult]::new('--check', 'check', [CompletionResultType]::ParameterName, 'Run `--fmt` in ''check'' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.') [CompletionResult]::new('--dry-run', 'dry-run', [CompletionResultType]::ParameterName, 'Print what just would do without doing it') [CompletionResult]::new('--highlight', 'highlight', [CompletionResultType]::ParameterName, 'Highlight echoed recipe lines in bold') + [CompletionResult]::new('--includes', 'includes', [CompletionResultType]::ParameterName, 'Enable the !include directive to include the contents of other justfiles') [CompletionResult]::new('--no-dotenv', 'no-dotenv', [CompletionResultType]::ParameterName, 'Don''t load `.env` file') [CompletionResult]::new('--no-highlight', 'no-highlight', [CompletionResultType]::ParameterName, 'Don''t highlight echoed recipe lines in bold') [CompletionResult]::new('-q', 'q', [CompletionResultType]::ParameterName, 'Suppress all output') diff --git a/completions/just.zsh b/completions/just.zsh index 78159200ae..a53e13a3bd 100644 --- a/completions/just.zsh +++ b/completions/just.zsh @@ -37,6 +37,7 @@ _just() { '--check[Run `--fmt` in '\''check'\'' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.]' \ '(-q --quiet)--dry-run[Print what just would do without doing it]' \ '--highlight[Highlight echoed recipe lines in bold]' \ +'--includes[Enable the !include directive to include the contents of other justfiles]' \ '--no-dotenv[Don'\''t load `.env` file]' \ '--no-highlight[Don'\''t highlight echoed recipe lines in bold]' \ '(--dry-run)-q[Suppress all output]' \ From 67b0d1310adae8a5b40dd2e93bd585b4b0cd94fd Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sat, 7 Jan 2023 01:13:25 -0800 Subject: [PATCH 09/28] Remove --includes option --- completions/just.bash | 2 +- completions/just.elvish | 1 - completions/just.fish | 1 - completions/just.powershell | 1 - completions/just.zsh | 1 - src/config.rs | 8 -------- src/subcommand.rs | 3 +-- 7 files changed, 2 insertions(+), 15 deletions(-) diff --git a/completions/just.bash b/completions/just.bash index 12ad1efc87..59d118acf6 100644 --- a/completions/just.bash +++ b/completions/just.bash @@ -20,7 +20,7 @@ _just() { case "${cmd}" in just) - opts=" -q -u -v -e -l -h -V -f -d -c -s --check --dry-run --highlight --includes --no-dotenv --no-highlight --quiet --shell-command --clear-shell-args --unsorted --unstable --verbose --changelog --choose --dump --edit --evaluate --fmt --init --list --summary --variables --help --version --chooser --color --dump-format --list-heading --list-prefix --justfile --set --shell --shell-arg --working-directory --command --completions --show --dotenv-filename --dotenv-path ... " + opts=" -q -u -v -e -l -h -V -f -d -c -s --check --dry-run --highlight --no-dotenv --no-highlight --quiet --shell-command --clear-shell-args --unsorted --unstable --verbose --changelog --choose --dump --edit --evaluate --fmt --init --list --summary --variables --help --version --chooser --color --dump-format --list-heading --list-prefix --justfile --set --shell --shell-arg --working-directory --command --completions --show --dotenv-filename --dotenv-path ... " if [[ ${cur} == -* ]] ; then COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) return 0 diff --git a/completions/just.elvish b/completions/just.elvish index adebc061cf..ef2f326a66 100644 --- a/completions/just.elvish +++ b/completions/just.elvish @@ -36,7 +36,6 @@ edit:completion:arg-completer[just] = [@words]{ cand --check 'Run `--fmt` in ''check'' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.' cand --dry-run 'Print what just would do without doing it' cand --highlight 'Highlight echoed recipe lines in bold' - cand --includes 'Enable the !include directive to include the contents of other justfiles' cand --no-dotenv 'Don''t load `.env` file' cand --no-highlight 'Don''t highlight echoed recipe lines in bold' cand -q 'Suppress all output' diff --git a/completions/just.fish b/completions/just.fish index 38ac3852fe..9b55069f8c 100644 --- a/completions/just.fish +++ b/completions/just.fish @@ -27,7 +27,6 @@ complete -c just -n "__fish_use_subcommand" -l dotenv-path -d 'Load environment complete -c just -n "__fish_use_subcommand" -l check -d 'Run `--fmt` in \'check\' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.' complete -c just -n "__fish_use_subcommand" -l dry-run -d 'Print what just would do without doing it' complete -c just -n "__fish_use_subcommand" -l highlight -d 'Highlight echoed recipe lines in bold' -complete -c just -n "__fish_use_subcommand" -l includes -d 'Enable the !include directive to include the contents of other justfiles' complete -c just -n "__fish_use_subcommand" -l no-dotenv -d 'Don\'t load `.env` file' complete -c just -n "__fish_use_subcommand" -l no-highlight -d 'Don\'t highlight echoed recipe lines in bold' complete -c just -n "__fish_use_subcommand" -s q -l quiet -d 'Suppress all output' diff --git a/completions/just.powershell b/completions/just.powershell index 0b90003750..02c55c10e6 100644 --- a/completions/just.powershell +++ b/completions/just.powershell @@ -41,7 +41,6 @@ Register-ArgumentCompleter -Native -CommandName 'just' -ScriptBlock { [CompletionResult]::new('--check', 'check', [CompletionResultType]::ParameterName, 'Run `--fmt` in ''check'' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.') [CompletionResult]::new('--dry-run', 'dry-run', [CompletionResultType]::ParameterName, 'Print what just would do without doing it') [CompletionResult]::new('--highlight', 'highlight', [CompletionResultType]::ParameterName, 'Highlight echoed recipe lines in bold') - [CompletionResult]::new('--includes', 'includes', [CompletionResultType]::ParameterName, 'Enable the !include directive to include the contents of other justfiles') [CompletionResult]::new('--no-dotenv', 'no-dotenv', [CompletionResultType]::ParameterName, 'Don''t load `.env` file') [CompletionResult]::new('--no-highlight', 'no-highlight', [CompletionResultType]::ParameterName, 'Don''t highlight echoed recipe lines in bold') [CompletionResult]::new('-q', 'q', [CompletionResultType]::ParameterName, 'Suppress all output') diff --git a/completions/just.zsh b/completions/just.zsh index a53e13a3bd..78159200ae 100644 --- a/completions/just.zsh +++ b/completions/just.zsh @@ -37,7 +37,6 @@ _just() { '--check[Run `--fmt` in '\''check'\'' mode. Exits with 0 if justfile is formatted correctly. Exits with 1 and prints a diff if formatting is required.]' \ '(-q --quiet)--dry-run[Print what just would do without doing it]' \ '--highlight[Highlight echoed recipe lines in bold]' \ -'--includes[Enable the !include directive to include the contents of other justfiles]' \ '--no-dotenv[Don'\''t load `.env` file]' \ '--no-highlight[Don'\''t highlight echoed recipe lines in bold]' \ '(--dry-run)-q[Suppress all output]' \ diff --git a/src/config.rs b/src/config.rs index f953bf2147..8c31b4efbc 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,7 +19,6 @@ pub(crate) struct Config { pub(crate) dry_run: bool, pub(crate) dump_format: DumpFormat, pub(crate) highlight: bool, - pub(crate) includes: bool, pub(crate) invocation_directory: PathBuf, pub(crate) list_heading: String, pub(crate) list_prefix: String, @@ -90,7 +89,6 @@ mod arg { pub(crate) const DRY_RUN: &str = "DRY-RUN"; pub(crate) const DUMP_FORMAT: &str = "DUMP-FORMAT"; pub(crate) const HIGHLIGHT: &str = "HIGHLIGHT"; - pub(crate) const INCLUDES: &str = "INCLUDES"; pub(crate) const JUSTFILE: &str = "JUSTFILE"; pub(crate) const LIST_HEADING: &str = "LIST-HEADING"; pub(crate) const LIST_PREFIX: &str = "LIST-PREFIX"; @@ -164,11 +162,6 @@ impl Config { .help("Highlight echoed recipe lines in bold") .overrides_with(arg::NO_HIGHLIGHT), ) - .arg( - Arg::with_name(arg::INCLUDES) - .long("includes") - .help("Enable the !include directive to include the contents of other justfiles") - ) .arg( Arg::with_name(arg::LIST_HEADING) .long("list-heading") @@ -579,7 +572,6 @@ impl Config { dry_run: matches.is_present(arg::DRY_RUN), dump_format: Self::dump_format_from_matches(matches)?, highlight: !matches.is_present(arg::NO_HIGHLIGHT), - includes: matches.is_present(arg::INCLUDES), shell: matches.value_of(arg::SHELL).map(str::to_owned), load_dotenv: !matches.is_present(arg::NO_DOTENV), shell_command: matches.is_present(arg::SHELL_COMMAND), diff --git a/src/subcommand.rs b/src/subcommand.rs index 77c1b12be7..f9451c1de7 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -177,8 +177,7 @@ impl Subcommand { loader: &'src Loader, search: &Search, ) -> Result<(&'src str, Ast<'src>, Justfile<'src>), Error<'src>> { - let src = if config.includes { - config.require_unstable("--includes is currently an unstable option")?; + let src = if config.unstable { loader.load_with_includes(&search.justfile)? } else { loader.load_without_includes(&search.justfile)? From c223957d30a1aab0fc5b4ad534f254a0b04d28f9 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sat, 7 Jan 2023 01:17:22 -0800 Subject: [PATCH 10/28] Rename back load --- src/loader.rs | 2 +- src/subcommand.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/loader.rs b/src/loader.rs index b04a553427..37014e8781 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -16,7 +16,7 @@ impl Loader { } } - pub(crate) fn load_without_includes<'src>(&'src self, path: &Path) -> RunResult<&'src str> { + pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { let src = fs::read_to_string(path).map_err(|io_error| Error::Load { path: path.to_owned(), io_error, diff --git a/src/subcommand.rs b/src/subcommand.rs index f9451c1de7..7ea72fd489 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -180,7 +180,7 @@ impl Subcommand { let src = if config.unstable { loader.load_with_includes(&search.justfile)? } else { - loader.load_without_includes(&search.justfile)? + loader.load(&search.justfile)? }; let tokens = Lexer::lex(src)?; From 917bac95c7922daa8931a1b3e908626cd1c269fe Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sat, 7 Jan 2023 02:10:19 -0800 Subject: [PATCH 11/28] Fix readme --- README.md | 47 +++++++++++++++++++++++++++++++++++++++++++++++ src/loader.rs | 35 +++++++++++++---------------------- 2 files changed, 60 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index e99aff00c9..dfa2d57cf8 100644 --- a/README.md +++ b/README.md @@ -2175,6 +2175,53 @@ But they must match: $ just foo/a bar/b error: Conflicting path arguments: `foo/` and `bar/` ``` +### Including `justfile`s + +Just currently has (unstable) support for including the verbatim text of +another justfile using the `!include ` directive: + +``` +# ./justfile + +!include foo/justfile + +recipe_a: recipe_b + echo "A" + +``` + +``` +# `./foo/justfile`: + +recipe_b: + echo "B" + +``` + +You can run `just` against the main justfile as if the contents of +`foo/justfile` were pasted into the file at the location where the `!include` +directive is. + +The path specified in an `!include` can be relative to the location of the +justfile that contains it, or absolute, and `.` and `..` are allowed. `!include +` must appear at the start of a line, and be the only contents of that +line. + +Currently, `!include` directives are allowed to appear in a justfile only +before the first non-blank, non-comment line. This is because this processing +step occurs before parsing, and we don't want to accidentally process the +string "!include" when it happens to occur within a string or in another +post-parsing syntactic context where it shouldn't be. This restriction may +be lifted in future versions. + +Also note that nothing stops a justfile from including a path to a file +that is not a valid `justfile` - it will simply include the contents of +that file, attempt to parse the justfile with all includes, and then fail +with a parse error. + +An included `justfile` can itself have includes, which will be processed +recursively. However, `just` checks to make sure that no file is circularly +included and will fail an error message if it detects this. ### Hiding `justfile`s diff --git a/src/loader.rs b/src/loader.rs index 37014e8781..3f3bfabfed 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -17,39 +17,33 @@ impl Loader { } pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { - let src = fs::read_to_string(path).map_err(|io_error| Error::Load { - path: path.to_owned(), - io_error, - })?; + let src = Self::load_file(path)?; Ok(self.arena.alloc(src)) } pub(crate) fn load_with_includes<'src>(&'src self, path: &Path) -> RunResult<&'src str> { - let src = self.perform_load(path, HashSet::new())?; + let src = self.load_with_includes_recursive(path, HashSet::new())?; Ok(self.arena.alloc(src)) } - fn perform_load(&self, path: &Path, seen_paths: HashSet) -> RunResult { - let src = fs::read_to_string(path).map_err(|io_error| Error::Load { + fn load_file<'a>(path: &Path) -> RunResult<'a, String> { + fs::read_to_string(path).map_err(|io_error| Error::Load { path: path.to_owned(), io_error, - })?; + }) + } + fn load_with_includes_recursive( + &self, + path: &Path, + seen_paths: HashSet, + ) -> RunResult { + let src = Self::load_file(path)?; self.process_includes(src, path, seen_paths) } /// Given the original contents of a Justfile (with include directives), load all the included /// paths to produce one String with the contents of all the files concatenated. - /// - /// Note that this does not do any parsing yet (i.e. nothing stops a justfile from including a - /// file that is not a valid justfile), and that (currently) line numbers in error messages will - /// reference lines in this concatenated String rather than probably-more-useful information - /// about the original file an error came from. - /// - /// !include directives are allowed to appear in a justfile only before the first non-blank, - /// non-comment line. This is because this processing step occurs before parsing, and we don't - /// want to accidentally process the string "!include" when it happens to occur within a string - /// or in another post-parsing syntactic context where it shouldn't be. fn process_includes( &self, original: String, @@ -60,9 +54,6 @@ impl Loader { let include_regexp = Regex::new(INCLUDE_REGEX).unwrap(); - //NOTE this string-processing code can be made a bit cleaner once the Rust std lib Intersperse - //API is stabilized (https://doc.rust-lang.org/std/iter/struct.Intersperse.html) - let mut buf = String::new(); let mut lines = original.lines().enumerate().peekable(); @@ -130,7 +121,7 @@ impl Loader { let mut seen_paths = seen_paths.clone(); seen_paths.insert(cur_path.lexiclean()); - self.perform_load(&canonical_path, seen_paths) + self.load_with_includes_recursive(&canonical_path, seen_paths) } } From ca4854ab4c4b1e169cfb9f9e6b08ba3f59a10059 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sat, 7 Jan 2023 02:35:34 -0800 Subject: [PATCH 12/28] More changes --- src/error.rs | 40 ++++++++++++++++++++-------------------- src/loader.rs | 28 +++++++++------------------- 2 files changed, 29 insertions(+), 39 deletions(-) diff --git a/src/error.rs b/src/error.rs index 6a99818f70..c4c9e99eeb 100644 --- a/src/error.rs +++ b/src/error.rs @@ -31,6 +31,10 @@ pub(crate) enum Error<'src> { chooser: OsString, io_error: io::Error, }, + CircularInclude { + cur_path: PathBuf, + recursively_included_path: PathBuf, + }, Code { recipe: &'src str, line_number: Option, @@ -87,16 +91,9 @@ pub(crate) enum Error<'src> { InitExists { justfile: PathBuf, }, - IncludeRecursive { - cur_path: PathBuf, - recursively_included_path: PathBuf, - }, Internal { message: String, }, - InvalidInclude { - line_number: usize, - }, Io { recipe: &'src str, io_error: io::Error, @@ -128,6 +125,9 @@ pub(crate) enum Error<'src> { recipe: &'src str, io_error: io::Error, }, + TrailingInclude { + line_number: usize, + }, Unknown { recipe: &'src str, line_number: Option, @@ -337,6 +337,12 @@ impl<'src> ColorDisplay for Error<'src> { io_error )?; } + CircularInclude { cur_path, recursively_included_path } => { + write!( + f, + "Justfile at {} tries to recursively include {}", cur_path.display(), recursively_included_path.display() + )?; + }, Code { recipe, line_number, @@ -492,12 +498,6 @@ impl<'src> ColorDisplay for Error<'src> { InitExists { justfile } => { write!(f, "Justfile `{}` already exists", justfile.display())?; } - IncludeRecursive { cur_path, recursively_included_path } => { - write!( - f, - "Justfile at {} tries to recursively include {}", cur_path.display(), recursively_included_path.display() - )?; - }, Internal { message } => { write!( f, @@ -506,13 +506,6 @@ impl<'src> ColorDisplay for Error<'src> { message )?; } - InvalidInclude { line_number } => { - write!( - f, - "!include statement at line {} occurs after the first non-blank, non-comment line", - line_number - )?; - } Io { recipe, io_error } => { match io_error.kind() { io::ErrorKind::NotFound => write!( @@ -587,6 +580,13 @@ impl<'src> ColorDisplay for Error<'src> { "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \ directory or write a file to that directory`:{io_error}", )?, + TrailingInclude { line_number } => { + write!( + f, + "!include statement at line {} occurs after the first non-blank, non-comment line", + line_number + )?; + } Unknown { recipe, line_number, diff --git a/src/loader.rs b/src/loader.rs index 3f3bfabfed..06e05fbfeb 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -35,34 +35,24 @@ impl Loader { fn load_with_includes_recursive( &self, - path: &Path, - seen_paths: HashSet, - ) -> RunResult { - let src = Self::load_file(path)?; - self.process_includes(src, path, seen_paths) - } - - /// Given the original contents of a Justfile (with include directives), load all the included - /// paths to produce one String with the contents of all the files concatenated. - fn process_includes( - &self, - original: String, current_justfile_path: &Path, seen_paths: HashSet, ) -> RunResult { - let has_final_newline = original.ends_with('\n'); + let original_src = Self::load_file(current_justfile_path)?; + + let has_final_newline = original_src.ends_with('\n'); let include_regexp = Regex::new(INCLUDE_REGEX).unwrap(); let mut buf = String::new(); - let mut lines = original.lines().enumerate().peekable(); + let mut lines = original_src.lines().enumerate().peekable(); let mut seen_first_contentful_line = false; while let Some((line_num, line)) = lines.next() { match include_regexp.captures(line) { Some(_) if seen_first_contentful_line => { - return Err(Error::InvalidInclude { + return Err(Error::TrailingInclude { line_number: line_num + 1, }); } @@ -112,7 +102,7 @@ impl Loader { let canonical_path = canonical_path.lexiclean(); if seen_paths.contains(&canonical_path) { - return Err(Error::IncludeRecursive { + return Err(Error::CircularInclude { cur_path: cur_path.to_owned(), recursively_included_path: canonical_path, }); @@ -208,14 +198,14 @@ recipe_b: let justfile_a_path = tmp.path().join("justfile"); let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err(); - assert_matches!(loader_output, Error::IncludeRecursive { cur_path, recursively_included_path } + assert_matches!(loader_output, Error::CircularInclude { cur_path, recursively_included_path } if cur_path == tmp.path().join("subdir").join("justfile_b").lexiclean() && recursively_included_path == tmp.path().join("justfile").lexiclean() ); } #[test] - fn invalid_includes() { + fn trailing_includes() { let justfile_a = r#" # A comment at the top of the file !include ./subdir/justfile_b @@ -244,6 +234,6 @@ recipe_b: let justfile_a_path = tmp.path().join("justfile"); let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err(); - assert_matches!(loader_output, Error::InvalidInclude { line_number } if line_number == 4); + assert_matches!(loader_output, Error::TrailingInclude { line_number } if line_number == 4); } } From 986632ab8dc2b753ecd99109e8d5e09634646a5f Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Sun, 8 Jan 2023 00:20:12 -0800 Subject: [PATCH 13/28] Add integration tests --- src/error.rs | 26 ++++++++-- src/loader.rs | 44 ++++++++++------- tests/includes.rs | 119 ++++++++++++++++++++++++++++++++++++++++++++++ tests/lib.rs | 1 + tests/test.rs | 19 +++++--- 5 files changed, 181 insertions(+), 28 deletions(-) create mode 100644 tests/includes.rs diff --git a/src/error.rs b/src/error.rs index c4c9e99eeb..9eef6aa9a3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -88,6 +88,10 @@ pub(crate) enum Error<'src> { function: Name<'src>, message: String, }, + IncludeMissingPath { + justfile: PathBuf, + line: usize, + }, InitExists { justfile: PathBuf, }, @@ -126,7 +130,8 @@ pub(crate) enum Error<'src> { io_error: io::Error, }, TrailingInclude { - line_number: usize, + justfile: PathBuf, + line: usize, }, Unknown { recipe: &'src str, @@ -495,6 +500,18 @@ impl<'src> ColorDisplay for Error<'src> { message )?; } + IncludeMissingPath { + justfile, line + } => { + + write!( + f, + "!include statement in {} line {} has no argument", + justfile.display(), + line + )?; + + }, InitExists { justfile } => { write!(f, "Justfile `{}` already exists", justfile.display())?; } @@ -580,11 +597,12 @@ impl<'src> ColorDisplay for Error<'src> { "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \ directory or write a file to that directory`:{io_error}", )?, - TrailingInclude { line_number } => { + TrailingInclude { justfile, line } => { write!( f, - "!include statement at line {} occurs after the first non-blank, non-comment line", - line_number + "!include statement in {} line {} occurs after the first non-blank, non-comment line", + justfile.display(), + line )?; } Unknown { diff --git a/src/loader.rs b/src/loader.rs index 06e05fbfeb..6f2ce9a0f7 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; // This regex defines the syntax for a Justfile include statement as `!include ` // occurring at the start of a line, and as the only contents of that line -const INCLUDE_REGEX: &str = "^!include\\s+(.+)$"; +const INCLUDE_REGEX: &str = "^!include(\\s+.+\\s*)$"; pub(crate) struct Loader { arena: Arena, @@ -50,26 +50,35 @@ impl Loader { let mut seen_first_contentful_line = false; while let Some((line_num, line)) = lines.next() { - match include_regexp.captures(line) { - Some(_) if seen_first_contentful_line => { + if line.starts_with("!include") { + if seen_first_contentful_line { return Err(Error::TrailingInclude { - line_number: line_num + 1, + justfile: current_justfile_path.to_owned(), + line: line_num + 1, }); } - Some(captures) => { - let path_match = captures.get(1).unwrap(); - let include_path = Path::new(path_match.as_str()); - let included_contents = - self.process_single_include(current_justfile_path, include_path, &seen_paths)?; - buf.push_str(&included_contents); - } - None => { - if !(line.is_empty() || line.starts_with('#')) { - seen_first_contentful_line = true; + + match include_regexp.captures(line) { + Some(captures) => { + let path_match = captures.get(1).unwrap().as_str(); + let include_path = Path::new(path_match.trim()); + let included_contents = + self.process_single_include(current_justfile_path, include_path, &seen_paths)?; + buf.push_str(&included_contents); + } + None => { + return Err(Error::IncludeMissingPath { + justfile: current_justfile_path.to_owned(), + line: line_num + 1, + }) } - buf.push_str(line); } - }; + } else if line.trim().is_empty() || line.starts_with('#') { + buf.push_str(line); + } else { + seen_first_contentful_line = true; + buf.push_str(line); + } if lines.peek().is_some() { buf.push('\n'); } @@ -178,6 +187,7 @@ some_recipe: recipe_b some_recipe: recipe_b echo "some recipe" + "#; let justfile_b = r#" @@ -234,6 +244,6 @@ recipe_b: let justfile_a_path = tmp.path().join("justfile"); let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err(); - assert_matches!(loader_output, Error::TrailingInclude { line_number } if line_number == 4); + assert_matches!(loader_output, Error::TrailingInclude { line, .. } if line == 4); } } diff --git a/tests/includes.rs b/tests/includes.rs new file mode 100644 index 0000000000..7343bf96c9 --- /dev/null +++ b/tests/includes.rs @@ -0,0 +1,119 @@ +use super::*; +use temptree::temptree; + +#[test] +fn include_fails_without_unstable() { + let justfile_contents = r#" + # Include include.justfile + !include ./include.justfile + + recipe_a: recipe_b + echo "A" + "#; + + let include_justfile_contents = unindent( + r#" + recipe_b: + echo "B" + "#, + ); + + let tmp = temptree! { + "include.justfile": include_justfile_contents, + }; + + Test::with_tempdir(tmp) + .justfile(justfile_contents) + .status(EXIT_FAILURE) + .stderr("error: Expected character `=`\n |\n2 | !include ./include.justfile\n | ^\n") + .run(); +} + +#[test] +fn include_succeeds_with_unstable() { + let justfile_contents = r#" + # !include should work with trailing spaces + !include ./include.justfile + + recipe_a: recipe_b + @echo "A" + "#; + + let include_justfile_contents = unindent( + r#" + recipe_b: + @echo "B" + "#, + ); + + let tmp = temptree! { + "include.justfile": include_justfile_contents, + }; + + Test::with_tempdir(tmp) + .justfile(justfile_contents) + .arg("--unstable") + .arg("recipe_a") + .status(EXIT_SUCCESS) + .stdout("B\nA\n") + .run(); +} + +#[test] +fn include_directive_with_no_path() { + let justfile_contents = r#" + + !include + + recipe_a: + @echo "hello" + "#; + + let tmp = temptree! { + "include.justfile": "#empty justfile", + }; + + let mut path = tmp.path().to_owned(); + path.push("justfile"); + + Test::with_tempdir(tmp) + .justfile(justfile_contents) + .arg("--unstable") + .status(EXIT_FAILURE) + .stderr(&format!( + "error: !include statement in {} line 2 has no argument\n", + path.display() + )) + .run(); +} + +#[test] +fn trailing_include() { + let justfile_contents = r#" + + recipe_b: + @echo "B" + + !include ./include.justfile + + recipe_a: + @echo "hello" + "#; + + let tmp = temptree! { + "include.justfile": "#empty justfile", + }; + + let mut path = tmp.path().to_owned(); + path.push("justfile"); + + Test::with_tempdir(tmp) + .justfile(justfile_contents) + .arg("--unstable") + .status(EXIT_FAILURE) + .stderr(format!( + "error: !include statement in {} line 5 occurs after the first non-blank, non-comment line\n", + path.display() + )) + .run(); +} diff --git a/tests/lib.rs b/tests/lib.rs index 29ddf2cc89..519e5ffb8c 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -51,6 +51,7 @@ mod fallback; mod fmt; mod functions; mod ignore_comments; +mod includes; mod init; #[cfg(unix)] mod interrupts; diff --git a/tests/test.rs b/tests/test.rs index 62bc0895bc..fe3d534d0a 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -165,6 +165,8 @@ impl Test { impl Test { pub(crate) fn run(self) -> TempDir { + let unstable = self.args.iter().any(|item| item.as_str() == "--unstable"); + if let Some(justfile) = &self.justfile { let justfile = unindent(justfile); fs::write(self.justfile_path(), justfile).unwrap(); @@ -246,7 +248,7 @@ impl Test { } if self.status == EXIT_SUCCESS { - test_round_trip(self.tempdir.path()); + test_round_trip(self.tempdir.path(), unstable); } self.tempdir @@ -260,14 +262,17 @@ struct Output<'a> { status: i32, } -fn test_round_trip(tmpdir: &Path) { +fn test_round_trip(tmpdir: &Path, unstable: bool) { println!("Reparsing..."); - let output = Command::new(executable_path("just")) - .current_dir(tmpdir) - .arg("--dump") - .output() - .expect("just invocation failed"); + let mut command = Command::new(executable_path("just")); + command.current_dir(tmpdir).arg("--dump"); + + if unstable { + command.arg("--unstable"); + } + + let output = command.output().expect("just invocation failed"); if !output.status.success() { panic!("dump failed: {}", output.status); From 929e6bb20af64f1035f29bb52ff2efab005a5e59 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Thu, 12 Jan 2023 17:19:58 -0800 Subject: [PATCH 14/28] SOme work --- src/error.rs | 18 +++---- src/loader.rs | 122 +++++++++++++++++++--------------------------- tests/includes.rs | 3 +- 3 files changed, 56 insertions(+), 87 deletions(-) diff --git a/src/error.rs b/src/error.rs index 9eef6aa9a3..ce2dbbcf05 100644 --- a/src/error.rs +++ b/src/error.rs @@ -98,6 +98,9 @@ pub(crate) enum Error<'src> { Internal { message: String, }, + InvalidDirective { + line: String, + }, Io { recipe: &'src str, io_error: io::Error, @@ -129,10 +132,6 @@ pub(crate) enum Error<'src> { recipe: &'src str, io_error: io::Error, }, - TrailingInclude { - justfile: PathBuf, - line: usize, - }, Unknown { recipe: &'src str, line_number: Option, @@ -523,6 +522,9 @@ impl<'src> ColorDisplay for Error<'src> { message )?; } + InvalidDirective { line } => { + write!(f, "Invalid directive: {}", line)?; + } Io { recipe, io_error } => { match io_error.kind() { io::ErrorKind::NotFound => write!( @@ -597,14 +599,6 @@ impl<'src> ColorDisplay for Error<'src> { "Recipe `{recipe}` could not be run because of an IO error while trying to create a temporary \ directory or write a file to that directory`:{io_error}", )?, - TrailingInclude { justfile, line } => { - write!( - f, - "!include statement in {} line {} occurs after the first non-blank, non-comment line", - justfile.display(), - line - )?; - } Unknown { recipe, line_number, diff --git a/src/loader.rs b/src/loader.rs index 6f2ce9a0f7..56229b493b 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -1,10 +1,6 @@ use super::*; use std::collections::HashSet; -// This regex defines the syntax for a Justfile include statement as `!include ` -// occurring at the start of a line, and as the only contents of that line -const INCLUDE_REGEX: &str = "^!include(\\s+.+\\s*)$"; - pub(crate) struct Loader { arena: Arena, } @@ -40,51 +36,66 @@ impl Loader { ) -> RunResult { let original_src = Self::load_file(current_justfile_path)?; - let has_final_newline = original_src.ends_with('\n'); - - let include_regexp = Regex::new(INCLUDE_REGEX).unwrap(); - let mut buf = String::new(); - let mut lines = original_src.lines().enumerate().peekable(); let mut seen_first_contentful_line = false; - while let Some((line_num, line)) = lines.next() { - if line.starts_with("!include") { - if seen_first_contentful_line { - return Err(Error::TrailingInclude { + /// Iterator yielding every line in a string. The line includes newline character(s). + struct LinesWithEndings<'a> { + input: &'a str, + } + + impl<'a> LinesWithEndings<'a> { + fn from(input: &'a str) -> LinesWithEndings<'a> { + LinesWithEndings { input } + } + } + + impl<'a> Iterator for LinesWithEndings<'a> { + type Item = &'a str; + + #[inline] + fn next(&mut self) -> Option<&'a str> { + if self.input.is_empty() { + return None; + } + let split = self + .input + .find('\n') + .map(|i| i + 1) + .unwrap_or(self.input.len()); + let (line, rest) = self.input.split_at(split); + self.input = rest; + Some(line) + } + } + + let iter = LinesWithEndings::from(&original_src); + + for (line_num, line) in iter.enumerate() { + if !seen_first_contentful_line && line.starts_with("!") { + let include = line + .strip_prefix("!include") + .ok_or_else(|| Error::InvalidDirective { line: line.into() })?; + + let include_path_str = include.trim(); + + if include_path_str.is_empty() { + return Err(Error::IncludeMissingPath { justfile: current_justfile_path.to_owned(), line: line_num + 1, }); } - - match include_regexp.captures(line) { - Some(captures) => { - let path_match = captures.get(1).unwrap().as_str(); - let include_path = Path::new(path_match.trim()); - let included_contents = - self.process_single_include(current_justfile_path, include_path, &seen_paths)?; - buf.push_str(&included_contents); - } - None => { - return Err(Error::IncludeMissingPath { - justfile: current_justfile_path.to_owned(), - line: line_num + 1, - }) - } - } - } else if line.trim().is_empty() || line.starts_with('#') { - buf.push_str(line); + let include_path = Path::new(include_path_str); + let included_contents = + self.process_single_include(current_justfile_path, include_path, &seen_paths)?; + buf.push_str(&included_contents); } else { - seen_first_contentful_line = true; + if !(line.trim().is_empty() || line.trim().starts_with('#')) { + seen_first_contentful_line = true; + } buf.push_str(line); } - if lines.peek().is_some() { - buf.push('\n'); - } - } - if has_final_newline { - buf.push('\n'); } Ok(buf) @@ -162,11 +173,9 @@ recipe_b: recipe_c recipe_c: echo "recipe c" - recipe_b: recipe_c echo "recipe b" - some_recipe: recipe_b echo "some recipe" "#; @@ -213,37 +222,4 @@ recipe_b: recursively_included_path == tmp.path().join("justfile").lexiclean() ); } - - #[test] - fn trailing_includes() { - let justfile_a = r#" -# A comment at the top of the file -!include ./subdir/justfile_b - -some_recipe: recipe_b - echo "some recipe" -"#; - - let justfile_b = r#" - -alias b := recipe_b -!include ../justfile - -recipe_b: - echo "recipe b" -"#; - let tmp = temptree! { - justfile: justfile_a, - subdir: { - justfile_b: justfile_b - } - }; - - let loader = Loader::new(); - - let justfile_a_path = tmp.path().join("justfile"); - let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err(); - - assert_matches!(loader_output, Error::TrailingInclude { line, .. } if line == 4); - } } diff --git a/tests/includes.rs b/tests/includes.rs index 7343bf96c9..affa4e6c48 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -112,8 +112,7 @@ fn trailing_include() { .arg("--unstable") .status(EXIT_FAILURE) .stderr(format!( - "error: !include statement in {} line 5 occurs after the first non-blank, non-comment line\n", - path.display() + "error: Expected character `=`\n |\n5 | !include ./include.justfile\n | ^\n", )) .run(); } From 8f61e8dd4c64364a9d1899783b558323c05e289b Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Thu, 12 Jan 2023 17:46:43 -0800 Subject: [PATCH 15/28] Tweaks --- src/loader.rs | 81 ++++++++++++++++++++--------------------- src/run.rs | 10 ++--- src/subcommand.rs | 6 +-- tests/error_messages.rs | 6 +-- tests/includes.rs | 6 +-- 5 files changed, 50 insertions(+), 59 deletions(-) diff --git a/src/loader.rs b/src/loader.rs index 56229b493b..f5b85a616a 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -1,23 +1,44 @@ use super::*; use std::collections::HashSet; +struct LinesWithEndings<'a> { + input: &'a str, +} + +impl<'a> LinesWithEndings<'a> { + fn new(input: &'a str) -> Self { + Self { input } + } +} + +impl<'a> Iterator for LinesWithEndings<'a> { + type Item = &'a str; + + fn next(&mut self) -> Option<&'a str> { + if self.input.is_empty() { + return None; + } + let split = self.input.find('\n').map_or(self.input.len(), |i| i + 1); + let (line, rest) = self.input.split_at(split); + self.input = rest; + Some(line) + } +} + pub(crate) struct Loader { arena: Arena, + unstable: bool, } impl Loader { - pub(crate) fn new() -> Self { + pub(crate) fn new(unstable: bool) -> Self { Loader { arena: Arena::new(), + unstable, } } pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { - let src = Self::load_file(path)?; - Ok(self.arena.alloc(src)) - } - - pub(crate) fn load_with_includes<'src>(&'src self, path: &Path) -> RunResult<&'src str> { let src = self.load_with_includes_recursive(path, HashSet::new())?; Ok(self.arena.alloc(src)) } @@ -40,44 +61,20 @@ impl Loader { let mut seen_first_contentful_line = false; - /// Iterator yielding every line in a string. The line includes newline character(s). - struct LinesWithEndings<'a> { - input: &'a str, - } - - impl<'a> LinesWithEndings<'a> { - fn from(input: &'a str) -> LinesWithEndings<'a> { - LinesWithEndings { input } - } - } - - impl<'a> Iterator for LinesWithEndings<'a> { - type Item = &'a str; - - #[inline] - fn next(&mut self) -> Option<&'a str> { - if self.input.is_empty() { - return None; - } - let split = self - .input - .find('\n') - .map(|i| i + 1) - .unwrap_or(self.input.len()); - let (line, rest) = self.input.split_at(split); - self.input = rest; - Some(line) - } - } - - let iter = LinesWithEndings::from(&original_src); + let iter = LinesWithEndings::new(&original_src); for (line_num, line) in iter.enumerate() { - if !seen_first_contentful_line && line.starts_with("!") { + if !seen_first_contentful_line && line.starts_with('!') { let include = line .strip_prefix("!include") .ok_or_else(|| Error::InvalidDirective { line: line.into() })?; + if !self.unstable { + return Err(Error::Unstable { + message: "The !include directive is currently unstable. ".into(), + }); + } + let include_path_str = include.trim(); if include_path_str.is_empty() { @@ -180,10 +177,10 @@ some_recipe: recipe_b echo "some recipe" "#; - let loader = Loader::new(); + let loader = Loader::new(true); let justfile_a_path = tmp.path().join("justfile"); - let loader_output = loader.load_with_includes(&justfile_a_path).unwrap(); + let loader_output = loader.load(&justfile_a_path).unwrap(); assert_eq!(loader_output, full_concatenated_output); } @@ -212,10 +209,10 @@ recipe_b: } }; - let loader = Loader::new(); + let loader = Loader::new(true); let justfile_a_path = tmp.path().join("justfile"); - let loader_output = loader.load_with_includes(&justfile_a_path).unwrap_err(); + let loader_output = loader.load(&justfile_a_path).unwrap_err(); assert_matches!(loader_output, Error::CircularInclude { cur_path, recursively_included_path } if cur_path == tmp.path().join("subdir").join("justfile_b").lexiclean() && diff --git a/src/run.rs b/src/run.rs index 950fa0bd5f..a1a18c2471 100644 --- a/src/run.rs +++ b/src/run.rs @@ -17,14 +17,14 @@ pub fn run() -> Result<(), i32> { info!("Parsing command line arguments…"); let matches = app.get_matches(); - let loader = Loader::new(); - let config = Config::from_matches(&matches).map_err(Error::from); - let (color, verbosity) = config + let (color, verbosity, unstable) = config .as_ref() - .map(|config| (config.color, config.verbosity)) - .unwrap_or((Color::auto(), Verbosity::default())); + .map(|config| (config.color, config.verbosity, config.unstable)) + .unwrap_or((Color::auto(), Verbosity::default(), false)); + + let loader = Loader::new(unstable); config .and_then(|config| config.run(&loader)) diff --git a/src/subcommand.rs b/src/subcommand.rs index 7ea72fd489..1c2ff857f6 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -177,11 +177,7 @@ impl Subcommand { loader: &'src Loader, search: &Search, ) -> Result<(&'src str, Ast<'src>, Justfile<'src>), Error<'src>> { - let src = if config.unstable { - loader.load_with_includes(&search.justfile)? - } else { - loader.load(&search.justfile)? - }; + let src = loader.load(&search.justfile)?; let tokens = Lexer::lex(src)?; let ast = Parser::parse(&tokens)?; diff --git a/tests/error_messages.rs b/tests/error_messages.rs index 8e5fb36ad0..e17a4afd13 100644 --- a/tests/error_messages.rs +++ b/tests/error_messages.rs @@ -26,11 +26,11 @@ test! { test! { name: unexpected_character, - justfile: "!~", + justfile: "&~", stderr: " - error: Expected character `=` + error: Expected character `&` | - 1 | !~ + 1 | &~ | ^ ", status: EXIT_FAILURE, diff --git a/tests/includes.rs b/tests/includes.rs index affa4e6c48..63a2bc6b70 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -25,7 +25,7 @@ fn include_fails_without_unstable() { Test::with_tempdir(tmp) .justfile(justfile_contents) .status(EXIT_FAILURE) - .stderr("error: Expected character `=`\n |\n2 | !include ./include.justfile\n | ^\n") + .stderr("error: The !include directive is currently unstable. Invoke `just` with the `--unstable` flag to enable unstable features.\n") .run(); } @@ -111,8 +111,6 @@ fn trailing_include() { .justfile(justfile_contents) .arg("--unstable") .status(EXIT_FAILURE) - .stderr(format!( - "error: Expected character `=`\n |\n5 | !include ./include.justfile\n | ^\n", - )) + .stderr("error: Expected character `=`\n |\n5 | !include ./include.justfile\n | ^\n") .run(); } From 8f8e472ed9f4b51d4fcb96368075ffc428b4a74e Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 18:30:24 -0800 Subject: [PATCH 16/28] Simplify tests --- tests/includes.rs | 130 ++++++++++++++++------------------------------ tests/test.rs | 28 +++++----- 2 files changed, 60 insertions(+), 98 deletions(-) diff --git a/tests/includes.rs b/tests/includes.rs index 63a2bc6b70..2053da75a3 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -3,27 +3,8 @@ use temptree::temptree; #[test] fn include_fails_without_unstable() { - let justfile_contents = r#" - # Include include.justfile - !include ./include.justfile - - recipe_a: recipe_b - echo "A" - "#; - - let include_justfile_contents = unindent( - r#" - recipe_b: - echo "B" - "#, - ); - - let tmp = temptree! { - "include.justfile": include_justfile_contents, - }; - - Test::with_tempdir(tmp) - .justfile(justfile_contents) + Test::new() + .justfile("!include ./include.justfile") .status(EXIT_FAILURE) .stderr("error: The !include directive is currently unstable. Invoke `just` with the `--unstable` flag to enable unstable features.\n") .run(); @@ -31,86 +12,65 @@ fn include_fails_without_unstable() { #[test] fn include_succeeds_with_unstable() { - let justfile_contents = r#" - # !include should work with trailing spaces - !include ./include.justfile - - recipe_a: recipe_b - @echo "A" - "#; - - let include_justfile_contents = unindent( - r#" - recipe_b: - @echo "B" - "#, - ); - - let tmp = temptree! { - "include.justfile": include_justfile_contents, - }; + Test::new() + .tree(tree! { + "include.justfile": " + b: + @echo B + ", + }) + .justfile( + " + !include ./include.justfile - Test::with_tempdir(tmp) - .justfile(justfile_contents) + a: b + @echo A + ", + ) .arg("--unstable") - .arg("recipe_a") - .status(EXIT_SUCCESS) + .test_round_trip(false) + .arg("a") .stdout("B\nA\n") .run(); } #[test] -fn include_directive_with_no_path() { - let justfile_contents = r#" - - !include - - recipe_a: - @echo "hello" - "#; - - let tmp = temptree! { - "include.justfile": "#empty justfile", - }; - - let mut path = tmp.path().to_owned(); - path.push("justfile"); +fn trailing_spaces_after_include_are_ignored() { + Test::new() + .tree(tree! { + "include.justfile": " + a: + @echo A + ", + }) + .justfile("!include ./include.justfile\x20") + .arg("--unstable") + .test_round_trip(false) + .stdout("A\n") + .run(); +} - Test::with_tempdir(tmp) - .justfile(justfile_contents) +#[test] +fn include_directive_with_no_path() { + Test::new() + .justfile("!include") .arg("--unstable") .status(EXIT_FAILURE) - .stderr(&format!( - "error: !include statement in {} line 2 has no argument\n", - path.display() - )) + .stderr_regex("error: !include statement in .* line 1 has no argument\n") .run(); } #[test] fn trailing_include() { - let justfile_contents = r#" - - recipe_b: - @echo "B" - - !include ./include.justfile - - recipe_a: - @echo "hello" - "#; - - let tmp = temptree! { - "include.justfile": "#empty justfile", - }; - - let mut path = tmp.path().to_owned(); - path.push("justfile"); - - Test::with_tempdir(tmp) - .justfile(justfile_contents) + Test::new() + .justfile( + " + b: + !include ./include.justfile + ", + ) .arg("--unstable") .status(EXIT_FAILURE) - .stderr("error: Expected character `=`\n |\n5 | !include ./include.justfile\n | ^\n") + .stderr("error: Expected character `=`\n |\n2 | !include ./include.justfile\n | ^\n") .run(); } diff --git a/tests/test.rs b/tests/test.rs index fe3d534d0a..77d28de844 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -47,6 +47,7 @@ pub(crate) struct Test { pub(crate) stdout: String, pub(crate) stdout_regex: Option, pub(crate) tempdir: TempDir, + pub(crate) test_round_trip: bool, pub(crate) unindent_stdout: bool, } @@ -69,6 +70,7 @@ impl Test { stdout: String::new(), stdout_regex: None, tempdir, + test_round_trip: true, unindent_stdout: true, } } @@ -144,6 +146,11 @@ impl Test { self } + pub(crate) fn test_round_trip(mut self, test_round_trip: bool) -> Self { + self.test_round_trip = test_round_trip; + self + } + pub(crate) fn tree(self, mut tree: Tree) -> Self { tree.map(|_name, content| unindent(content)); tree.instantiate(self.tempdir.path()).unwrap(); @@ -165,8 +172,6 @@ impl Test { impl Test { pub(crate) fn run(self) -> TempDir { - let unstable = self.args.iter().any(|item| item.as_str() == "--unstable"); - if let Some(justfile) = &self.justfile { let justfile = unindent(justfile); fs::write(self.justfile_path(), justfile).unwrap(); @@ -247,8 +252,8 @@ impl Test { panic!("Output mismatch."); } - if self.status == EXIT_SUCCESS { - test_round_trip(self.tempdir.path(), unstable); + if self.test_round_trip && self.status == EXIT_SUCCESS { + test_round_trip(self.tempdir.path()); } self.tempdir @@ -262,17 +267,14 @@ struct Output<'a> { status: i32, } -fn test_round_trip(tmpdir: &Path, unstable: bool) { +fn test_round_trip(tmpdir: &Path) { println!("Reparsing..."); - let mut command = Command::new(executable_path("just")); - command.current_dir(tmpdir).arg("--dump"); - - if unstable { - command.arg("--unstable"); - } - - let output = command.output().expect("just invocation failed"); + let output = Command::new(executable_path("just")) + .current_dir(tmpdir) + .arg("--dump") + .output() + .expect("just invocation failed"); if !output.status.success() { panic!("dump failed: {}", output.status); From e693ccd2527edadaf05e8a369c5384ebce56504f Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 18:41:54 -0800 Subject: [PATCH 17/28] Reword readme --- README.md | 60 +++++++++++++++++++++++++------------------------------ 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index dfa2d57cf8..18bae44f90 100644 --- a/README.md +++ b/README.md @@ -2175,53 +2175,47 @@ But they must match: $ just foo/a bar/b error: Conflicting path arguments: `foo/` and `bar/` ``` -### Including `justfile`s +### Include Directives -Just currently has (unstable) support for including the verbatim text of -another justfile using the `!include ` directive: +The `!include` directive, currently unstable, can be used to include the +verbatim text of another file. -``` -# ./justfile - -!include foo/justfile +If you have the following `justfile`: -recipe_a: recipe_b - echo "A" +```mf +!include foo/bar.just -``` +a: b + @echo A ``` -# `./foo/justfile`: -recipe_b: - echo "B" +And the following text in `foo/bar.just`: +```mf +b: + @echo B ``` -You can run `just` against the main justfile as if the contents of -`foo/justfile` were pasted into the file at the location where the `!include` -directive is. +`foo/bar.just` will be included in `justfile` and recipe `b` will be defined: -The path specified in an `!include` can be relative to the location of the -justfile that contains it, or absolute, and `.` and `..` are allowed. `!include -` must appear at the start of a line, and be the only contents of that -line. +```sh +$ just --unstable b +B +$ just --unstable a +B +A +``` -Currently, `!include` directives are allowed to appear in a justfile only -before the first non-blank, non-comment line. This is because this processing -step occurs before parsing, and we don't want to accidentally process the -string "!include" when it happens to occur within a string or in another -post-parsing syntactic context where it shouldn't be. This restriction may -be lifted in future versions. +The `!include` directive path can be absolute or relative to the location of +the justfile containing it. `!include` directives must appear at the beginning +of a line. line. -Also note that nothing stops a justfile from including a path to a file -that is not a valid `justfile` - it will simply include the contents of -that file, attempt to parse the justfile with all includes, and then fail -with a parse error. +`!include` directives are only processed before the first non-blank, +non-comment line. -An included `justfile` can itself have includes, which will be processed -recursively. However, `just` checks to make sure that no file is circularly -included and will fail an error message if it detects this. +Included files can themselves contain `!include` directives, which are +processed recursively. ### Hiding `justfile`s From 3a71434d218d5a9e08365bf314350d2b311078a2 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 18:58:51 -0800 Subject: [PATCH 18/28] Make everything slightly more inscrutable --- src/error.rs | 8 ++--- src/loader.rs | 78 ++++++++++++++++++++++------------------------- tests/includes.rs | 16 +++++++++- 3 files changed, 55 insertions(+), 47 deletions(-) diff --git a/src/error.rs b/src/error.rs index ce2dbbcf05..3213c46355 100644 --- a/src/error.rs +++ b/src/error.rs @@ -32,8 +32,8 @@ pub(crate) enum Error<'src> { io_error: io::Error, }, CircularInclude { - cur_path: PathBuf, - recursively_included_path: PathBuf, + current: PathBuf, + include: PathBuf, }, Code { recipe: &'src str, @@ -341,10 +341,10 @@ impl<'src> ColorDisplay for Error<'src> { io_error )?; } - CircularInclude { cur_path, recursively_included_path } => { + CircularInclude { current, include } => { write!( f, - "Justfile at {} tries to recursively include {}", cur_path.display(), recursively_included_path.display() + "Include `{}` in `{}` is a circular include", include.display(), current.display() )?; }, Code { diff --git a/src/loader.rs b/src/loader.rs index f5b85a616a..9bc84e58de 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -39,7 +39,7 @@ impl Loader { } pub(crate) fn load<'src>(&'src self, path: &Path) -> RunResult<&'src str> { - let src = self.load_with_includes_recursive(path, HashSet::new())?; + let src = self.load_recursive(path, HashSet::new())?; Ok(self.arena.alloc(src)) } @@ -50,85 +50,79 @@ impl Loader { }) } - fn load_with_includes_recursive( - &self, - current_justfile_path: &Path, - seen_paths: HashSet, - ) -> RunResult { - let original_src = Self::load_file(current_justfile_path)?; - - let mut buf = String::new(); + fn load_recursive(&self, file: &Path, seen: HashSet) -> RunResult { + let src = Self::load_file(file)?; - let mut seen_first_contentful_line = false; + let mut output = String::new(); - let iter = LinesWithEndings::new(&original_src); + let mut seen_content = false; - for (line_num, line) in iter.enumerate() { - if !seen_first_contentful_line && line.starts_with('!') { + for (line_num, line) in LinesWithEndings::new(&src).enumerate() { + if !seen_content && line.starts_with('!') { let include = line .strip_prefix("!include") .ok_or_else(|| Error::InvalidDirective { line: line.into() })?; if !self.unstable { return Err(Error::Unstable { - message: "The !include directive is currently unstable. ".into(), + message: "The !include directive is currently unstable.".into(), }); } - let include_path_str = include.trim(); + let argument = include.trim(); - if include_path_str.is_empty() { + if argument.is_empty() { return Err(Error::IncludeMissingPath { - justfile: current_justfile_path.to_owned(), + justfile: file.to_owned(), line: line_num + 1, }); } - let include_path = Path::new(include_path_str); - let included_contents = - self.process_single_include(current_justfile_path, include_path, &seen_paths)?; - buf.push_str(&included_contents); + + let contents = self.process_include(file, Path::new(argument), &seen)?; + + output.push_str(&contents); } else { if !(line.trim().is_empty() || line.trim().starts_with('#')) { - seen_first_contentful_line = true; + seen_content = true; } - buf.push_str(line); + output.push_str(line); } } - Ok(buf) + Ok(output) } - fn process_single_include( + fn process_include( &self, - cur_path: &Path, - include_path: &Path, - seen_paths: &HashSet, + file: &Path, + include: &Path, + seen: &HashSet, ) -> RunResult { - let canonical_path = if include_path.is_relative() { - let current_dir = cur_path.parent().ok_or(Error::Internal { + let canonical_path = if include.is_relative() { + let current_dir = file.parent().ok_or(Error::Internal { message: format!( "Justfile path `{}` has no parent directory", - include_path.display() + include.display() ), })?; - current_dir.join(include_path) + current_dir.join(include) } else { - include_path.to_owned() + include.to_owned() }; let canonical_path = canonical_path.lexiclean(); - if seen_paths.contains(&canonical_path) { + if seen.contains(&canonical_path) { return Err(Error::CircularInclude { - cur_path: cur_path.to_owned(), - recursively_included_path: canonical_path, + current: file.to_owned(), + include: canonical_path, }); } - let mut seen_paths = seen_paths.clone(); - seen_paths.insert(cur_path.lexiclean()); + let mut seen_paths = seen.clone(); + seen_paths.insert(file.lexiclean()); - self.load_with_includes_recursive(&canonical_path, seen_paths) + self.load_recursive(&canonical_path, seen_paths) } } @@ -214,9 +208,9 @@ recipe_b: let justfile_a_path = tmp.path().join("justfile"); let loader_output = loader.load(&justfile_a_path).unwrap_err(); - assert_matches!(loader_output, Error::CircularInclude { cur_path, recursively_included_path } - if cur_path == tmp.path().join("subdir").join("justfile_b").lexiclean() && - recursively_included_path == tmp.path().join("justfile").lexiclean() + assert_matches!(loader_output, Error::CircularInclude { current, include } + if current == tmp.path().join("subdir").join("justfile_b").lexiclean() && + include == tmp.path().join("justfile").lexiclean() ); } } diff --git a/tests/includes.rs b/tests/includes.rs index 2053da75a3..d541257da8 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -6,7 +6,7 @@ fn include_fails_without_unstable() { Test::new() .justfile("!include ./include.justfile") .status(EXIT_FAILURE) - .stderr("error: The !include directive is currently unstable. Invoke `just` with the `--unstable` flag to enable unstable features.\n") + .stderr("error: The !include directive is currently unstable. Invoke `just` with the `--unstable` flag to enable unstable features.\n") .run(); } @@ -74,3 +74,17 @@ fn trailing_include() { .stderr("error: Expected character `=`\n |\n2 | !include ./include.justfile\n | ^\n") .run(); } + +#[test] +fn circular_include() { + Test::new() + .justfile("!include a") + .tree(tree! { + a: "!include b", + b: "!include a", + }) + .arg("--unstable") + .status(EXIT_FAILURE) + .stderr_regex("error: Include `.*/a` in `.*/b` is a circular include") + .run(); +} From 178d3508bae0e7b3efbc35ca71cfadf6698d6b66 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:01:35 -0800 Subject: [PATCH 19/28] tweak --- src/error.rs | 6 +++--- src/loader.rs | 4 ++-- tests/includes.rs | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/error.rs b/src/error.rs index 3213c46355..b8c9c485e2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -500,14 +500,14 @@ impl<'src> ColorDisplay for Error<'src> { )?; } IncludeMissingPath { - justfile, line + justfile, line } => { write!( f, - "!include statement in {} line {} has no argument", + "!include directive on line {} of `{}` has no argument", + line.ordinal(), justfile.display(), - line )?; }, diff --git a/src/loader.rs b/src/loader.rs index 9bc84e58de..8ddea91a8c 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -57,7 +57,7 @@ impl Loader { let mut seen_content = false; - for (line_num, line) in LinesWithEndings::new(&src).enumerate() { + for (i, line) in LinesWithEndings::new(&src).enumerate() { if !seen_content && line.starts_with('!') { let include = line .strip_prefix("!include") @@ -74,7 +74,7 @@ impl Loader { if argument.is_empty() { return Err(Error::IncludeMissingPath { justfile: file.to_owned(), - line: line_num + 1, + line: i, }); } diff --git a/tests/includes.rs b/tests/includes.rs index d541257da8..621bfc3901 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -56,7 +56,7 @@ fn include_directive_with_no_path() { .justfile("!include") .arg("--unstable") .status(EXIT_FAILURE) - .stderr_regex("error: !include statement in .* line 1 has no argument\n") + .stderr_regex("error: !include directive on line 1 of `.*` has no argument\n") .run(); } From 10d71befa5b3a359c930b934c48e572a170e50ca Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:02:34 -0800 Subject: [PATCH 20/28] tweak --- src/error.rs | 4 ++-- src/loader.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/error.rs b/src/error.rs index b8c9c485e2..caef44e736 100644 --- a/src/error.rs +++ b/src/error.rs @@ -89,7 +89,7 @@ pub(crate) enum Error<'src> { message: String, }, IncludeMissingPath { - justfile: PathBuf, + file: PathBuf, line: usize, }, InitExists { @@ -500,7 +500,7 @@ impl<'src> ColorDisplay for Error<'src> { )?; } IncludeMissingPath { - justfile, line + file: justfile, line } => { write!( diff --git a/src/loader.rs b/src/loader.rs index 8ddea91a8c..7ea5612e9a 100644 --- a/src/loader.rs +++ b/src/loader.rs @@ -73,7 +73,7 @@ impl Loader { if argument.is_empty() { return Err(Error::IncludeMissingPath { - justfile: file.to_owned(), + file: file.to_owned(), line: i, }); } From 30bbdfa61092b94faf2f35e3c4b656a7f3852bd3 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:03:44 -0800 Subject: [PATCH 21/28] tweak --- tests/includes.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/includes.rs b/tests/includes.rs index 621bfc3901..41cb75c9bc 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -1,5 +1,4 @@ use super::*; -use temptree::temptree; #[test] fn include_fails_without_unstable() { From 4a04674b5200e6513ff16f248bb5db38c7751f02 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:04:28 -0800 Subject: [PATCH 22/28] tweak --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index caef44e736..a0e6173259 100644 --- a/src/error.rs +++ b/src/error.rs @@ -523,7 +523,7 @@ impl<'src> ColorDisplay for Error<'src> { )?; } InvalidDirective { line } => { - write!(f, "Invalid directive: {}", line)?; + write!(f, "Invalid directive: {}", line.ordinal())?; } Io { recipe, io_error } => { match io_error.kind() { From ca3c710627027387369b37f72e14389d00542892 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:05:28 -0800 Subject: [PATCH 23/28] tweak --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index a0e6173259..54bebc8cfa 100644 --- a/src/error.rs +++ b/src/error.rs @@ -523,7 +523,7 @@ impl<'src> ColorDisplay for Error<'src> { )?; } InvalidDirective { line } => { - write!(f, "Invalid directive: {}", line.ordinal())?; + write!(f, "Invalid directive: {line}")?; } Io { recipe, io_error } => { match io_error.kind() { From 403dfc30cf7a3ed177c6620e9d2b5165ec66fa6a Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:08:21 -0800 Subject: [PATCH 24/28] tweak --- tests/includes.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/includes.rs b/tests/includes.rs index 41cb75c9bc..63ea31b2d0 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -84,6 +84,8 @@ fn circular_include() { }) .arg("--unstable") .status(EXIT_FAILURE) - .stderr_regex("error: Include `.*/a` in `.*/b` is a circular include") + .stderr_regex(path( + "error: Include `.*/a` in `.*/b` is a circular include", + )) .run(); } From cefd457e8b833cba1d0d6637ea073fcbe73468d3 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:14:57 -0800 Subject: [PATCH 25/28] tweak --- tests/choose.rs | 2 +- tests/includes.rs | 2 +- tests/test.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/choose.rs b/tests/choose.rs index d89c31bc9f..b7e9f4425f 100644 --- a/tests/choose.rs +++ b/tests/choose.rs @@ -125,7 +125,7 @@ fn invoke_error_function() { echo bar ", ) - .stderr_regex("error: Chooser `/ -cu fzf` invocation failed: .*") + .stderr_regex("error: Chooser `/ -cu fzf` invocation failed: .*\n") .status(EXIT_FAILURE) .shell(false) .args(["--shell", "/", "--choose"]) diff --git a/tests/includes.rs b/tests/includes.rs index 63ea31b2d0..4644b8b290 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -85,7 +85,7 @@ fn circular_include() { .arg("--unstable") .status(EXIT_FAILURE) .stderr_regex(path( - "error: Include `.*/a` in `.*/b` is a circular include", + "error: Include `.*/a` in `.*/b` is a circular include\n", )) .run(); } diff --git a/tests/test.rs b/tests/test.rs index 77d28de844..bf94facf88 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -127,7 +127,7 @@ impl Test { } pub(crate) fn stderr_regex(mut self, stderr_regex: impl AsRef) -> Self { - self.stderr_regex = Some(Regex::new(&format!("(?m)^{}$", stderr_regex.as_ref())).unwrap()); + self.stderr_regex = Some(Regex::new(&format!("^{}$", stderr_regex.as_ref())).unwrap()); self } From c0be1d90774dab386e5c933408858fe24eb5468d Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:15:41 -0800 Subject: [PATCH 26/28] Don't use multiline mode in stdout_regex --- tests/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test.rs b/tests/test.rs index bf94facf88..8271074fd7 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -142,7 +142,7 @@ impl Test { } pub(crate) fn stdout_regex(mut self, stdout_regex: impl AsRef) -> Self { - self.stdout_regex = Some(Regex::new(&format!("(?m)^{}$", stdout_regex.as_ref())).unwrap()); + self.stdout_regex = Some(Regex::new(&format!("^{}$", stdout_regex.as_ref())).unwrap()); self } From c2ae3d8b33c8b81fcf7a4df638a9363a075cccb3 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:19:26 -0800 Subject: [PATCH 27/28] tweak --- tests/includes.rs | 2 +- tests/lib.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/includes.rs b/tests/includes.rs index 4644b8b290..8850b9b6cc 100644 --- a/tests/includes.rs +++ b/tests/includes.rs @@ -84,7 +84,7 @@ fn circular_include() { }) .arg("--unstable") .status(EXIT_FAILURE) - .stderr_regex(path( + .stderr_regex(path_for_regex( "error: Include `.*/a` in `.*/b` is a circular include\n", )) .run(); diff --git a/tests/lib.rs b/tests/lib.rs index 519e5ffb8c..6dbc733e86 100644 --- a/tests/lib.rs +++ b/tests/lib.rs @@ -94,3 +94,11 @@ fn path(s: &str) -> String { s.into() } } + +fn path_for_regex(s: &str) -> String { + if cfg!(windows) { + s.replace('/', "\\\\") + } else { + s.into() + } +} From f53ba5d9eed87302e40c81a222d5728f50c00f68 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 12 Jan 2023 19:23:08 -0800 Subject: [PATCH 28/28] tweak --- .github/workflows/ci.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e0d8adea78..4a74619267 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -30,9 +30,6 @@ jobs: - uses: Swatinem/rust-cache@v1 - - name: Check Lockfile - run: cargo update --locked --package just - - name: Clippy run: cargo clippy --all --all-targets