casey / just

🤖 Just a command runner
https://just.systems
Creative Commons Zero v1.0 Universal
20.21k stars 451 forks source link

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

Open alerque opened 1 month ago

alerque commented 1 month ago

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 commented 1 month ago

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" :wink: , 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 commented 1 month ago

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 commented 1 month ago

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