Add confirm prompt #1834
Add confirm prompt #1834
Conversation
I will document it in the README.md later today (I expect) EDIT: Done. |
src/parser.rs
Outdated
| let arguments = if self.accepted(ParenL)? { | ||
| let mut arguments = Vec::new(); | ||
|
|
||
| while self.next_is(StringToken) { |
There was a problem hiding this comment.
Actually, we only have one attribute that accepts a string literal, and if we're accepting multiple arguments, we would want to accept commas between them, so we can make arguments be an Option.
There was a problem hiding this comment.
Comma-separated arguments are now accepted, do should comma+space also be accepted?
There was a problem hiding this comment.
Ah, no sorry, what I was getting at is that we don't have any multi-argument attributes, so we shouldn't bother either with parsing multiple arguments, with or without commas. We can just parse ( STRING ).
There was a problem hiding this comment.
Alright, but now it is implemented, do you want it removed? It's like 2 lines and all the scaffolding is there now if there's a need/want for features that take advantage of recipe attributes with arguments. Off the top of my head it could be stuff like [linux("arm64", "amd64")] that enables recipes based on OS + Architecture, or adding kernel version for requirements etc.
But I respect if you invoke YAGNI, then I'll revert it.
There was a problem hiding this comment.
I agree we'll probably add it later, but that could be a while, so I'd assume just remove it. It also makes Attribute::with_arguments simpler, since it can take an Option instead of a Vec.
|
I made some semi-related changes to Now the |
|
Merged! Thank you very much! This release is turning out to have a lot of great contributions ^_^ |
Add functionality to allow specifying a prompt for the
confirmrecipe 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!