Skip to content

Quickly fire up a console with 'zip' pre-loaded. #420

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

Merged
merged 1 commit into from
Feb 15, 2020

Conversation

hainesr
Copy link
Member

@hainesr hainesr commented Oct 30, 2019

I find myself testing things in irb enough that this has proved useful, so I decided to commit it.

It's purely a developer nicety so isn't stored in the gem.

@coveralls
Copy link

coveralls commented Oct 30, 2019

Coverage Status

Coverage remained the same at 95.674% when pulling 862892c on hainesr:console into 247fd43 on rubyzip:master.

Copy link
Member

@jdleesmiller jdleesmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this would be useful. One suggestion on the implementation.

Another possible implementation (based on https://stackoverflow.com/a/46941281):

#!/usr/bin/env sh
exec irb -I lib -r zip

bin/console Outdated
Comment on lines 3 to 4
lib = File.expand_path(File.join('..', 'lib'), __dir__)
$LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
require 'zip'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's possible to do this with require_relative now (I think since 1.9), without the path manipulation:

require_relative '../lib/zip'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always forget about require_relative. I'll change that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having now tried this, require_relative doesn't actually work, because everything in lib/zip.rb is required, so also relies on $LOAD_PATH being set correctly.

So I could fix this, but it would be a lot more involved that just changing this file. Happy to do so if you think it's worth it?

@hainesr
Copy link
Member Author

hainesr commented Dec 15, 2019

The problem with exec irb -I lib -r zip is that it doesn't take cwd into account. To be able to run both

  • ./bin/console
  • cd bin; ./console

you'd need something like exec irb -I lib:../lib -r zip, and then it still wouldn't be that robust. I'm tempted to prefer the more wordy ruby version.

@jdleesmiller
Copy link
Member

OK, the current script isn't bad, and I don't want to belabour the point, but to make sure we've gone through the options:

  • I tried it with require_relative '../lib/zip' and the resulting console seemed to work --- what didn't work when you tried it?
  • That is true about the shell script assuming it's run from the root. I have never found a particularly elegant solution for this problem, but my usual approach has been to rely on bash with something like
#!/usr/bin/env bash
exec irb -I $(dirname "${BASH_SOURCE[0]}")../lib -r zip

@hainesr
Copy link
Member Author

hainesr commented Feb 15, 2020

Sorry, forgot about this one for a while. The other reason I went the ruby (rather than shell) route, is that this mimics the current working of bundler: when you're starting a gem from scratch you can do bundler gem name and get a load of nice boilerplate created for you.

I've now gone back and looked at how bundler sets this file up and adjusted it accordingly. I think it's pretty neat now, and doesn't rely on shell variables and so on.

Copy link
Member

@jdleesmiller jdleesmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice 👍 .

@jdleesmiller jdleesmiller merged commit 5da87ef into rubyzip:master Feb 15, 2020
@hainesr hainesr deleted the console branch February 15, 2020 12:29
jdleesmiller added a commit that referenced this pull request Feb 16, 2020
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