[Console] Fix bar width with multilines ProgressBar's format #21958
Conversation
|
I guess needs to merged in a lower branch thought |
|
Should I rebase the PR @nicolas-grekas? |
|
If you don't mind, please do, and change the base branch here yes. |
|
Rebase onto 2.7 has some conflicts. Create a new PR on 2.7 seems a better idea. |
|
OK for me |
|
Fix this onto 2.7 takes more time than expected. Could you merge this PR and later I will create new PR for 2.7? |
|
We only merge fixes in the lowest maintained branch (we don't backport fixes). |
|
Ok, so I will create new PR on 2.7 an other time. |
|
In fact, the resizing of ProgressBar appears from 3.2 and not before. So, I propose to rebase on 3.2. |
|
Let's go for 3.2. |
| $lineLength = Helper::strlenWithoutDecoration($this->output->getFormatter(), $line); | ||
| // gets string length for each sub line with multiline format | ||
| $linesLength = array_map(function ($subLine) { | ||
| return Helper::strlenWithoutDecoration($this->output->getFormatter(), $subLine); |
There was a problem hiding this comment.
rtrim($subLine, "\r") to handle "\r\n" line endings
There was a problem hiding this comment.
AppVeyor tests are broken because of this?
| return Helper::strlenWithoutDecoration($this->output->getFormatter(), $subLine); | ||
| }, explode("\n", $line)); | ||
|
|
||
| $maxLineLength = max($linesLength); |
There was a problem hiding this comment.
I'd suggest renaming maxLineLength to linesWidth
maidmaid
force-pushed
the
fixprogressbar
branch
from
b0cba85 to
8a21fe4
Compare
|
Apparently, tests are still broken on Windows |
maidmaid
force-pushed
the
fixprogressbar
branch
2 times, most recently
from
33029b4 to
8349d1d
Compare
maidmaid
force-pushed
the
fixprogressbar
branch
from
76da8f2 to
b0c3a35
Compare
This reverts commit fb8e8fa.
|
Need some help to fix tests please. |
|
|
||
| public function testBarWidthWithMultilineFormat() | ||
| { | ||
| putenv('COLUMNS=10'); |
There was a problem hiding this comment.
this must be reset at the end of the test to avoid affecting other tests (including when the test fails, so use try/finally)
There was a problem hiding this comment.
Thank you @stof, I will check that.
maidmaid
force-pushed
the
fixprogressbar
branch
from
201a3c9 to
2a2a78c
Compare
|
Tests are now green :) |
|
PR ready. |
|
Thank you @maidmaid. |
…at (maidmaid) This PR was squashed before being merged into the 3.2 branch (closes #21958). Discussion ---------- [Console] Fix bar width with multilines ProgressBar's format | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - The bar width is badly recalculated when we use multilines (``\n``) and bar placeholer (``%bar%``) in ProgressBar's format. The bar width is reduced because multilines is considered as a big oneline. This PR fixes this. Commits ------- 3d58ab5 [Console] Fix bar width with multilines ProgressBar's format
The bar width is badly recalculated when we use multilines (
\n) and bar placeholer (%bar%) in ProgressBar's format. The bar width is reduced because multilines is considered as a big oneline. This PR fixes this.