Repository: casey/just
Pull request #1834 (2024-01-10 22:51:11)

Add confirm prompt
Add functionality to allow specifying a prompt for the `confirm` recipe attribute.

e.g. `[confirm("this recipe is dangerous because blabla")]`

It's a quick and dirty implementation, feel free to nitpick. And thanks a lot for your great work!

CramBL: I made some semi-related changes to `DisplayRange` because it was bugged, and I needed to use ranges to make it more generic. I prefer using `RangeInclusive` because `Range` just looks ugly when you have e.g. `1..1` which at first glance appears to contain `1` but it is a range of length 0 that does not contain 1.

Now the `Display` impl for `DisplayRange` looks pretty messy but it is actually correct now.

Patch:
diff --git a/GRAMMAR.md b/GRAMMAR.md
index 4c4a25ca..5c23cc2f 100644
--- a/GRAMMAR.md
+++ b/GRAMMAR.md
@@ -104,9 +104,11 @@ string        : STRING
 sequence      : expression ',' sequence
               | expression ','?
 
-recipe        : attribute? '@'? NAME parameter* variadic? ':' dependency* body?
+recipe        : attributes* '@'? NAME parameter* variadic? ':' dependency* body?
 
-attribute     : '[' NAME ']' eol
+attributes    : '[' attribute* ']' eol
+
+attribute     : NAME ( '(' string ')' )?
 
 parameter     : '$'? NAME
               | '$'? NAME '=' value
diff --git a/README.md b/README.md
index 0020dabd..b7f58658 100644
--- a/README.md
+++ b/README.md
@@ -1456,6 +1456,7 @@ Recipes may be annotated with attributes that change their behavior.
 | Name | Description |
 |------|-------------|
 | `[confirm]`<sup>1.17.0</sup> | Require confirmation prior to executing recipe. |
+| `[confirm("prompt")]`<sup>master</sup> | Require confirmation prior to executing recipe with a custom prompt. |
 | `[linux]`<sup>1.8.0</sup> | Enable recipe on Linux. |
 | `[macos]`<sup>1.8.0</sup> | Enable recipe on MacOS. |
 | `[no-cd]`<sup>1.9.0</sup> | Don't change directory before executing recipe. |
@@ -1544,6 +1545,16 @@ delete all:
   rm -rf *
 ```
 
+#### Custom Confirmation Prompt<sup>master</sup>
+
+The default confirmation prompt can be overridden with `[confirm(PROMPT)]`:
+
+```just
+[confirm("Are you sure you want to delete everything?")]
+delete-everything:
+  rm -rf *
+```
+
 ### Command Evaluation Using Backticks
 
 Backticks can be used to store the result of commands:
diff --git a/src/alias.rs b/src/alias.rs
index bb9d4400..286cefd0 100644
--- a/src/alias.rs
+++ b/src/alias.rs
@@ -3,7 +3,7 @@ use super::*;
 /// An alias, e.g. `name := target`
 #[derive(Debug, PartialEq, Clone, Serialize)]
 pub(crate) struct Alias<'src, T = Rc<Recipe<'src>>> {
-  pub(crate) attributes: BTreeSet<Attribute>,
+  pub(crate) attributes: BTreeSet<Attribute<'src>>,
   pub(crate) name: Name<'src>,
   #[serde(
     bound(serialize = "T: Keyed<'src>"),
diff --git a/src/analyzer.rs b/src/analyzer.rs
index eb306421..19f36e1d 100644
--- a/src/analyzer.rs
+++ b/src/analyzer.rs
@@ -211,11 +211,11 @@ impl<'src> Analyzer<'src> {
   fn analyze_alias(alias: &Alias<'src, Name<'src>>) -> CompileResult<'src> {
     let name = alias.name.lexeme();
 
-    for attr in &alias.attributes {
-      if *attr != Attribute::Private {
+    for attribute in &alias.attributes {
+      if *attribute != Attribute::Private {
         return Err(alias.name.token.error(AliasInvalidAttribute {
           alias: name,
-          attr: *attr,
+          attribute: attribute.clone(),
         }));
       }
     }
diff --git a/src/attribute.rs b/src/attribute.rs
index 0c361e3b..8516ce94 100644
--- a/src/attribute.rs
+++ b/src/attribute.rs
@@ -1,12 +1,10 @@
 use super::*;
 
-#[derive(
-  EnumString, PartialEq, Debug, Copy, Clone, Serialize, Ord, PartialOrd, Eq, IntoStaticStr,
-)]
+#[derive(EnumString, PartialEq, Debug, Clone, Serialize, Ord, PartialOrd, Eq, IntoStaticStr)]
 #[strum(serialize_all = "kebab-case")]
 #[serde(rename_all = "kebab-case")]
-pub(crate) enum Attribute {
-  Confirm,
+pub(crate) enum Attribute<'src> {
+  Confirm(Option<StringLiteral<'src>>),
   Linux,
   Macos,
   NoCd,
@@ -17,14 +15,45 @@ pub(crate) enum Attribute {
   Windows,
 }
 
-impl Attribute {
-  pub(crate) fn from_name(name: Name) -> Option<Attribute> {
+impl<'src> Attribute<'src> {
+  pub(crate) fn from_name(name: Name) -> Option<Self> {
     name.lexeme().parse().ok()
   }
 
-  pub(crate) fn to_str(self) -> &'static str {
+  pub(crate) fn name(&self) -> &'static str {
     self.into()
   }
+
+  pub(crate) fn with_argument(
+    self,
+    name: Name<'src>,
+    argument: StringLiteral<'src>,
+  ) -> CompileResult<'src, Self> {
+    match self {
+      Self::Confirm(_) => Ok(Self::Confirm(Some(argument))),
+      _ => Err(name.error(CompileErrorKind::UnexpectedAttributeArgument { attribute: self })),
+    }
+  }
+
+  fn argument(&self) -> Option<&StringLiteral> {
+    if let Self::Confirm(prompt) = self {
+      prompt.as_ref()
+    } else {
+      None
+    }
+  }
+}
+
+impl<'src> Display for Attribute<'src> {
+  fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> {
+    write!(f, "{}", self.name())?;
+
+    if let Some(argument) = self.argument() {
+      write!(f, "({argument})")?;
+    }
+
+    Ok(())
+  }
 }
 
 #[cfg(test)]
@@ -32,7 +61,7 @@ mod tests {
   use super::*;
 
   #[test]
-  fn to_str() {
-    assert_eq!(Attribute::NoExitMessage.to_str(), "no-exit-message");
+  fn name() {
+    assert_eq!(Attribute::NoExitMessage.name(), "no-exit-message");
   }
 }
diff --git a/src/compile_error.rs b/src/compile_error.rs
index d1af6a0f..1457dff5 100644
--- a/src/compile_error.rs
+++ b/src/compile_error.rs
@@ -32,11 +32,13 @@ impl Display for CompileError<'_> {
     use CompileErrorKind::*;
 
     match &*self.kind {
-      AliasInvalidAttribute { alias, attr } => write!(
-        f,
-        "Alias {alias} has an invalid attribute `{}`",
-        attr.to_str(),
-      ),
+      AliasInvalidAttribute { alias, attribute } => {
+        write!(
+          f,
+          "Alias `{alias}` has invalid attribute `{}`",
+          attribute.name(),
+        )
+      }
       AliasShadowsRecipe { alias, recipe_line } => write!(
         f,
         "Alias `{alias}` defined on line {} shadows recipe `{alias}` defined on line {}",
@@ -209,6 +211,13 @@ impl Display for CompileError<'_> {
         "Non-default parameter `{parameter}` follows default parameter"
       ),
       UndefinedVariable { variable } => write!(f, "Variable `{variable}` not defined"),
+      UnexpectedAttributeArgument { attribute } => {
+        write!(
+          f,
+          "Attribute `{}` specified with argument but takes no arguments",
+          attribute.name(),
+        )
+      }
       UnexpectedCharacter { expected } => write!(f, "Expected character `{expected}`"),
       UnexpectedClosingDelimiter { close } => {
         write!(f, "Unexpected closing delimiter `{}`", close.close())
diff --git a/src/compile_error_kind.rs b/src/compile_error_kind.rs
index 4c98d5e2..fbdbf2a8 100644
--- a/src/compile_error_kind.rs
+++ b/src/compile_error_kind.rs
@@ -4,7 +4,7 @@ use super::*;
 pub(crate) enum CompileErrorKind<'src> {
   AliasInvalidAttribute {
     alias: &'src str,
-    attr: Attribute,
+    attribute: Attribute<'src>,
   },
   AliasShadowsRecipe {
     alias: &'src str,
@@ -85,6 +85,9 @@ pub(crate) enum CompileErrorKind<'src> {
   UndefinedVariable {
     variable: &'src str,
   },
+  UnexpectedAttributeArgument {
+    attribute: Attribute<'src>,
+  },
   UnexpectedCharacter {
     expected: char,
   },
diff --git a/src/parser.rs b/src/parser.rs
index 3f7deda7..ba006ac5 100644
--- a/src/parser.rs
+++ b/src/parser.rs
@@ -435,7 +435,7 @@ impl<'run, 'src> Parser<'run, 'src> {
   /// Parse an alias, e.g `alias name := target`
   fn parse_alias(
     &mut self,
-    attributes: BTreeSet<Attribute>,
+    attributes: BTreeSet<Attribute<'src>>,
   ) -> CompileResult<'src, Alias<'src, Name<'src>>> {
     self.presume_keyword(Keyword::Alias)?;
     let name = self.parse_name()?;
@@ -672,7 +672,7 @@ impl<'run, 'src> Parser<'run, 'src> {
     &mut self,
     doc: Option<&'src str>,
     quiet: bool,
-    attributes: BTreeSet<Attribute>,
+    attributes: BTreeSet<Attribute<'src>>,
   ) -> CompileResult<'src, UnresolvedRecipe<'src>> {
     let name = self.parse_name()?;
 
@@ -905,7 +905,7 @@ impl<'run, 'src> Parser<'run, 'src> {
   }
 
   /// Parse recipe attributes
-  fn parse_attributes(&mut self) -> CompileResult<'src, Option<BTreeSet<Attribute>>> {
+  fn parse_attributes(&mut self) -> CompileResult<'src, Option<BTreeSet<Attribute<'src>>>> {
     let mut attributes = BTreeMap::new();
 
     while self.accepted(BracketL)? {
@@ -922,6 +922,15 @@ impl<'run, 'src> Parser<'run, 'src> {
             first: *line,
           }));
         }
+
+        let attribute = if self.accepted(ParenL)? {
+          let argument = self.parse_string_literal()?;
+          self.expect(ParenR)?;
+          attribute.with_argument(name, argument)?
+        } else {
+          attribute
+        };
+
         attributes.insert(attribute, name.line);
 
         if !self.accepted(Comma)? {
diff --git a/src/range_ext.rs b/src/range_ext.rs
index 2a96064e..bf4995e8 100644
--- a/src/range_ext.rs
+++ b/src/range_ext.rs
@@ -13,11 +13,13 @@ pub(crate) struct DisplayRange<T>(T);
 impl Display for DisplayRange<&Range<usize>> {
   fn fmt(&self, f: &mut Formatter) -> fmt::Result {
     if self.0.start == self.0.end {
+      write!(f, "0")?;
+    } else if self.0.start == self.0.end - 1 {
       write!(f, "{}", self.0.start)?;
     } else if self.0.end == usize::MAX {
       write!(f, "{} or more", self.0.start)?;
     } else {
-      write!(f, "{} to {}", self.0.start, self.0.end)?;
+      write!(f, "{} to {}", self.0.start, self.0.end - 1)?;
     }
     Ok(())
   }
@@ -47,15 +49,11 @@ mod tests {
 
   #[test]
   fn exclusive() {
-    assert!(!(0..0).range_contains(&0));
     assert!(!(0..0).range_contains(&0));
     assert!(!(1..10).range_contains(&0));
     assert!(!(1..10).range_contains(&10));
     assert!(!(1..10).range_contains(&0));
-    assert!(!(1..10).range_contains(&10));
     assert!((0..1).range_contains(&0));
-    assert!((0..1).range_contains(&0));
-    assert!((10..20).range_contains(&15));
     assert!((10..20).range_contains(&15));
   }
 
@@ -75,8 +73,13 @@ mod tests {
 
   #[test]
   fn display() {
-    assert_eq!((1..1).display().to_string(), "1");
-    assert_eq!((1..2).display().to_string(), "1 to 2");
+    assert!(!(1..1).contains(&1));
+    assert!((1..1).is_empty());
+    assert!((5..5).is_empty());
+    assert_eq!((1..1).display().to_string(), "0");
+    assert_eq!((1..2).display().to_string(), "1");
+    assert_eq!((5..6).display().to_string(), "5");
+    assert_eq!((5..10).display().to_string(), "5 to 9");
     assert_eq!((1..usize::MAX).display().to_string(), "1 or more");
   }
 }
diff --git a/src/recipe.rs b/src/recipe.rs
index bc06cf46..3ef2843f 100644
--- a/src/recipe.rs
+++ b/src/recipe.rs
@@ -22,7 +22,7 @@ fn error_from_signal(recipe: &str, line_number: Option<usize>, exit_status: Exit
 /// A recipe, e.g. `foo: bar baz`
 #[derive(PartialEq, Debug, Clone, Serialize)]
 pub(crate) struct Recipe<'src, D = Dependency<'src>> {
-  pub(crate) attributes: BTreeSet<Attribute>,
+  pub(crate) attributes: BTreeSet<Attribute<'src>>,
   pub(crate) body: Vec<Line<'src>>,
   pub(crate) dependencies: Vec<D>,
   #[serde(skip)]
@@ -71,17 +71,22 @@ impl<'src, D> Recipe<'src, D> {
   }
 
   pub(crate) fn confirm(&self) -> RunResult<'src, bool> {
-    if self.attributes.contains(&Attribute::Confirm) {
-      eprint!("Run recipe `{}`? ", self.name);
-      let mut line = String::new();
-      std::io::stdin()
-        .read_line(&mut line)
-        .map_err(|io_error| Error::GetConfirmation { io_error })?;
-      let line = line.trim().to_lowercase();
-      Ok(line == "y" || line == "yes")
-    } else {
-      Ok(true)
+    for attribute in &self.attributes {
+      if let Attribute::Confirm(prompt) = attribute {
+        if let Some(prompt) = prompt {
+          eprint!("{} ", prompt.cooked);
+        } else {
+          eprint!("Run recipe `{}`? ", self.name);
+        }
+        let mut line = String::new();
+        std::io::stdin()
+          .read_line(&mut line)
+          .map_err(|io_error| Error::GetConfirmation { io_error })?;
+        let line = line.trim().to_lowercase();
+        return Ok(line == "y" || line == "yes");
+      }
     }
+    Ok(true)
   }
 
   pub(crate) fn check_can_be_default_recipe(&self) -> RunResult<'src, ()> {
@@ -423,7 +428,7 @@ impl<'src, D: Display> ColorDisplay for Recipe<'src, D> {
     }
 
     for attribute in &self.attributes {
-      writeln!(f, "[{}]", attribute.to_str())?;
+      writeln!(f, "[{attribute}]")?;
     }
 
     if self.quiet {
diff --git a/src/string_literal.rs b/src/string_literal.rs
index 47459cfc..4a2b7744 100644
--- a/src/string_literal.rs
+++ b/src/string_literal.rs
@@ -1,6 +1,6 @@
 use super::*;
 
-#[derive(PartialEq, Debug, Clone)]
+#[derive(PartialEq, Debug, Clone, Ord, Eq, PartialOrd)]
 pub(crate) struct StringLiteral<'src> {
   pub(crate) kind: StringKind,
   pub(crate) raw: &'src str,


Test patch:
diff --git a/tests/attributes.rs b/tests/attributes.rs
index 605e7476..a3a451cc 100644
--- a/tests/attributes.rs
+++ b/tests/attributes.rs
@@ -72,7 +72,7 @@ fn multiple_attributes_one_line_error_message() {
     )
     .stderr(
       "
-        error: Expected ']' or ',', but found identifier
+        error: Expected ']', ',', or '(', but found identifier
          ——▶ justfile:1:17
           │
         1 │ [macos, windows linux]
@@ -106,3 +106,26 @@ fn multiple_attributes_one_line_duplicate_check() {
     .status(1)
     .run();
 }
+
+#[test]
+fn unexpected_attribute_argument() {
+  Test::new()
+    .justfile(
+      "
+      [private('foo')]
+      foo:
+        exit 1
+    ",
+    )
+    .stderr(
+      "
+        error: Attribute `private` specified with argument but takes no arguments
+         ——▶ justfile:1:2
+          │
+        1 │ [private('foo')]
+          │  ^^^^^^^
+          ",
+    )
+    .status(1)
+    .run();
+}
diff --git a/tests/confirm.rs b/tests/confirm.rs
index 8b37489c..b1a2657d 100644
--- a/tests/confirm.rs
+++ b/tests/confirm.rs
@@ -103,3 +103,49 @@ fn do_not_confirm_recipe_with_confirm_recipe_dependency() {
     .status(1)
     .run();
 }
+
+#[test]
+fn confirm_recipe_with_prompt() {
+  Test::new()
+    .justfile(
+      "
+        [confirm(\"This is dangerous - are you sure you want to run it?\")]
+        requires_confirmation:
+            echo confirmed
+        ",
+    )
+    .stderr("This is dangerous - are you sure you want to run it? echo confirmed\n")
+    .stdout("confirmed\n")
+    .stdin("y")
+    .run();
+}
+
+#[test]
+fn confirm_recipe_with_prompt_too_many_args() {
+  Test::new()
+    .justfile(
+      "
+        [confirm(\"This is dangerous - are you sure you want to run it?\",\"this second argument is not supported\")]
+        requires_confirmation:
+            echo confirmed
+        ",
+    )
+    .stderr("error: Expected ')', but found ','\n ——▶ justfile:1:64\n  │\n1 │ [confirm(\"This is dangerous - are you sure you want to run it?\",\"this second argument is not supported\")]\n  │                                                                ^\n")
+    .stdout("")
+    .status(1)
+    .run();
+}
+
+#[test]
+fn confirm_attribute_is_formatted_correctly() {
+  Test::new()
+    .justfile(
+      "
+        [confirm('prompt')]
+        foo:
+      ",
+    )
+    .arg("--dump")
+    .stdout("[confirm('prompt')]\nfoo:\n")
+    .run();
+}
diff --git a/tests/error_messages.rs b/tests/error_messages.rs
index 89b4fd1c..5c96035b 100644
--- a/tests/error_messages.rs
+++ b/tests/error_messages.rs
@@ -4,7 +4,7 @@ test! {
   name: invalid_alias_attribute,
   justfile: "[private]\n[linux]\nalias t := test\n\ntest:\n",
   stderr: "
-    error: Alias t has an invalid attribute `linux`
+    error: Alias `t` has invalid attribute `linux`
      ——▶ justfile:3:7
       │
     3 │ alias t := test