Repository: casey/just
Pull request #1475 (2023-01-04 22:48:17)

Support multiple recipe search path arguments
Allow multiple search-path recipe arguments if the recipe search path is the same for all of them, and raise an error message if they have conflicting search paths. 

This means that if you have a subdirectory `subdir` with a Justfile containing `recipe_a` and `recipe_b`, you can now invoke just as `just subdir/recipe_a subdir/recipe_b`, and have it correctly run `recipe_a` and `recipe_b` from that Justfile (the current behavior is that it will try to parse "subdir/recipe_b" as a recipe name in and of itself and fail). 

If you try to specify multiple separate Justfiles using this syntax (e.g. `just subdir1/recipe_a subdir2/recipe_b` where subdir1 and subdir2 contain different Justfiles), `just` will fail with an error message.

jpbochi: @neunenak I'm not acquainted with the source code here, nor with rust, but I'm interested in this change. Would you mind writing a higher level example[s] of what this change does? 🥺 
neunenak: > @neunenak I'm not acquainted with the source code here, nor with rust, but I'm interested in this change. Would you mind writing a higher level example[s] of what this change does? pleading_face

Updated the description to hopefully make the intent of this change clearer.
jpbochi: @neunenak Thanks! That makes sense to me. I really like the compromise you found.

Should you also update the README.md part about "Invoking justfiles in Other Directories"? Currently, it says that a `/` will only matter in the first argument. With your change, it will matter in other arguments, too. 

Patch:
diff --git a/README.md b/README.md
index f5b95848..725e9757 100644
--- a/README.md
+++ b/README.md
@@ -2153,6 +2153,29 @@ $ just foo/build
 $ just foo/
 ```
 
+Additional recipes after the first are sought in the same `justfile`. For
+example, the following are both equivalent:
+
+```sh
+$ just foo/a b
+$ (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/`
+```
+
 ### Hiding `justfile`s
 
 `just` looks for `justfile`s named `justfile` and `.justfile`, which can be used to keep a `justfile` hidden.
diff --git a/src/config.rs b/src/config.rs
index ce9438d1..8c31b4ef 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -433,7 +433,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 36a22742..0fd62b3c 100644
--- a/src/config_error.rs
+++ b/src/config_error.rs
@@ -11,6 +11,8 @@ 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 a74a4740..c5643ccf 100644
--- a/src/positional.rs
+++ b/src/positional.rs
@@ -36,41 +36,75 @@ pub struct Positional {
   pub arguments: Vec<String>,
 }
 
+#[derive(Copy, Clone)]
+enum ProcessingStep {
+  Overrides,
+  SearchDir,
+  Arguments,
+}
+
 impl Positional {
-  pub fn from_values<'values>(values: Option<impl IntoIterator<Item = &'values str>>) -> Self {
+  pub(crate) fn from_values<'values>(
+    values: Option<impl IntoIterator<Item = &'values str>>,
+  ) -> Result<Self, ConfigError> {
     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 {
-      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());
+      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;
             }
-          } else {
+          }
+          ProcessingStep::Arguments => {
             arguments.push(value.to_owned());
+            values.next();
           }
-        } else {
-          arguments.push(value.to_owned());
         }
       }
     }
 
-    Self {
+    Ok(Self {
       overrides,
       search_directory,
       arguments,
-    }
+    })
   }
 
   /// Parse an override from a value of the form `NAME=.*`.
@@ -107,7 +141,7 @@ mod tests {
       #[test]
       fn $name() {
         assert_eq! (
-          Positional::from_values(Some($vals.iter().cloned())),
+          Positional::from_values(Some($vals.iter().cloned())).unwrap(),
           Positional {
             overrides: $overrides
               .iter()
@@ -225,4 +259,35 @@ 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/");
+  }
 }


Test patch:
diff --git a/tests/lib.rs b/tests/lib.rs
index cceacc86..29ddf2cc 100644
--- a/tests/lib.rs
+++ b/tests/lib.rs
@@ -72,6 +72,7 @@ mod recursion_limit;
 mod regexes;
 mod run;
 mod search;
+mod search_arguments;
 mod shadowing_parameters;
 mod shebang;
 mod shell;
diff --git a/tests/search_arguments.rs b/tests/search_arguments.rs
new file mode 100644
index 00000000..e842f6d8
--- /dev/null
+++ b/tests/search_arguments.rs
@@ -0,0 +1,77 @@
+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"
+  );
+}