Add a confirm attribute for recipes #1723

Hwatwasthat · 2023-11-12T17:16:38Z

This would add a "Confirm" attribute that requests confirmation before running a recipe. Runs prior to evaluation, and defaults to not running. Potential solution to issue #1711.

this in the recipe runner.

casey

I just added some comments, and I thought of something else: If a recipe doesn't run, any recipe in the current invocation which depends on that recipe should also not run.

.gitignore

casey · 2023-11-12T20:49:32Z

src/attribute.rs

@@ -13,6 +13,7 @@ pub(crate) enum Attribute {
 Private,
 Unix,
 Windows,
+ Confirm


These enum variants should be sorted.

casey · 2023-11-12T20:51:59Z

src/recipe.rs

@@ -63,6 +63,16 @@ impl<'src, D> Recipe<'src, D> {
 self.name.line
 }

+ pub(crate) fn confirm(&self) -> bool {


This function should be fallible, since read_line can fail, so it should return RunResult<bool>

src/recipe.rs

Corrected format Co-authored-by: Casey Rodarmor <casey@rodarmor.com>

Removed extraneous whitespace Co-authored-by: Casey Rodarmor <casey@rodarmor.com>

Allow for full word response in confirmation, allow for failure case of read_line. Co-authored-by: Casey Rodarmor <casey@rodarmor.com>

could not be read.

a depencency that requires confirmation is denied.

Hwatwasthat · 2023-11-13T11:27:05Z

I just added some comments, and I thought of something else: If a recipe doesn't run, any recipe in the current invocation which depends on that recipe should also not run.

I've added in the confirmation during the running of dependencies. Now, if a recipe that requires confirmation is run as a dependency and is not confirmed then it will return an error of type DependencyNotConfirmed, which I could only find a way to insert the name of the dependency being run, I can't find an easy way to inject the parents name without adding in a lot more convolution.

The confirmation is carried out at the start of the Justfile::run_recipe function, as I couldn't see another place to put it that would prevent anything running and would allow the returning of an error rather than the ok of a single recipe that is being run (as that would stop any later recipes that were from running as I understand it, which is not the intended use of confirmation (again, as I understand it).

Hwatwasthat · 2023-11-13T13:42:05Z

Apologies, on testing a bit I realised that I've added in a double confirm. I'll figure out the best way to resolve this and then update the pull.

casey · 2023-11-14T01:01:36Z

Aside from the double confirm, that all sounds good. I think returning an error if a recipe isn't confirmed is sensible. I think that if we get to a recipe that needs confirmation, and it isn't confirmed, we can abort the whole run, and not run any other recipes. Although it is true that we could run any recipes which aren't dependencies of the unconfirmed recipe, not running them doesn't seem like a big deal.

Also, this will require a couple of tests: One that a recipe that requires and receives confirmation runs, and one that a recipe which requires confirmation but doesn't receive it does not run.

casey · 2023-11-14T01:02:48Z

Also, if you feel like adding a --yes flag that confirms all recipes (per this comment #1711 (comment)) which seems useful, go for it, otherwise it can be left to another PR.

Hwatwasthat · 2023-11-14T09:34:34Z

Aside from the double confirm, that all sounds good. I think returning an error if a recipe isn't confirmed is sensible. I think that if we get to a recipe that needs confirmation, and it isn't confirmed, we can abort the whole run, and not run any other recipes. Although it is true that we could run any recipes which aren't dependencies of the unconfirmed recipe, not running them doesn't seem like a big deal.

Also, this will require a couple of tests: One that a recipe that requires and receives confirmation runs, and one that a recipe which requires confirmation but doesn't receive it does not run.

Ok, that makes things easier then, I've got a version working that does that so I'll just plow on with it. Tests to come with that in the next commit.

Hwatwasthat · 2023-11-14T16:57:39Z

Also, if you feel like adding a --yes flag that confirms all recipes (per this comment #1711 (comment)) which seems useful, go for it, otherwise it can be left to another PR.

I was just wondering on the name. yes is a bit ambiguous, maybe --force-confirm (just to make it clear), I feel like --force or --no-confirm doesn't work, as force is ambiguous and no confirm reads to me as the inverse (automatically denying confirm stuff). I have a flag working, right now it's --yes.

Updated readme to reflect new attribute, and added a TOC. Added new error type, NotConfirmed, which returns the name of the recipe that was not confirmed for running.

casey

I tweaked a few error messages, as well as printed the error confirmation message to stderr, which is also line-buffered, so no need to flush.

Thanks for the PR!

Hwatwasthat added 2 commits November 10, 2023 17:19

Add .vscode to ignore settings, for debug etc

4d815f3

Add the confirm attribute for recipes and a check for

9811f5e

this in the recipe runner.

casey requested changes Nov 12, 2023

View reviewed changes

Hwatwasthat and others added 6 commits November 13, 2023 09:11

Update .gitignore

4bddd44

Corrected format Co-authored-by: Casey Rodarmor <casey@rodarmor.com>

Update src/recipe.rs

b0e6a86

Removed extraneous whitespace Co-authored-by: Casey Rodarmor <casey@rodarmor.com>

Update src/recipe.rs

d04a788

Allow for full word response in confirmation, allow for failure case of read_line. Co-authored-by: Casey Rodarmor <casey@rodarmor.com>

Made confirm fallible, returns an error if stdio

f302ccc

could not be read.

Sorted Attributes

9467433

Added confirmation of dependencies. Errors out if

9fdf8a6

a depencency that requires confirmation is denied.

Hwatwasthat marked this pull request as draft November 14, 2023 18:22

Refined confirm attribute and added --force-confirm flag.

8c8f797

Updated readme to reflect new attribute, and added a TOC. Added new error type, NotConfirmed, which returns the name of the recipe that was not confirmed for running.

Hwatwasthat marked this pull request as ready for review November 16, 2023 09:13

casey added 12 commits November 16, 2023 15:26

Sort .gitignore

0c70988

Remove table of contents

8962cf1

Sort recipe attributes

f454cac

Change flag name to --yes

eb9684f

Change flag name to --yes

f7e42c9

Edit readme

c6b15d2

Add newline back to end of justfile

8c25cf4

Change error message

0e27097

Fix test error messages

fd23965

Change confirm text

4e44d02

Print confirmation message to stderr

54e27bd

Add getconfirmation error

f060ac6

casey added 2 commits November 16, 2023 15:48

Merge branch 'master' into confirm_attribute

c2a8fb8

Update completion script

35e9967

casey enabled auto-merge (squash) November 16, 2023 23:50

casey merged commit 9415bee into casey:master Nov 16, 2023
5 checks passed

casey approved these changes Nov 16, 2023

View reviewed changes

Hwatwasthat mentioned this pull request Nov 21, 2023

Feature request – confirmation option #1711

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a confirm attribute for recipes #1723

Add a confirm attribute for recipes #1723

Hwatwasthat commented Nov 12, 2023

casey left a comment

casey Nov 12, 2023

casey Nov 12, 2023

Hwatwasthat commented Nov 13, 2023

Hwatwasthat commented Nov 13, 2023

casey commented Nov 14, 2023

casey commented Nov 14, 2023

Hwatwasthat commented Nov 14, 2023

Hwatwasthat commented Nov 14, 2023

casey left a comment

Add a confirm attribute for recipes #1723

Add a confirm attribute for recipes #1723

Conversation

Hwatwasthat commented Nov 12, 2023

casey left a comment

Choose a reason for hiding this comment

casey Nov 12, 2023

Choose a reason for hiding this comment

casey Nov 12, 2023

Choose a reason for hiding this comment

Hwatwasthat commented Nov 13, 2023

Hwatwasthat commented Nov 13, 2023

casey commented Nov 14, 2023

casey commented Nov 14, 2023

Hwatwasthat commented Nov 14, 2023

Hwatwasthat commented Nov 14, 2023

casey left a comment

Choose a reason for hiding this comment