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

Fix align_center for multiline text with tight width #3

Merged
merged 1 commit into from Sep 10, 2018

Conversation

Projects
None yet
3 participants
@DannyBen
Copy link
Contributor

commented Sep 10, 2018

This PR fixes #align_center in cases where the text is a multi-line string, and the requested width is exactly or lower than the longest line in the string.

This code:

text = "Drawing some \ntext"
puts Strings.align(text, 13, direction: :center)

Created this output before the PR:

Drawing some
text
    text

And it now - correctly - creates this output:

Drawing some
    text

This was discovered in piotrmurach/tty-box#2

In addition, I have added a test case for it, and normalized the indentation in the align_spec.rb file - it sometimes had two spaces, sometimes one.

Specs are passing locally, and failing without the fix.

@coveralls

This comment has been minimized.

Copy link

commented Sep 10, 2018

Coverage Status

Coverage increased (+0.3%) to 97.196% when pulling b4884be on DannyBen:fix-align-center into c3c4acb on piotrmurach:master.

@DannyBen DannyBen changed the title fix align_center for multiline text with tight width Fix align_center for multiline text with tight width Sep 10, 2018

@piotrmurach piotrmurach merged commit c792c8c into piotrmurach:master Sep 10, 2018

4 checks passed

codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.3%) to 97.196%
Details
@piotrmurach

This comment has been minimized.

Copy link
Owner

commented Sep 10, 2018

Thanks ❤️

@DannyBen DannyBen deleted the DannyBen:fix-align-center branch Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.