[RFC] Add new method to Progress Helper getCurrent() #11022

Closed
wants to merge 3 commits into
from

Projects

None yet

4 participants

@jcroll
jcroll commented May 30, 2014

Please see my example below for my reasoning on how this method can be useful:

$progress = $this->getHelperSet()->get('progress');

$total = 55;
$progress->start($output, $total);
$stepSize = 10;
$lastStepSize = $total % $stepSize; // 5
$i = 0;
while ($i++ <= $total) {
    // ... do some work

    if (($i % $stepSize) == 0) {
        // advances the progress bar 10 units or on the final iteration 5 units
        $progress->advance($progress->getCurrent() > $total - $lastStepSize ? $lastStepSize : $stepSize);
    }
}

$progress->finish();

I do this work in a listener so it would be nice if the method was part of the helper's api.

@jcroll jcroll add new method getCurrent()
43148ac
@jcroll jcroll changed the title from [WIP] Add new method to Progress Helper getCurrent() to [RFC] Add new method to Progress Helper getCurrent() May 30, 2014
@jakzal jakzal and 1 other commented on an outdated diff May 30, 2014
src/Symfony/Component/Console/Helper/ProgressHelper.php
@@ -268,6 +268,14 @@ public function setCurrent($current, $redraw = false)
$this->display();
}
}
+
+ /**
+ * @return int $current The current progress
@jakzal
jakzal May 30, 2014 Symfony member

$current is not needed here and there's too many spaces after int.

@jcroll
jcroll May 30, 2014

Hi. How many spaces should there be?

@jakzal
jakzal May 30, 2014 Symfony member

Just one: @return int The current progress

I just noticed that other docblocks are messed up in this file (but that's not sth for this PR).

@jcroll
jcroll May 30, 2014

Ok thanks @jakzal yes perhaps that is the reason I am having trouble telling how to format the docblocks :S

@jakzal
jakzal May 30, 2014 Symfony member

Yeah, the issue was introduce when we standardised type usage and moved from integer to int :/

jcroll added some commits May 30, 2014
@jcroll jcroll Update ProgressHelper.php
1e9a9a7
@jcroll jcroll Update ProgressHelper.php
14e1432
@fabpot
Member
fabpot commented Jun 3, 2014

This change should also be done in ProgressBar instead as ProgressHelper is deprecated.

@jcroll jcroll added the Console label Jun 16, 2014
@fabpot
Member
fabpot commented Jun 16, 2014

@jcroll Can you make the changes?

@jcroll
jcroll commented Jun 18, 2014

@fabpot I would like to just in a time crunch to meet a deadline. Happy to make the change when I can find the time.

@Tobion
Member
Tobion commented Jun 20, 2014

Guys, ProgressBar already has such a method. But it's badly named. See #11184

@gido gido referenced this pull request Jul 8, 2014
Merged

[Console] ProgressBar developer experience #11346

9 of 9 tasks complete
@Tobion
Member
Tobion commented Aug 14, 2014

Closing as ProgressHelper is deprecated and the new ProgressBar already has a getter.

@Tobion Tobion closed this Aug 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment