feature #11346 [Console] ProgressBar developer experience (stefanosala, gido)

This PR was merged into the 2.6-dev branch.

Discussion
----------

[Console] ProgressBar developer experience

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #11184
| License       | MIT
| Doc PR        | WIP

## TODO

- [x] Create `getProgress/setProgress` methods to replace `getStep/setCurrent`
- [x] `ProgressBar::setCurrent` should auto-start the ProgressBar.
- [x] You should be able to pass `max` to `start`
- [x] `barCharOriginal` not needed. Logic can simply be part of `getBarChar`
- [x] `getStepWidth` is internal information that should not be public
- [x] when verbosity set to quiet, the progress bar does not even need to execute all the logic to generate output that is then thrown away
- [x] Allow to advance past max.
- [x] negative max needs to be validated
- [x] `getProgressPercent` should return float instead of int.

Commits
-------

42b95df [Console][ProgressBar] Developer experience:  - Removed barCharOriginal  - getProgressPercent should return float instead of int.  - Minor refactoring
3011685 [Console][ProgressBar] Allow to advance past max.
73ca340 [Console][ProgressBar] Developer experience  - Create getProgress/setProgress methods to replace getStep/setCurrent  - ProgressBar::setCurrent should auto-start the ProgressBar.  - You should be able to pass max to start  - getStepWidth is internal information that should not be public  - when verbosity set to quiet, the progress bar does not even need to    execute all the logic to generate output that is then thrown away

src/Symfony/Component/Console/Helper/ProgressBar.php

