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] Deprecate Helper::strlen() for width() and length()#40695
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
Nyholm left a comment
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.
There are a lot of changes in this PR, but only the two lines I've marked below is significant, the others are just renaming.
What I fail to understand is that in theTable, it feels like it should be "width" instead of "length". But "length" is the one working".
| $display =str_repeat($bar->getBarCharacter(),$completeBars); | ||
| if ($completeBars <$bar->getBarWidth()) { | ||
| $emptyBars =$bar->getBarWidth() -$completeBars - Helper::strlenWithoutDecoration($output->getFormatter(),$bar->getProgressCharacter()); | ||
| $emptyBars =$bar->getBarWidth() -$completeBars - Helper::strlengthWithoutDecoration($output->getFormatter(),$bar->getProgressCharacter()); |
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.
Note this change.
| $width += Helper::strlen($cell) - Helper::strlenWithoutDecoration($this->output->getFormatter(),$cell); | ||
| $x =2; | ||
| $width += Helper::strlength($cell) - Helper::strlengthWithoutDecoration($this->output->getFormatter(),$cell); |
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.
Note these two changes.
27228d3 to7484158Comparechalasr commentedApr 3, 2021
We cannot deprecate something in 5.2 as it's a maintenance branch, it should be done on 5.x. |
Nyholm commentedApr 3, 2021
Thank you. I was hesitant about this. Are we okey with reverting#40524, adding that PR as a feature to 5.x and then merge this PR in 5.x? |
…, grasmash)This PR was merged into the 5.2 branch.Discussion----------[Console] Add Helper::width() and Helper::length()| Q | A| ------------- | ---| Branch? | 5.2| Bug fix? | yes| New feature? | no| Deprecations? | no| Tickets |Close#40697Fix#40634,fix#40635| License | MIT| Doc PR |This PR will add add a Helper::strwidth() and a Helper::strlength(). Same with with the Helper::strlenWithoutDecoration(). It does not deprecate anything. That is done in#40695With this PR we dont have to revert the emoji issue (ieclose#40697)FYI@grasmash, I used your tests from#40635Commits-------d9ea4c5 Add test.dc02ab3 [Console] Add Helper::strwidth() and Helper::strlength()
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
chalasr left a comment
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 for taking care of this.
fabpot commentedApr 9, 2021
Thank you@Nyholm. |
Nyholm commentedApr 9, 2021
Thank you for merging |
Uh oh!
There was an error while loading.Please reload this page.
This PR will deprecated
Helper::strlen()since it is actually calculating the width. I remove the@internalonHelper::width()and aHelper::length(). I will also deprecateHelper::strlenWithoutDecoration()because you should useHelper::removeDecoration()instead.