Return an Enumerator for #each_key without a block #2

Merged
merged 1 commit into from Dec 13, 2014

Conversation

Projects
None yet
2 participants
@joeyates
Contributor

joeyates commented Dec 13, 2014

  • Makes the implementation more conformant with MRI,
  • I've made a Pull Request to MRI Ruby (ruby/ruby#783) adding a test for the same case,
  • With Ruby >= 2 it would be etter to return a lazy enumerator.
Return an Enumerator for #each_key without a block
* Makes the implementation more conformant with MRI
@@ -149,11 +149,17 @@ def self.each_pair(file)
#Iterates over each key in the _file_.
def self.each_key(file)
+ keys = []

This comment has been minimized.

@presidentbeef

presidentbeef Dec 13, 2014

Owner

-Any reason to not check block_given? at the beginning and return self.keys.each? Seems simpler.-

Edit: haha I don't know what I'm talking about...never mind.

@presidentbeef

presidentbeef Dec 13, 2014

Owner

-Any reason to not check block_given? at the beginning and return self.keys.each? Seems simpler.-

Edit: haha I don't know what I'm talking about...never mind.

presidentbeef added a commit that referenced this pull request Dec 13, 2014

Merge pull request #2 from joeyates/return_enumerator_for_each_key_wi…
…thout_block

Return an Enumerator for #each_key without a block

@presidentbeef presidentbeef merged commit a9b451d into presidentbeef:master Dec 13, 2014

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Dec 13, 2014

Owner

Thanks, Joe!

Are you actually using this library? I kind of assumed anyone who installed it was just confused...

I'll push a new gem version.

Owner

presidentbeef commented Dec 13, 2014

Thanks, Joe!

Are you actually using this library? I kind of assumed anyone who installed it was just confused...

I'll push a new gem version.

@joeyates

This comment has been minimized.

Show comment
Hide comment
@joeyates

joeyates Dec 14, 2014

Contributor

I hope I'm not confused...

I'm using https://github.com/y10k/rims which depends on the MRI Ruby builtin. As I didn't have GDBM installed when I compiled Ruby, I had to install libgdbm-dev and then reinstall Ruby to use it. I'm sure I'm not the first to have to do this.

If this gem became the replacement for the builtin an GDBM was removed from MRI core, it would be good - one less dependency. It might be worth proposing as much to the MRI maintainers.

Contributor

joeyates commented Dec 14, 2014

I hope I'm not confused...

I'm using https://github.com/y10k/rims which depends on the MRI Ruby builtin. As I didn't have GDBM installed when I compiled Ruby, I had to install libgdbm-dev and then reinstall Ruby to use it. I'm sure I'm not the first to have to do this.

If this gem became the replacement for the builtin an GDBM was removed from MRI core, it would be good - one less dependency. It might be worth proposing as much to the MRI maintainers.

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Dec 14, 2014

Owner

I worry some people think this is how you get gdbm because the gem is named "gdbm" - but you need the actual gdbm C library installed no matter what.

Owner

presidentbeef commented Dec 14, 2014

I worry some people think this is how you get gdbm because the gem is named "gdbm" - but you need the actual gdbm C library installed no matter what.

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Dec 14, 2014

Owner

I released version 1.3.0 of the gem.

Please let me know if I can help with anything else! I'm just glad someone found this useful.

Owner

presidentbeef commented Dec 14, 2014

I released version 1.3.0 of the gem.

Please let me know if I can help with anything else! I'm just glad someone found this useful.

@joeyates

This comment has been minimized.

Show comment
Hide comment
@joeyates

joeyates Dec 14, 2014

Contributor

Thanks.

I'll try bugging the MRI maintainers about removing GDBM from the standard library. Who knows.

Contributor

joeyates commented Dec 14, 2014

Thanks.

I'll try bugging the MRI maintainers about removing GDBM from the standard library. Who knows.

@presidentbeef

This comment has been minimized.

Show comment
Hide comment
@presidentbeef

presidentbeef Dec 15, 2014

Owner

I suspect an FFI-based library for GDBM would need to use Fiddle instead of relying on the FFI gem.

Owner

presidentbeef commented Dec 15, 2014

I suspect an FFI-based library for GDBM would need to use Fiddle instead of relying on the FFI gem.

Repository owner locked and limited conversation to collaborators May 20, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.