@@ -24,7 +24,7 @@ class ProgressBar
 {
     // options
     private $barWidth     = 28;
-    private $barChar      = '=';
+    private $barChar;
     private $emptyBarChar = '-';
     private $progressChar = '>';
     private $format       = null;
@@ -34,13 +34,12 @@ class ProgressBar
      * @var OutputInterface
      */
     private $output;
-    private $step;
+    private $step = 0;
     private $max;
     private $startTime;
     private $stepWidth;
-    private $percent;
+    private $percent = 0.0;
-    private $lastMessagesLength;
+    private $lastMessagesLength = 0;
-    private $barCharOriginal;
     private $formatLineCount;
     private $messages;
 
@@ -57,18 +56,11 @@ class ProgressBar
     {
         // Disabling output when it does not support ANSI codes as it would result in a broken display anyway.
         $this->output = $output->isDecorated() ? $output : new NullOutput();
-        $this->max = (int) $max;
+        $this->setMaxSteps($max);
-        $this->stepWidth = $this->max > 0 ? Helper::strlen($this->max) : 4;
-
-        if (!self::$formatters) {
-            self::$formatters = self::initPlaceholderFormatters();
-        }
-
-        if (!self::$formats) {
-            self::$formats = self::initFormats();
-        }
 
         $this->setFormat($this->determineBestFormat());
+
+        $this->startTime = time();
     }
 
     /**
@@ -170,9 +162,21 @@ class ProgressBar
     /**
      * Gets the progress bar step.
      *
+     * @deprecated since 2.6, to be removed in 3.0. Use {@link getProgress()} instead.
+     *
      * @return int     The progress bar step
      */
     public function getStep()
+    {
+        return $this->getProgress();
+    }
+
+    /**
+     * Gets the current step position.
+     *
+     * @return int The progress bar step
+     */
+    public function getProgress()
     {
         return $this->step;
     }
@@ -180,6 +184,8 @@ class ProgressBar
     /**
      * Gets the progress bar step width.
      *
+     * @internal This method is public for PHP 5.3 compatibility, it should not be used.
+     *
      * @return int     The progress bar step width
      */
     public function getStepWidth()
@@ -190,7 +196,7 @@ class ProgressBar
     /**
      * Gets the current progress bar percent.
      *
-     * @return int     The current progress bar percent
+     * @return float The current progress bar percent
      */
     public function getProgressPercent()
     {
@@ -234,6 +240,10 @@ class ProgressBar
      */
     public function getBarCharacter()
     {
+        if (null === $this->barChar) {
+            return $this->max ? '=' : $this->emptyBarChar;
+        }
+
         return $this->barChar;
     }
 
@@ -285,10 +295,10 @@ class ProgressBar
     public function setFormat($format)
     {
         // try to use the _nomax variant if available
-        if (!$this->max && isset(self::$formats[$format.'_nomax'])) {
+        if (!$this->max && null !== self::getFormatDefinition($format.'_nomax')) {
-            $this->format = self::$formats[$format.'_nomax'];
+            $this->format = self::getFormatDefinition($format.'_nomax');
-        } elseif (isset(self::$formats[$format])) {
+        } elseif (null !== self::getFormatDefinition($format)) {
-            $this->format = self::$formats[$format];
+            $this->format = self::getFormatDefinition($format);
         } else {
             $this->format = $format;
         }
@@ -308,18 +318,17 @@ class ProgressBar
 
     /**
      * Starts the progress output.
+     *
+     * @param int|null $max Number of steps to complete the bar (0 if indeterminate), null to leave unchanged
      */
-    public function start()
+    public function start($max = null)
     {
         $this->startTime = time();
         $this->step = 0;
-        $this->percent = 0;
+        $this->percent = 0.0;
-        $this->lastMessagesLength = 0;
-        $this->barCharOriginal = '';
 
-        if (!$this->max) {
+        if (null !== $max) {
-            $this->barCharOriginal = $this->barChar;
+            $this->setMaxSteps($max);
-            $this->barChar = $this->emptyBarChar;
         }
 
         $this->display();
@@ -334,7 +343,21 @@ class ProgressBar
      */
     public function advance($step = 1)
     {
-        $this->setCurrent($this->step + $step);
+        $this->setProgress($this->step + $step);
+    }
+
+    /**
+     * Sets the current progress.
+     *
+     * @deprecated since 2.6, to be removed in 3.0. Use {@link setProgress()} instead.
+     *
+     * @param int     $step The current progress
+     *
+     * @throws \LogicException
+     */
+    public function setCurrent($step)
+    {
+        $this->setProgress($step);
     }
 
     /**
@@ -344,25 +367,21 @@ class ProgressBar
      *
      * @throws \LogicException
      */
-    public function setCurrent($step)
+    public function setProgress($step)
     {
-        if (null === $this->startTime) {
-            throw new \LogicException('You must start the progress bar before calling setCurrent().');
-        }
-
         $step = (int) $step;
         if ($step < $this->step) {
             throw new \LogicException('You can\'t regress the progress bar.');
         }
 
-        if ($this->max > 0 && $step > $this->max) {
+        if ($this->max && $step > $this->max) {
-            throw new \LogicException('You can\'t advance the progress bar past the max value.');
+            $this->max = $step;
         }
 
         $prevPeriod = intval($this->step / $this->redrawFreq);
         $currPeriod = intval($step / $this->redrawFreq);
         $this->step = $step;
-        $this->percent = $this->max > 0 ? (float) $this->step / $this->max : 0;
+        $this->percent = $this->max ? (float) $this->step / $this->max : 0;
         if ($prevPeriod !== $currPeriod || $this->max === $step) {
             $this->display();
         }
@@ -373,32 +392,20 @@ class ProgressBar
      */
     public function finish()
     {
-        if (null === $this->startTime) {
-            throw new \LogicException('You must start the progress bar before calling finish().');
-        }
-
         if (!$this->max) {
-            $this->barChar = $this->barCharOriginal;
             $this->max = $this->step;
-            $this->setCurrent($this->max);
-            $this->max = 0;
-            $this->barChar = $this->emptyBarChar;
-        } else {
-            $this->setCurrent($this->max);
         }
 
-        $this->startTime = null;
+        $this->setProgress($this->max);
     }
 
     /**
      * Outputs the current progress string.
-     *
-     * @throws \LogicException
      */
     public function display()
     {
-        if (null === $this->startTime) {
+        if (OutputInterface::VERBOSITY_QUIET === $this->output->getVerbosity()) {
-            throw new \LogicException('You must start the progress bar before calling display().');
+            return;
         }
 
         // these 3 variables can be removed in favor of using $this in the closure when support for PHP 5.3 will be dropped.
@@ -434,6 +441,17 @@ class ProgressBar
         $this->overwrite(str_repeat("\n", $this->formatLineCount));
     }
 
+    /**
+     * Sets the progress bar maximal steps.
+     *
+     * @param int     The progress bar max steps
+     */
+    private function setMaxSteps($max)
+    {
+        $this->max = max(0, (int) $max);
+        $this->stepWidth = $this->max ? Helper::strlen($this->max) : 4;
+    }
+
     /**
      * Overwrites a previous message to the output.
      *
@@ -473,13 +491,13 @@ class ProgressBar
         switch ($this->output->getVerbosity()) {
             // OutputInterface::VERBOSITY_QUIET: display is disabled anyway
             case OutputInterface::VERBOSITY_VERBOSE:
-                return $this->max > 0 ? 'verbose' : 'verbose_nomax';
+                return $this->max ? 'verbose' : 'verbose_nomax';
             case OutputInterface::VERBOSITY_VERY_VERBOSE:
-                return $this->max > 0 ? 'very_verbose' : 'very_verbose_nomax';
+                return $this->max ? 'very_verbose' : 'very_verbose_nomax';
             case OutputInterface::VERBOSITY_DEBUG:
-                return $this->max > 0 ? 'debug' : 'debug_nomax';
+                return $this->max ? 'debug' : 'debug_nomax';
             default:
-                return $this->max > 0 ? 'normal' : 'normal_nomax';
+                return $this->max ? 'normal' : 'normal_nomax';
         }
     }
 
@@ -487,7 +505,7 @@ class ProgressBar
     {
         return array(
             'bar' => function (ProgressBar $bar, OutputInterface $output) {
-                $completeBars = floor($bar->getMaxSteps() > 0 ? $bar->getProgressPercent() * $bar->getBarWidth() : $bar->getStep() % $bar->getBarWidth());
+                $completeBars = floor($bar->getMaxSteps() > 0 ? $bar->getProgressPercent() * $bar->getBarWidth() : $bar->getProgress() % $bar->getBarWidth());
                 $display = str_repeat($bar->getBarCharacter(), $completeBars);
                 if ($completeBars < $bar->getBarWidth()) {
                     $emptyBars = $bar->getBarWidth() - $completeBars - Helper::strlenWithoutDecoration($output->getFormatter(), $bar->getProgressCharacter());
@@ -504,10 +522,10 @@ class ProgressBar
                     throw new \LogicException('Unable to display the remaining time if the maximum number of steps is not set.');
                 }
 
-                if (!$bar->getStep()) {
+                if (!$bar->getProgress()) {
                     $remaining = 0;
                 } else {
-                    $remaining = round((time() - $bar->getStartTime()) / $bar->getStep() * ($bar->getMaxSteps() - $bar->getStep()));
+                    $remaining = round((time() - $bar->getStartTime()) / $bar->getProgress() * ($bar->getMaxSteps() - $bar->getProgress()));
                 }
 
                 return Helper::formatTime($remaining);
@@ -517,10 +535,10 @@ class ProgressBar
                     throw new \LogicException('Unable to display the estimated time if the maximum number of steps is not set.');
                 }
 
-                if (!$bar->getStep()) {
+                if (!$bar->getProgress()) {
                     $estimated = 0;
                 } else {
-                    $estimated = round((time() - $bar->getStartTime()) / $bar->getStep() * $bar->getMaxSteps());
+                    $estimated = round((time() - $bar->getStartTime()) / $bar->getProgress() * $bar->getMaxSteps());
                 }
 
                 return Helper::formatTime($estimated);
@@ -529,7 +547,7 @@ class ProgressBar
                 return Helper::formatMemory(memory_get_usage(true));
             },
             'current' => function (ProgressBar $bar) {
-                return str_pad($bar->getStep(), $bar->getStepWidth(), ' ', STR_PAD_LEFT);
+                return str_pad($bar->getProgress(), $bar->getStepWidth(), ' ', STR_PAD_LEFT);
             },
             'max' => function (ProgressBar $bar) {
                 return $bar->getMaxSteps();

src/Symfony/Component/Console/Tests/Helper/ProgressBarTest.php

@@ -17,7 +17,21 @@ use Symfony\Component\Console\Output\StreamOutput;
 
 class ProgressBarTest extends \PHPUnit_Framework_TestCase
 {
-    protected $lastMessagesLength;
+    public function testMultipleStart()
+    {
+        $bar = new ProgressBar($output = $this->getOutputStream());
+        $bar->start();
+        $bar->advance();
+        $bar->start();
+
+        rewind($output->getStream());
+        $this->assertEquals(
+            $this->generateOutput('    0 [>---------------------------]').
+            $this->generateOutput('    1 [->--------------------------]').
+            $this->generateOutput('    0 [>---------------------------]'),
+            stream_get_contents($output->getStream())
+        );
+    }
 
     public function testAdvance()
     {
@@ -63,6 +77,22 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testAdvanceOverMax()
+    {
+        $bar = new ProgressBar($output = $this->getOutputStream(), 10);
+        $bar->setProgress(9);
+        $bar->advance();
+        $bar->advance();
+
+        rewind($output->getStream());
+        $this->assertEquals(
+            $this->generateOutput('  9/10 [=========================>--]  90%').
+            $this->generateOutput(' 10/10 [============================] 100%').
+            $this->generateOutput(' 11/11 [============================] 100%'),
+            stream_get_contents($output->getStream())
+        );
+    }
+
     public function testCustomizations()
     {
         $bar = new ProgressBar($output = $this->getOutputStream(), 10);
@@ -82,6 +112,42 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testDisplayWithoutStart()
+    {
+        $bar = new ProgressBar($output = $this->getOutputStream(), 50);
+        $bar->display();
+
+        rewind($output->getStream());
+        $this->assertEquals(
+            $this->generateOutput('  0/50 [>---------------------------]   0%'),
+            stream_get_contents($output->getStream())
+        );
+    }
+
+    public function testDisplayWithQuietVerbosity()
+    {
+        $bar = new ProgressBar($output = $this->getOutputStream(true, StreamOutput::VERBOSITY_QUIET), 50);
+        $bar->display();
+
+        rewind($output->getStream());
+        $this->assertEquals(
+            '',
+            stream_get_contents($output->getStream())
+        );
+    }
+
+    public function testFinishWithoutStart()
+    {
+        $bar = new ProgressBar($output = $this->getOutputStream(), 50);
+        $bar->finish();
+
+        rewind($output->getStream());
+        $this->assertEquals(
+            $this->generateOutput(' 50/50 [============================] 100%'),
+            stream_get_contents($output->getStream())
+        );
+    }
+
     public function testPercent()
     {
         $bar = new ProgressBar($output = $this->getOutputStream(), 50);
@@ -122,14 +188,29 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testStartWithMax()
+    {
+        $bar = new ProgressBar($output = $this->getOutputStream());
+        $bar->setFormat('%current%/%max% [%bar%]');
+        $bar->start(50);
+        $bar->advance();
+
+        rewind($output->getStream());
+        $this->assertEquals(
+            $this->generateOutput(' 0/50 [>---------------------------]').
+            $this->generateOutput(' 1/50 [>---------------------------]'),
+            stream_get_contents($output->getStream())
+        );
+    }
+
     public function testSetCurrentProgress()
     {
         $bar = new ProgressBar($output = $this->getOutputStream(), 50);
         $bar->start();
         $bar->display();
         $bar->advance();
-        $bar->setCurrent(15);
+        $bar->setProgress(15);
-        $bar->setCurrent(25);
+        $bar->setProgress(25);
 
         rewind($output->getStream());
         $this->assertEquals(
@@ -143,13 +224,12 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @expectedException        \LogicException
-     * @expectedExceptionMessage You must start the progress bar
      */
     public function testSetCurrentBeforeStarting()
     {
         $bar = new ProgressBar($this->getOutputStream());
-        $bar->setCurrent(15);
+        $bar->setProgress(15);
+        $this->assertNotNull($bar->getStartTime());
     }
 
     /**
@@ -160,8 +240,8 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
     {
         $bar = new ProgressBar($output = $this->getOutputStream(), 50);
         $bar->start();
-        $bar->setCurrent(15);
+        $bar->setProgress(15);
-        $bar->setCurrent(10);
+        $bar->setProgress(10);
     }
 
     public function testRedrawFrequency()
@@ -171,7 +251,7 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
 
         $bar->setRedrawFrequency(2);
         $bar->start();
-        $bar->setCurrent(1);
+        $bar->setProgress(1);
         $bar->advance(2);
         $bar->advance(2);
         $bar->advance(1);
@@ -200,7 +280,7 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
     {
         $bar = new ProgressBar($output = $this->getOutputStream(), 50);
         $bar->start();
-        $bar->setCurrent(25);
+        $bar->setProgress(25);
         $bar->clear();
 
         rewind($output->getStream());
@@ -299,10 +379,32 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testWithoutMax()
+    {
+        $output = $this->getOutputStream();
+
+        $bar = new ProgressBar($output);
+        $bar->start();
+        $bar->advance();
+        $bar->advance();
+        $bar->advance();
+        $bar->finish();
+
+        rewind($output->getStream());
+        $this->assertEquals(
+            rtrim($this->generateOutput('    0 [>---------------------------]')).
+            rtrim($this->generateOutput('    1 [->--------------------------]')).
+            rtrim($this->generateOutput('    2 [-->-------------------------]')).
+            rtrim($this->generateOutput('    3 [--->------------------------]')).
+            rtrim($this->generateOutput('    3 [============================]')),
+            stream_get_contents($output->getStream())
+        );
+    }
+
     public function testAddingPlaceholderFormatter()
     {
         ProgressBar::setPlaceholderFormatterDefinition('remaining_steps', function (ProgressBar $bar) {
-            return $bar->getMaxSteps() - $bar->getStep();
+            return $bar->getMaxSteps() - $bar->getProgress();
         });
         $bar = new ProgressBar($output = $this->getOutputStream(), 3);
         $bar->setFormat(' %remaining_steps% [%bar%]');
@@ -432,9 +534,9 @@ class ProgressBarTest extends \PHPUnit_Framework_TestCase
         );
     }
 
-    protected function getOutputStream($decorated = true)
+    protected function getOutputStream($decorated = true, $verbosity = StreamOutput::VERBOSITY_NORMAL)
     {
-        return new StreamOutput(fopen('php://memory', 'r+', false), StreamOutput::VERBOSITY_NORMAL, $decorated);
+        return new StreamOutput(fopen('php://memory', 'r+', false), $verbosity, $decorated);
     }
 
     protected function generateOutput($expected)