-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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 the Table
, it feels like it should be "width" instead of "length". But "length" is the one working".
@@ -513,7 +513,7 @@ private static function initPlaceholderFormatters(): array | |||
$completeBars = $bar->getBarOffset(); | |||
$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.
@@ -511,7 +511,8 @@ private function renderCell(array $row, int $column, string $cellFormat): string | |||
return sprintf($style->getBorderFormat(), str_repeat($style->getBorderChars()[2], $width)); | |||
} | |||
|
|||
$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
to
7484158
Compare
We cannot deprecate something in 5.2 as it's a maintenance branch, it should be done on 5.x. |
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? |
7484158
to
c9c3c01
Compare
…, 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 #40697 Fix #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 #40695 With this PR we dont have to revert the emoji issue (ie close #40697) FYI @grasmash, I used your tests from #40635 Commits ------- d9ea4c5 Add test. dc02ab3 [Console] Add Helper::strwidth() and Helper::strlength()
c9c3c01
to
5679e99
Compare
5e65d3b
to
b883112
Compare
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.
6ca1bf6
to
3c24aa9
Compare
Thank you @Nyholm. |
Thank you for merging |
This PR will deprecated
Helper::strlen()
since it is actually calculating the width. I remove the@internal
onHelper::width()
and aHelper::length()
. I will also deprecateHelper::strlenWithoutDecoration()
because you should useHelper::removeDecoration()
instead.