Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Console] Fix bar width with multilines ProgressBar's format#21958
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
nicolas-grekas commentedMar 15, 2017
I guess needs to merged in a lower branch thought |
maidmaid commentedMar 15, 2017
Should I rebase the PR@nicolas-grekas? |
nicolas-grekas commentedMar 15, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
If you don't mind, please do, and change the base branch here yes. |
maidmaid commentedMar 15, 2017
Rebase onto 2.7 has some conflicts. Create a new PR on 2.7 seems a better idea. |
nicolas-grekas commentedMar 15, 2017
OK for me |
maidmaid commentedMar 15, 2017
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? |
fabpot commentedMar 15, 2017
We only merge fixes in the lowest maintained branch (we don't backport fixes). |
maidmaid commentedMar 15, 2017
Ok, so I will create new PR on 2.7 an other time. |
maidmaid commentedMar 18, 2017
In fact, the resizing of ProgressBar appearsfrom 3.2 and not before. So, I propose to rebase on 3.2. |
nicolas-grekas commentedMar 20, 2017
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
rtrim($subLine, "\r") to handle "\r\n" line endings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'd suggest renaming maxLineLength to linesWidth
fabpot commentedMar 22, 2017
Apparently, tests are still broken on Windows |
33029b4 to8349d1dCompareThis reverts commitfb8e8fa.
maidmaid commentedMar 23, 2017
Need some help to fix tests please. |
| publicfunctiontestBarWidthWithMultilineFormat() | ||
| { | ||
| putenv('COLUMNS=10'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
this must be reset at the end of the test to avoid affecting other tests (including when the test fails, so usetry/finally)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thank you@stof, I will check that.
maidmaid commentedApr 12, 2017
Tests are now green :) |
maidmaid commentedApr 25, 2017
PR ready. |
fabpot commentedApr 25, 2017
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
Uh oh!
There was an error while loading.Please reload this page.
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.