Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "Allow matching search path arguments (#1475)" #1528

Merged
merged 5 commits into from
Jan 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 0 additions & 2 deletions src/config_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`."
))]
Expand Down
105 changes: 20 additions & 85 deletions src/positional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,75 +36,41 @@ pub struct Positional {
pub arguments: Vec<String>,
}

#[derive(Copy, Clone)]
enum ProcessingStep {
Overrides,
SearchDir,
Arguments,
}

impl Positional {
pub(crate) fn from_values<'values>(
values: Option<impl IntoIterator<Item = &'values str>>,
) -> Result<Self, ConfigError> {
pub fn from_values<'values>(values: Option<impl IntoIterator<Item = &'values str>>) -> 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);

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

if !tail.is_empty() {
arguments.push(tail.to_owned());
}
values.next();
} else {
processing_step = ProcessingStep::Arguments;
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);

search_directory = Some(dir.to_owned());

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=.*`.
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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/");
}
}
78 changes: 5 additions & 73 deletions tests/search_arguments.rs
Original file line number Diff line number Diff line change
@@ -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();
}