Add Progress Bar Helper

[joetannenbaum]

This PR adds a progress bar helper. I modeled it largely after the existing Artisan progress bar with some minor alterations, primarily, that you can add optinoally label identifying the current item as the bar progresses.

I added a playground file so you can see the different options, it looks like the following:

$states = [
    'Alabama', 'Alaska', 'Arizona', 'Arkansas', 'California', 'Colorado',
    'Connecticut', 'Delaware', 'Florida', 'Georgia', 'Hawaii', 'Idaho',
];

progressBar(
    label: 'Adding States',
    items: $states,
    callback: function ($item) {
        usleep(250_000);
    },
);

progressBar(
    label: 'Adding States With Label',
    items: $states,
    callback: function ($item) {
        usleep(250_000);
        return $item;
    },
);

// If you don't pass a callback, we return ProgressBar so you can operate in manual mode
$progressBar = progressBar(
    label: 'Adding States Manually',
    items: $states,
);

$progressBar->start();

foreach ($states as $state) {
    usleep(250_000);
    $progressBar->advance($state);
}

$progressBar->finish();

progressBar(
    'Processing with Exception',
    $states,
    fn ($item) => $item === 'Arkansas' ? throw new Exception('Issue with Arkansas!') : usleep(250_000),
);

CleanShot.2023-09-21.at.21.57.14.mp4

In terms of manual mode, I considered returning a different class with only the required public functions, but thought that might be over thinking it.

[jessarcher]

This looks awesome! I will check it out locally soon.

What are your thoughts on naming the function progress and the class Progress? Mostly to keep the function names all lowercase.

Also wondering whether we could support passing a LazyCollection too?

[joetannenbaum]

Good call on the naming convention, I'll switch that out now (it was bothering me that it was cased differently than the others but didn't think to just shorten it to progress). And... yes to support a LazyCollection too? I haven't dealt with them a ton, but happy to check it out, shouldn't be a problem.

[jessarcher]

I noticed a small bug when the terminal is narrow, and the progress bar reaches the right-hand side:

I think this is because the DrawsBoxes trait doesn't adjust the minWidth property until you draw a box, so you'd probably need to duplicate the logic or have it called earlier.

[jessarcher]

I've fixed the terminal width issue and also handled restoring the cursor when Ctrl+C is pressed.

I also removed the LazyCollection support because I realised it needs to iterate everything to get the total count, which defeats its purpose. People would need to use the "manual" feature to use a LazyCollection or other generator. The only extra change I'd make is allowing them to pass an int to items if they aren't providing a callback, as they'd currently need to pass an array with the same number of items as the generator will yield.

[jessarcher]

Hey @joetannenbaum,

I've made a few extra changes:

• It now accepts an integer in addition to an array, so you don't need to pass an array in manual mode purely for it to get the count. This is good for generators/LazyCollections and is more like Laravel's existing version. If they also pass a callback, it will receive the current step.
• I modified the advance method to be more like Laravel/Symfony, where you can pass the number of steps.
• The callback can optionally receive the Progress instance as the second param, so you can still update the text beneath the progress bar and the label too! You could even display the current progress somewhere by querying the Progress object.
• I've renamed itemLabel to hint and allowed it to be passed in - useful if you don't plan on updating it while it renders.
• I've made it so that it stores all of the return values from the callback and gives them back to you at the end. Essentially making it a map function.
• It shows the current progress on the bottom right border.

Keen to know your thoughts! I hope you don't mind me messing around with it - I just got quite excited about the possibilities 😁

[jessarcher]

Oh, I was also wondering about your thoughts on making the label optional.

[joetannenbaum]

I love everything (especially the current progress on the bottom right and the new map-like ability)!

Re: the label being optional. It's hard to imagine showing a progress bar with any context or label in a real application, but maybe that's not our problem? I'm sure it happens. We can omit it, but it looks like we'd have to modify the DrawsBoxes trait to close the box, it leaves a gap if the title is blank because of the padding around the it:

Something like this:

$titleLabel = strlen($title) > 0 ? " {$title} " : $this->{$color}('──');

$topBorder = str_repeat('─', $width - mb_strwidth($this->stripEscapeSequences($title)));
$this->line("{$this->{$color}(' ┌')}{$titleLabel}{$this->{$color}($topBorder . '┐')}");

Not sure if we want to mess with that trait or if there's a trick I'm not aware of, but let me know.

