Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Console] Add Helper::width() and Helper::length()#40698

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

Merged
fabpot merged 2 commits intosymfony:5.2fromNyholm:helper-strlengh
Apr 8, 2021

Conversation

@Nyholm
Copy link
Member

@NyholmNyholm commentedApr 3, 2021•
edited
Loading

QA
Branch?5.2
Bug fix?yes
New feature?no
Deprecations?no
TicketsClose#40697Fix#40634,fix#40635
LicenseMIT
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 (ieclose#40697)

FYI@grasmash, I used your tests from#40635

@chalasr
Copy link
Member

Fromhttps://semver.org/#spec-item-7

Minor version Y (x.Y.z | x > 0) MUST be incremented if new, backwards compatible functionality is introduced to the public API

Should we mark the new methods as@internal until 5.3?

@Nyholm
Copy link
MemberAuthor

Oh, thank you. The PR is updated

@nicolas-grekas
Copy link
Member

nicolas-grekas commentedApr 5, 2021•
edited
Loading

about the name, what about length() and width()?

On 5.x, I'd be in favor of deprecating the current methods, including strlenWithoutDecoration(), and tell ppl to use removeDecoration instead.

if you agree with this plan, we could skip using strlenWithoutDecoration() in this PR.

WDYT?

chalasr and Nyholm reacted with thumbs up emoji

@chalasr
Copy link
Member

Agree 👍

$string =self::removeDecoration($formatter,$string);

if (preg_match('//u',$string)) {
return (newUnicodeString($string))->width(true);
Copy link
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thetrue here is for "remove ansi decoration", but that is already done inself::removeDecoration.

@Nyholm
Copy link
MemberAuthor

Thank you for the reviews. The PR is updated.

@jderusse
Copy link
Member

If we deprecatedstrlen, then replacement methods should not be internal. right?

@chalasr
Copy link
Member

They will no longer be internal as soon as they will be publicly introduced in 5.3, at the same time we will deprecatestrlen() (this is a 5.2 bugfix)

jderusse, derrabus, and Nyholm reacted with thumbs up emoji

@NyholmNyholm changed the title[Console] Add Helper::strwidth() and Helper::strlength()[Console] Add Helper::width() and Helper::length()Apr 6, 2021
@NyholmNyholm added the Ready labelApr 7, 2021
@fabpot
Copy link
Member

Thank you@Nyholm.

@fabpotfabpot merged commit74056a7 intosymfony:5.2Apr 8, 2021
@Nyholm
Copy link
MemberAuthor

Great. Thank you for merging

@NyholmNyholm deleted the helper-strlengh branchApril 8, 2021 06:56
fabpot added a commit that referenced this pull requestApr 9, 2021
…ength() (Nyholm)This PR was squashed before being merged into the 5.3-dev branch.Discussion----------[Console] Deprecate Helper::strlen() for width() and length()| Q             | A| ------------- | ---| Branch?       | 5.2| Bug fix?      | yes| New feature?  | yes| Deprecations? | yes| Tickets       | Follow up form#40698| License       | MIT| Doc PR        |This PR will deprecated `Helper::strlen()` since it is actually calculating the width. I remove the `@internal` on `Helper::width()` and a `Helper::length()`. I will also deprecate `Helper::strlenWithoutDecoration()` because you should use `Helper::removeDecoration()` instead.Commits-------3c24aa9 [Console] Deprecate Helper::strlen() for width() and length()
@fabpotfabpot mentioned this pull requestMay 1, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@jderussejderussejderusse approved these changes

@derrabusderrabusderrabus approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

8 participants

@Nyholm@chalasr@nicolas-grekas@jderusse@fabpot@derrabus@carsonbot@grasmash
[8]ページ先頭
©2009-2025 Movatter.jp