Skip to content
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

Add String#center #3924

Closed
wants to merge 1 commit into from
Closed

Add String#center #3924

wants to merge 1 commit into from

Conversation

ilja
Copy link

@ilja ilja commented Jan 21, 2017

This adds String#center: "hello".center(9, '-') => --hello--

Copy link
Member

@bcardiff bcardiff left a comment

Choose a reason for hiding this comment

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

I don't see a reason to share code between center and r/ljust as they are written now. The String.justify barely overlaps between these two cases.

I would say to either, handle the center logic directly in String#center and leave String.justify as is.

Or, change String.justify to determine prefix/postfix spaces at the beginning for all direction alignment so to the logic is shared across all cases.

@asterite
Copy link
Member

I agree with @bcardiff , it's better to keep the code clean and separate from the other methods

@straight-shoota
Copy link
Member

@ilja Are you still interested in meeting the requested changes?

@hutou
Copy link
Contributor

hutou commented Nov 17, 2019

A String.center method would be useful for me.
Is there any chance to see this PR validated ?

@straight-shoota straight-shoota added the pr:needs-work A PR requires modifications by the author. label Nov 17, 2019
@straight-shoota
Copy link
Member

The author hasn't responded in two years, so I expect this PR to be stalled. An implementation as suggested above would be very welcome. Feel free to submit a new PR for adding this!

@hutou
Copy link
Contributor

hutou commented Nov 18, 2019

Ok, I'll submit a PR soon.

@bcardiff
Copy link
Member

bcardiff commented Dec 9, 2019

Closed by #8557

@bcardiff bcardiff closed this Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:feature pr:needs-work A PR requires modifications by the author. topic:stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants