-
Notifications
You must be signed in to change notification settings - Fork 15.5k
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
ruby: return nil for out of bounds #332
Conversation
if (index < 0 && repeated_field->size > 0) { | ||
index = repeated_field->size + index; | ||
} | ||
return index; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not add this to the header file; it has been awhile but if I remember correctly header files are really for methods you want exposed vs internal-use-only methods.
anyway, let me know if there are any issues here, including styling and naming conventions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, file-internal helpers don't need to go in the header. Probably good to add a static
though (this keeps the symbol internal to the file, like an anonymous namespace in C++).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
|
||
$ gem install bundler | ||
$ bundle | ||
|
||
Then install the required Ruby gems (JRuby): | ||
|
||
$ jgem install bundler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is jgem
necessary even when using rvm? IIRC, I had managed to just gem install bundler
while in an environment that had selected JRuby via rvm, but it's been a while.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right; jgem
is no longer needed. I'll remove (running gem
was hanging, but there was a stuck thread; I reran and it worked fine)
ruby arrays don't throw an exception; they return nil. Lets do the same! this fix also includes the ability to use negative array indicies
#make sure we set the RepeatedField and can add to it | ||
m = TestMessage.new | ||
assert m.repeated_string == [] | ||
m.repeated_string << 'ok' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was not working in master
; see comment in RubyMessage
.
OK, this looks good; I'll go ahead and merge. Thanks! |
ruby: return nil for out of bounds
ruby returns nil if an array index is out of bounds
also allows negative indicies (again, to make it act more like a ruby lib)