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

bytesize on nil #22

Merged
merged 2 commits into from
Jul 3, 2016
Merged

bytesize on nil #22

merged 2 commits into from
Jul 3, 2016

Conversation

ondra-m
Copy link
Contributor

@ondra-m ondra-m commented Jul 3, 2016

I have error NoMethodError: undefined method bytesize for nil:NilClass on

  cmd = TTY::Command.new
  cmd.run "wget http://ftp.ruby-lang.org/pub/ruby/2.3/ruby-2.3.0.tar.gz"

Problem is that value[offset..-1] return nil. Offsett is equal to value size.

@coveralls
Copy link

coveralls commented Jul 3, 2016

Coverage Status

Coverage increased (+0.2%) to 91.925% when pulling fd4bf20 on ondra-m:bytesize-on-nil into a3b9e24 on piotrmurach:master.

@piotrmurach
Copy link
Owner

Hi Ondrej, thanks for fixing the bug, would you mind creating a test case to trap this problem in truncator_spec?

@coveralls
Copy link

coveralls commented Jul 3, 2016

Coverage Status

Coverage increased (+0.2%) to 91.925% when pulling 9eed29c on ondra-m:bytesize-on-nil into a3b9e24 on piotrmurach:master.

@ondra-m
Copy link
Contributor Author

ondra-m commented Jul 3, 2016

I think this is just workaround. Would not it be better to

  1. Use .byteslice instead of .[]
  2. Or use .[] but without .bytesize (just .size)

@piotrmurach piotrmurach merged commit 6338a08 into piotrmurach:master Jul 3, 2016
piotrmurach added a commit that referenced this pull request Jul 3, 2016
@piotrmurach
Copy link
Owner

Thanks for providing test case and suggestions for fixing the issue, much appreciated!

Decided to go with the first option and released v0.2.0. Enjoy!

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

Successfully merging this pull request may close these issues.

3 participants