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

Feat Proposal: add progressbar to output #78

Merged

Conversation

dimtrovich
Copy link
Contributor

it would be nice to have a progress bar in the console. At times, a command can execute several tasks simultaneously (for example creation of controllers, models, views, etc.). It would therefore be practical for the person opposite to know the current level of progress of the activity.

@adhocore
Copy link
Owner

thank you very much. checking soon. this is definitely a missing and good to have stuff. 👍


/**
* Get the progress bar string, basically:
* =============> 50% label
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty nice. it would be even better if we can choose progressbar char from a set eg =, , , , , etc with default = :)
which seems as simple as swapping the options['loader']

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's the idea of the options property. it is planned to put a setter which will modify the pointer, the loader and the color of the progress bar.
we can do something like
$progress->option('loader', '*'); or
$progress->option([ 'loader' => '*', 'pointer' => '>>' ]);

protected function progress(int $total = null): ProgressBar
{
if ($this->_progressBar === null) {
$this->_progressBar = new ProgressBar(null, $this->writer());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is same progressbar reusable again n again without resetting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh no... i was too focused on minimal resource usage but this way we'll have the same instance of the progressbar every time. thanks very much.

Nevertheless I do not see why someone would venture to call this method several times in the same command

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea that may not be too normal but not impossible either. maybe in rare cases someone would like to have multiple phases and each phase with different steps that would show progresses.
we can revisit on this later though.

Copy link
Owner

@adhocore adhocore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks again, looks good 💯. liked how the code quality is aligned to the project.
only missing is unit test :D

dimtrovich and others added 3 commits March 20, 2023 11:51
Co-authored-by: Jitendra <2908547+adhocore@users.noreply.github.com>
Co-authored-by: Jitendra <2908547+adhocore@users.noreply.github.com>
Co-authored-by: Jitendra <2908547+adhocore@users.noreply.github.com>
@@ -166,7 +166,9 @@ public function each($items, callable $callback = null)
$items = iterator_to_array($items);
}

$total = count($items);
if (0 === $total = count($items)) {
Copy link
Owner

@adhocore adhocore Mar 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh oops, i think i didn't know how to format code suggestion of github.
the idea was to combine assignment and checks in oneliner

// before
$total = count($items) 
if (!$total) {
    return;
}

// after
if (0 === $total = count($items)) {
    return;
}

@adhocore
Copy link
Owner

adhocore commented Mar 25, 2023

hello @dimtrovich , not sure if you lost interest on this pull request? :)
i think some unit tests, and options for progressbar should be enough to merge it.

@dimtrovich
Copy link
Contributor Author

hello @adhocore. sorry for the absence, I had a few small occupations.

I added progress bar customization via options and removed singletton behavior from progress bar initialization in commands.

however I had difficulty writing tests. it must be said that I do not use phpunit too much but rather kahlan or pest.
It will make me happy if you give me a hand for setting up the tests, it will allow me to learn a little more 🙏🙏

@adhocore adhocore changed the base branch from main to 81-progress-bar March 28, 2023 12:45
@adhocore adhocore merged commit b25e04f into adhocore:81-progress-bar Mar 28, 2023
@dimtrovich dimtrovich deleted the feat-add-progressbar-to-output branch March 28, 2023 13:01
$line_count = $this->hasLabelLine ? 2 : 1;

$progress_bar = $this->cursor->up($line_count);
$progress_bar .= "\r";
Copy link
Owner

@adhocore adhocore Mar 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this CR char required? in unix it doesnt seem to be required

(i'm on branch 81-progress-bar trying to write tests)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried with unix but with windows it is essential. when removed, the progress bar goes to a new line on each iteration

Preview => https://ibb.co/W0kMTcz

@dimtrovich dimtrovich restored the feat-add-progressbar-to-output branch March 28, 2023 16:29
@adhocore adhocore mentioned this pull request Mar 29, 2023
@dimtrovich dimtrovich deleted the feat-add-progressbar-to-output branch April 11, 2023 08:44
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

Successfully merging this pull request may close these issues.

2 participants