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 method #8557

Merged
merged 3 commits into from
Dec 9, 2019
Merged

Add String#center method #8557

merged 3 commits into from
Dec 9, 2019

Conversation

hutou
Copy link
Contributor

@hutou hutou commented Dec 5, 2019

String#center(width, char) returns a new string of length width with original string centered, and padded with char (default = space)

src/string.cr Outdated

unless left
buffer.copy_from(to_unsafe, bytesize)
Intrinsics.memcpy(buffer.as(Void*), to_unsafe.as(Void*), bytesize, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you might be able to use Pointer.copy_from

Suggested change
Intrinsics.memcpy(buffer.as(Void*), to_unsafe.as(Void*), bytesize, false)
buffer.copy_from(to_unsafe, bytesize)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now this is diff noise so this might be outside the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this "better" than memcpy ?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same as memcpy, but shorter.

It's fine to leave this as-is for this PR.

@straight-shoota
Copy link
Member

This continues the previous PR #3924 (stalled).

src/string.cr Outdated
when .> 0
leftpadding, rightpadding = padding, 0
else
leftpadding = padding >> 1
Copy link
Contributor

Choose a reason for hiding this comment

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

// 2, please

there's no need for manual pinhole optimizations the compiler is perfectly capable of.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, done !

src/string.cr Outdated

unless left
buffer.copy_from(to_unsafe, bytesize)
Intrinsics.memcpy(buffer.as(Void*), to_unsafe.as(Void*), bytesize, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

It's the same as memcpy, but shorter.

It's fine to leave this as-is for this PR.

@RX14
Copy link
Contributor

RX14 commented Dec 7, 2019

Thank you!

@bcardiff bcardiff added this to the 0.32.0 milestone Dec 9, 2019
@bcardiff bcardiff merged commit 70f04ab into crystal-lang:master Dec 9, 2019
@bcardiff bcardiff mentioned this pull request Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants