From 8bd411de45fa12df91a4c174f3ff68e2497b6615 Mon Sep 17 00:00:00 2001 From: Marc <72197092+CramBL@users.noreply.github.com> Date: Sat, 13 Jan 2024 03:44:13 +0100 Subject: [PATCH] Allow setting custom confirm prompt (#1834) --- GRAMMAR.md | 6 +++-- README.md | 11 +++++++++ src/alias.rs | 2 +- src/analyzer.rs | 6 ++--- src/attribute.rs | 49 +++++++++++++++++++++++++++++++-------- src/compile_error.rs | 19 +++++++++++---- src/compile_error_kind.rs | 5 +++- src/parser.rs | 15 +++++++++--- src/range_ext.rs | 17 ++++++++------ src/recipe.rs | 29 +++++++++++++---------- src/string_literal.rs | 2 +- tests/attributes.rs | 25 +++++++++++++++++++- tests/confirm.rs | 46 ++++++++++++++++++++++++++++++++++++ tests/error_messages.rs | 2 +- 14 files changed, 187 insertions(+), 47 deletions(-) diff --git a/GRAMMAR.md b/GRAMMAR.md index 4c4a25c..5c23cc2 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 0020dab..b7f5865 100644 --- a/README.md +++ b/README.md @@ -1456,6 +1456,7 @@ Recipes may be annotated with attributes that change their behavior. | Name | Description | |------|-------------| | `[confirm]`1.17.0 | Require confirmation prior to executing recipe. | +| `[confirm("prompt")]`master | Require confirmation prior to executing recipe with a custom prompt. | | `[linux]`1.8.0 | Enable recipe on Linux. | | `[macos]`1.8.0 | Enable recipe on MacOS. | | `[no-cd]`1.9.0 | Don't change directory before executing recipe. | @@ -1544,6 +1545,16 @@ delete all: rm -rf * ``` +#### Custom Confirmation Promptmaster + +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 bb9d440..286cefd 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>> { - pub(crate) attributes: BTreeSet, + pub(crate) attributes: BTreeSet>, pub(crate) name: Name<'src>, #[serde( bound(serialize = "T: Keyed<'src>"), diff --git a/src/analyzer.rs b/src/analyzer.rs index eb30642..19f36e1 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 0c361e3..8516ce9 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>), Linux, Macos, NoCd, @@ -17,14 +15,45 @@ pub(crate) enum Attribute { Windows, } -impl Attribute { - pub(crate) fn from_name(name: Name) -> Option { +impl<'src> Attribute<'src> { + pub(crate) fn from_name(name: Name) -> Option { 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 d1af6a0..1457dff 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 4c98d5e..fbdbf2a 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 3f7deda..ba006ac 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, + attributes: BTreeSet>, ) -> 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, + attributes: BTreeSet>, ) -> 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>> { + fn parse_attributes(&mut self) -> CompileResult<'src, Option>>> { 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 2a96064..bf4995e 100644 --- a/src/range_ext.rs +++ b/src/range_ext.rs @@ -13,11 +13,13 @@ pub(crate) struct DisplayRange(T); impl Display for DisplayRange<&Range> { 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 bc06cf4..3ef2843 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -22,7 +22,7 @@ fn error_from_signal(recipe: &str, line_number: Option, 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, + pub(crate) attributes: BTreeSet>, pub(crate) body: Vec>, pub(crate) dependencies: Vec, #[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 47459cf..4a2b774 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, diff --git a/tests/attributes.rs b/tests/attributes.rs index 605e747..a3a451c 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 8b37489..b1a2657 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 89b4fd1..5c96035 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