From 1a5f07c672e015415262dccbce83e876ca3a3501 Mon Sep 17 00:00:00 2001 From: Greg Shuflin Date: Tue, 24 Jan 2023 20:25:17 -0800 Subject: [PATCH] Ignore additional search path arguments (#1528) - Revert #1475 - Add a test that arguments with different path prefixes are allowed --- README.md | 12 ----- src/config.rs | 2 +- src/config_error.rs | 2 - src/positional.rs | 101 +++++++------------------------------- tests/search_arguments.rs | 78 ++--------------------------- 5 files changed, 24 insertions(+), 171 deletions(-) diff --git a/README.md b/README.md index 6add00f..e757409 100644 --- a/README.md +++ b/README.md @@ -2174,18 +2174,6 @@ $ (cd foo && just a b) And will both invoke recipes `a` and `b` in `foo/justfile`. -For consistency, it possible to use path prefixes for all recipes: - -```sh -$ just foo/a foo/b -``` - -But they must match: - -```sh -$ just foo/a bar/b -error: Conflicting path arguments: `foo/` and `bar/` -``` ### Include Directives The `!include` directive, currently unstable, can be used to include the diff --git a/src/config.rs b/src/config.rs index f36d094..e6d5def 100644 --- a/src/config.rs +++ b/src/config.rs @@ -434,7 +434,7 @@ impl Config { } } - let positional = Positional::from_values(matches.values_of(arg::ARGUMENTS))?; + let positional = Positional::from_values(matches.values_of(arg::ARGUMENTS)); for (name, value) in positional.overrides { overrides.insert(name.clone(), value.clone()); diff --git a/src/config_error.rs b/src/config_error.rs index 0fd62b3..36a2274 100644 --- a/src/config_error.rs +++ b/src/config_error.rs @@ -11,8 +11,6 @@ pub(crate) enum ConfigError { message ))] Internal { message: String }, - #[snafu(display("Conflicting path arguments: `{}` and `{}`", seen, conflicting))] - ConflictingSearchDirArgs { seen: String, conflicting: String }, #[snafu(display( "Path-prefixed recipes may not be used with `--working-directory` or `--justfile`." ))] diff --git a/src/positional.rs b/src/positional.rs index c5643cc..a74a474 100644 --- a/src/positional.rs +++ b/src/positional.rs @@ -36,75 +36,41 @@ pub struct Positional { pub arguments: Vec, } -#[derive(Copy, Clone)] -enum ProcessingStep { - Overrides, - SearchDir, - Arguments, -} - impl Positional { - pub(crate) fn from_values<'values>( - values: Option>, - ) -> Result { + pub fn from_values<'values>(values: Option>) -> Self { let mut overrides = Vec::new(); let mut search_directory = None; let mut arguments = Vec::new(); - let mut processing_step = ProcessingStep::Overrides; - if let Some(values) = values { - let mut values = values.into_iter().peekable(); - while let Some(value) = values.peek() { - let value = *value; - match processing_step { - ProcessingStep::Overrides => { - if let Some(o) = Self::override_from_value(value) { - overrides.push(o); - values.next(); - } else { - processing_step = ProcessingStep::SearchDir; - } - } - ProcessingStep::SearchDir => { - if value == "." || value == ".." { - search_directory = Some(value.to_owned()); - values.next(); - } else if let Some(i) = value.rfind('/') { - let (dir, tail) = value.split_at(i + 1); + for value in values { + if search_directory.is_none() && arguments.is_empty() { + if let Some(o) = Self::override_from_value(value) { + overrides.push(o); + } else if value == "." || value == ".." { + search_directory = Some(value.to_owned()); + } else if let Some(i) = value.rfind('/') { + let (dir, tail) = value.split_at(i + 1); - if let Some(ref seen) = search_directory { - if seen != dir { - return Err(ConfigError::ConflictingSearchDirArgs { - seen: seen.clone(), - conflicting: dir.into(), - }); - } - } else { - search_directory = Some(dir.to_owned()); - } + search_directory = Some(dir.to_owned()); - if !tail.is_empty() { - arguments.push(tail.to_owned()); - } - values.next(); - } else { - processing_step = ProcessingStep::Arguments; + if !tail.is_empty() { + arguments.push(tail.to_owned()); } - } - ProcessingStep::Arguments => { + } else { arguments.push(value.to_owned()); - values.next(); } + } else { + arguments.push(value.to_owned()); } } } - Ok(Self { + Self { overrides, search_directory, arguments, - }) + } } /// Parse an override from a value of the form `NAME=.*`. @@ -141,7 +107,7 @@ mod tests { #[test] fn $name() { assert_eq! ( - Positional::from_values(Some($vals.iter().cloned())).unwrap(), + Positional::from_values(Some($vals.iter().cloned())), Positional { overrides: $overrides .iter() @@ -259,35 +225,4 @@ mod tests { search_directory: None, arguments: ["a", "a=b"], } - - test! { - name: search_dir_and_recipe_only, - values: ["some/path/recipe_a"], - overrides: [], - search_directory: Some("some/path/"), - arguments: ["recipe_a"], - } - - test! { - name: multiple_same_valued_search_directories, - values: ["some/path/recipe_a", "some/path/recipe_b"], - overrides: [], - search_directory: Some("some/path/"), - arguments: ["recipe_a", "recipe_b"], - } - - #[test] - fn invalid_multiple_search_paths() { - let err = Positional::from_values(Some( - [ - "some/path/recipe_a", - "some/path/recipe_b", - "other/path/recipe_c", - ] - .iter() - .copied(), - )) - .unwrap_err(); - assert_matches!(err, ConfigError::ConflictingSearchDirArgs { seen, conflicting } if seen == "some/path/" && conflicting == "other/path/"); - } } diff --git a/tests/search_arguments.rs b/tests/search_arguments.rs index e842f6d..c122404 100644 --- a/tests/search_arguments.rs +++ b/tests/search_arguments.rs @@ -1,77 +1,9 @@ use super::*; #[test] -fn same_path_argument() { - let justfile_contents = unindent( - r#" - recipe_a: - echo "A" - - recipe_b: - echo "B" - "#, - ); - let tmp = temptree! { - subdir: { - justfile: justfile_contents - } - }; - - for arg_list in [ - ["subdir/recipe_a", "recipe_b"], - ["subdir/recipe_a", "subdir/recipe_b"], - ] { - let mut command = Command::new(executable_path("just")); - command.current_dir(tmp.path()); - - for arg in arg_list { - command.arg(arg); - } - - let output = command.output().unwrap(); - - let stdout = String::from_utf8(output.stdout).unwrap(); - - assert!(output.status.success()); - assert_eq!(stdout, "A\nB\n"); - } -} - -#[test] -fn different_path_arguments() { - let justfile_contents1 = unindent( - r#" - recipe_a: - echo "A" - - "#, - ); - let justfile_contents2 = unindent( - r#" - recipe_b: - echo "B" - "#, - ); - let tmp = temptree! { - subdir: { - justfile: justfile_contents1 - }, - subdir2: { - justfile: justfile_contents2 - } - }; - - let output = Command::new(executable_path("just")) - .current_dir(tmp.path()) - .arg("subdir/recipe_a") - .arg("subdir2/recipe_b") - .output() - .unwrap(); - - let stderr = String::from_utf8(output.stderr).unwrap(); - - assert_eq!( - stderr, - "error: Conflicting path arguments: `subdir/` and `subdir2/`\n" - ); +fn argument_with_different_path_prefix_is_allowed() { + Test::new() + .justfile("foo bar:") + .args(["./foo", "../bar"]) + .run(); }