laravel / prompts

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

Progress bar label and hint not updating immediately #153

Closed noplanman closed 4 months ago

noplanman commented 5 months ago

Laravel Prompts Version

0.1.23

Laravel Version

11.10

PHP Version

8.2.20

Operating System & Version

Ubuntu 22.04

Terminal Application

Tilix

Description

When setting the label or hint within the callback of a progress bar task processing, it seems the fields only get updated at the very end of the task, not during. Same thing applies for the initial label, which stays until the first callback run has completed

Steps To Reproduce

progress(
    label: 'Warming up...', // This label sticks until the first callback run has finished
    steps: 100,
    callback: function ($step, $progress) {
        $progress->label("Performing step {$step}...");

        sleep(1); // I expect label and hint to be different at this point!

        $progress->hint("Step {$step} completed");

        sleep(1);
    },
);
jessarcher commented 5 months ago

Thanks for reporting @noplanman.

Currently, a re-render isn't triggered until after each step. It's straightforward to change it to re-render any time label or hint is called, but we need to be mindful about the performance impact when iterating over many steps given the render call can be pretty expensive.

Here's a comparison with 10,000 iterations using your code sample (without the sleeps):

Current behaviour (re-render after each step) - 8.65s

image

Re-render after each step and after each call to the label and hint methods - 24.24s

image

I ran them both a few times and got similar numbers.

It's worth noting that if we made this change, it would only impact folks calling the label and hint methods in their callback and wouldn't add any additional overhead for other users.

What are your thoughts @joetannenbaum?

noplanman commented 5 months ago

Thanks for the explanation, @jessarcher. It's clear that adding this to the library by default is not a good idea in terms of performance.

Is there a method i can call to force a re-render within the callback? As my steps are quite time-intensive anyway, i don't mind activating the performance hit.

jessarcher commented 4 months ago

There isn't a public render method you can call, but I'm thinking we could add a $force parameter to the label and hint methods to make them re-render immediately.

jessarcher commented 4 months ago

Turns out you can make a protected method public on a child class, so I've created #155 that exposes this for the Progress class.

noplanman commented 4 months ago

Thanks @jessarcher, such a nice and simple fix. Works perfectly for me :pray:

rootfebri commented 3 months ago

Is there a way to add new line at the hint area?

// Called on ->hint($this->estimate())
    private function estimate(): string
    {
        return $this->progress->magenta("Est. time remaining: " . $this->calculateTime($this->oauthLimits, $this->submitted)) . $this->additionalHint;
    }

I created this method and it not work as expected, additionalHint has \n char on its value