The only other thing is that while I like moving the hint-and-label-setting into the callback itself (updating the label is a really cool side effect of handling it this way, I updated the playground to try it out and it's really fun), it doesn't necessarily feel intuitive to me that just setting the property has that effect.

I could be overthinking it, but does it make sense to have a method like hint() or setHint()/label() or setLabel() to handle this? Or do you think setting the property clear enough to the end user?

progress(
    label: 'Adding States With Label',
    steps: $states,
    callback: function ($item, $progress) {
        usleep(250_000);
        $progress->hint = $item;
    },
);

// vs

progress(
    label: 'Adding States With Label',
    steps: $states,
    callback: function ($item, $progress) {
        usleep(250_000);
        $progress->hint($item);
    },
);

All-in-all though: you just took this thing to the next level and I love it! 🤩

[jessarcher]

I love everything (especially the current progress on the bottom right and the new map-like ability)!

Glad to hear!

Re: the label being optional. It's hard to imagine showing a progress bar with any context or label in a real application, but maybe that's not our problem? I'm sure it happens.

I agree. I think it's just because the existing progress bar doesn't have a label - it seems like it should be optional here. The main reason I'm bringing it up now is in case we need to change the param order to support it being optional.

Not sure if we want to mess with that trait or if there's a trick I'm not aware of, but let me know.

Modifying the trait seems fine to me. Kinda feels like a bugfix for any prompt that uses it tbh.

I could be overthinking it, but does it make sense to have a method like hint() or setHint()/label() or setLabel() to handle this? Or do you think setting the property clear enough to the end user?

I agree - it almost feels like you're doing something that wasn't intended. I've added a fluent label and hint methods 👍

All-in-all though: you just took this thing to the next level and I love it! 🤩

Thanks! I'm really digging this feature! Thanks for all your work!

[joetannenbaum]

Optional labels: makes sense, totally. Let's do it. Do you want me to modify that trait?

[jessarcher]

I'm having second thoughts about moving the label parameter 😅

It felt weird being anywhere other than the start.

I've just pushed a change I'd love your thoughts on. It lets you skip the label if you're using positional arguments or named arguments, so you can do stuff like:

progress(['one', 'two', 'three'], fn ($item) => strtoupper($item));

progress('Label', ['one', 'two', 'three'], fn ($item) => strtoupper($item));

progress(10, fn ($i) => $i);

$progress = progress(10);

$progress = progress(steps: 10);

// etc...

[joetannenbaum]

So I really like the ergonomics of the examples you gave, but I'm a little concerned that the signature is now a bit confusing. It's not clear to me without heading to documentation that I would be able to do this, for example:

progress(['one', 'two', 'three'], fn ($item) => strtoupper($item));

Is there a way we can make it clearer to the user about the positional variations they can use in the codebase itself? Or is this a case where the vast majority of users will use be using label anyways and they can discover the variations via the docs? I'm not sure.

[jessarcher]

So I really like the ergonomics of the examples you gave, but I'm a little concerned that the signature is now a bit confusing.

Totally agree. I was inspired by various methods in Laravel, but I don't think those have as many parameters to juggle.

I'm not aware of a good way to express overloaded function signatures in PHP. If it were a class method, we could add multiple @method tags to the class-level docblock.

Maybe we should just go back to what you had with a mandatory $label and $steps, and if you really don't want a label you just have to specify it as an empty string. The main issue was the gap, but you've solved that one.

I'll make that change shortly and see how it goes (unless you have any other ideas).

Thanks for the feedback!

[jessarcher]

Hey @joetannenbaum,

Are you happy with where this landed? If so, I think we're good to open it for Taylor to review.

Thanks again!

[joetannenbaum]

Hey @jessarcher,

I am happy with where this is at, thank you so much for all of your work on this! Do you feel good about it?

I looked into some sort of @method equivalent as well because I really did love the way those alternate examples looked, but I couldn't find a way to clearly notate the overloads, which is honestly a bit of a bummer.

I'm good with this if you're good, I think it looks super polished. Thanks again!

[jessarcher]

Thanks for double-checking!

Yeah, I'm really happy with where it's at. It can do everything that the existing progress bar can do and more, and it matches the style of Prompts. Fantastic addition!

[woodspire]

@joetannenbaum @jessarcher The only thing missing is some documentation !

Thanks