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 Strings.truncate with length '1' #8

Merged
merged 1 commit into from Oct 7, 2019

Conversation

@slowbro
Copy link
Contributor

@slowbro slowbro commented Oct 4, 2019

Describe the change

Fixes an issue where passing a truncate value of 1 results in an unhandled exception.

[2] pry(main)> Strings.truncate('data', 1)
NoMethodError: undefined method `gsub' for nil:NilClass
from /usr/home/kschiesser/.rvm/gems/ruby-2.6.3/gems/strings-ansi-0.1.0/lib/strings/ansi.rb:29:in `sanitize'
[3] pry(main)> wtf
Exception: NoMethodError: undefined method `gsub' for nil:NilClass
--
0: /usr/home/kschiesser/.rvm/gems/ruby-2.6.3/gems/strings-ansi-0.1.0/lib/strings/ansi.rb:29:in `sanitize'
1: /usr/home/kschiesser/code/strings/lib/strings/truncate.rb:103:in `display_width'
2: /usr/home/kschiesser/code/strings/lib/strings/truncate.rb:74:in `shorten'
3: /usr/home/kschiesser/code/strings/lib/strings/truncate.rb:62:in `truncate'
4: /usr/home/kschiesser/code/strings/lib/strings.rb:99:in `truncate'

vs

[2] pry(main)> Strings.truncate('data', 1)
=> "\u2026"

Why are we doing this?

Unhandled exceptions are generally bad :)
Related to piotrmurach/tty-table#20

Benefits

Better handling for all cases on Strings::Truncate

Drawbacks

None that I can see.

Requirements

Put an X between brackets on each line if you have done the item:
[x] Tests written & passing locally?
[x] Code style checked?
[x] Rebased with master branch?
[x] Documentaion updated?

… blank array immediately
@coveralls
Copy link

@coveralls coveralls commented Oct 4, 2019

Coverage Status

Coverage increased (+0.06%) to 97.356% when pulling e5382dd on slowbro:truncate-zero-length into 6ff3143 on piotrmurach:master.

@coveralls
Copy link

@coveralls coveralls commented Oct 4, 2019

Coverage Status

Coverage increased (+0.05%) to 97.348% when pulling e5382dd on slowbro:truncate-zero-length into 6ff3143 on piotrmurach:master.

@piotrmurach piotrmurach merged commit 68e69c5 into piotrmurach:master Oct 7, 2019
4 checks passed
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.05%) to 97.348%
Details
@piotrmurach
Copy link
Owner

@piotrmurach piotrmurach commented Oct 7, 2019

Thanks ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.