laravel / prompts

Beautiful and user-friendly forms for your command-line PHP applications.
https://laravel.com/docs/prompts
MIT License
533 stars 94 forks source link

Added pause prompt #108

Closed allanmcarvalho closed 8 months ago

allanmcarvalho commented 9 months ago

This PR introduce the pause prompt that will be useful in a lot of scenarios. Eg.:

Captura de Tela 2024-02-08 às 14 19 52

jessarcher commented 8 months ago

Hey @allanmcarvalho, I'm in favour of a pause function, but I'm not sure about the UI.

I've had a similar use case previously, but I just wanted a simple "Press enter to continue..." to allow the user to read the previous output. In that scenario, I wouldn't want a box with text, as the output I want them to read had already been written.

What are your thoughts on something more simple API like:

function pause($message = 'Press enter to continue...'): void
{
    //...
}

Which would just output something similar to the info function:

image

In your case, you could output the license separately, followed by a pause. Although I'd probably use a confirm rather than pause to get an explicit yes/no.

allanmcarvalho commented 8 months ago

Hi @jessarcher! Thanks by your review, you are definitely right. I will refactor with your suggestions ASAP and then push here. Have a good week!

allanmcarvalho commented 8 months ago

@jessarcher I made the suggestions that you provided.

warning("Warning: The changes you are about to make cannot be undone. Please proceed with caution.");

pause();

info('Ok, processing...');

will output after press enter Captura de Tela 2024-02-19 às 16 10 02

and then Captura de Tela 2024-02-19 às 16 10 14

Message attribute is present, but optional and it's default is Press enter to continue....

What do you think now?

jessarcher commented 8 months ago

Hey @allanmcarvalho,

Looks good! I changed it so that when the terminal is non-interactive, the pause has no effect instead of throwing a validation error. The return value will be false if the prompt wasn't rendered in case someone needs to know that. I don't think pause should be used to get a confirmation for destructive actions - just to pause the output to allow the user to read the previous output before continuing execution. It's almost like a confirm prompt where you can't select "No" (but can obviously Ctrl-C, potentially leaving things only halfway complete).

As mentioned previously, for scenarios where you need explicit confirmation from the user before doing something potentially destructive, I'd recommend using the confirm prompt (along with a custom --force option to handle non-interactive scenarios). This is what we do in the framework with the ConfirmableTrait (See MigrateCommand for example usage)

What are your thoughts?

allanmcarvalho commented 8 months ago

Yes, I think that's it. Your point of view appears to well describe the objective behind of creation of this prompt. To be honest, I had the idea to create this prompt in a scenario where I needed to make a pause to read the previous content, so, it's like you described.

For scenarios where the user must to agree if continues or exit, maybe in future an exclusive prompt that made this like a charm can be crafted, but definitely it's not a job for pause.

Thanks for you review and contribution.