-
Notifications
You must be signed in to change notification settings - Fork 605
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 chunk_while #3579
Add chunk_while #3579
Conversation
@@ -212,6 +212,12 @@ def slice_when(&block) | |||
end | |||
end | |||
|
|||
def chunk_while(&block) | |||
raise ArgumentError, "wrong number of arguments (0 for 1)" unless block_given? |
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 doesn't strike me as a particularly accurate or helpful error message.
Even if it's the same as the one given by MRI, I'd prefer to see it made clear that a block is required. The current error message makes it look like a regular argument is required.
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.
Hm, interesting that this doesn't return an Enumerator when no block is given. I see this in the 2.3.0 ri chunk_while
docs:
= .chunk_while
(from ruby core)
=== Implementation from Enumerable
------------------------------------------------------------------------------
enum.chunk_while {|elt_before, elt_after| bool } -> an_enumerator
------------------------------------------------------------------------------
Assuming this should raise an ArgumentError, I agree with @jemc here, we should use a more helpful error message.
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.
Ah, I see now why the block is required. So yeah, we should fix the error message.
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.
What should the error be? How about "Block required"?
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.
Or how about "no block given"? For consistency with chunk
in https://github.com/rubinius/rubinius/blob/master/kernel/common/enumerable.rb#L8
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.
👍
Changed error message to "no block given". |
def chunk_while(&block) | ||
raise ArgumentError, "no block given" unless block_given? | ||
|
||
slice_when { |before, after| !(block.call(before, after)) } |
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.
The explicit block isn't needed here, this can just be:
!(yield before, after)
Thanks! |
No description provided.