Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Order of operations for [confirm()] attribute unexpected and not useful for dry-run workflows #2284

Open
alerque opened this issue Jul 30, 2024 · 3 comments

Comments

@alerque
Copy link

alerque commented Jul 30, 2024

The confirm() attributes looks like it could be useful, but the primary use case I imagine for it is for giving an opportunity to manually review some status or output before continuing. This could be especially useful with commands that have a --dry-run or similar option where an action can be previewed. Outputting info from a dry run and allowing the user to see it before confirming actually taking some action(s) seems like a useful workflow.

The way just currently handles this jinxes that workflow by prompting for user input before actually running the dependencies. Take for example this Justfile:

[confirm("Did checks look good?")]
action: checks
	echo "Do stuff"

checks:
	echo "Stuff that dumps some output for manual review"

The output of checks is shown only after the user has been prompted to run action:

$ just
Did checks look good? y
echo "Stuff that dumps some output for manual review"
Stuff that dumps some output for manual review
echo "Do stuff"
Do stuff

I would expect all the dependencies to have been processed (including any prompts they might have triggered) before prompting for a recipe.

@laniakea64
Copy link
Contributor

The way [confirm] currently works is good for "delete everything" type recipes where the actual deletion is done in dependencies. Changing [confirm] behavior to make recipes like this first delete everything and THEN ask for confirmation would be...um..."suboptimal" 😉 , especially in light of just's commitment to backwards-compatibility.

I agree that the functionality you're requesting would be useful, however it needs to be a new, different attribute.

@casey
Copy link
Owner

casey commented Jul 30, 2024

Echoing what @laniakea64 said, I can definitely see the usefulness of running dependencies first, but I think the current behavior is the correct default, in case dependencies are expensive or potentially destructive. Confirming after running dependencies could be added, and I think seems reasonable, but would be needed to be done in a way which was backwards compatible, i.e., not change the meaning or behavior of existing justfiles.

@laniakea64
Copy link
Contributor

Needed something like this myself, and found a solution:

action: checks _action

[confirm("Did checks look good?")]
_action:
	echo "Do stuff"

checks:
	echo "Stuff that dumps some output for manual review"
$ just action
echo "Stuff that dumps some output for manual review"
Stuff that dumps some output for manual review
Did checks look good? y
echo "Do stuff"
Do stuff
$ just action
echo "Stuff that dumps some output for manual review"
Stuff that dumps some output for manual review
Did checks look good? n
error: Recipe `_action` was not confirmed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants