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

require should accept a Pathname object like Kernel#require #663

Merged
merged 2 commits into from Oct 8, 2013

Conversation

tenderlove
Copy link
Contributor

Hi,

Kernel#require accepts a Pathname object (actually any object that responds to to_path), but Rubygems' require does not. Here is a demo:

$ ruby -rpathname -e'require Pathname.new("psych")'
$ ruby -rpathname -e'require "active_record"'
$ ruby -rpathname -e'require Pathname.new("active_record")'
/Users/aaron/.rbenv/versions/2.1.0-dev/lib/ruby/site_ruby/2.1.0/rubygems/core_ext/kernel_require.rb:109:in `end_with?': no implicit conversion of Pathname into String (TypeError)
    from /Users/aaron/.rbenv/versions/2.1.0-dev/lib/ruby/site_ruby/2.1.0/rubygems/core_ext/kernel_require.rb:109:in `rescue in require'
    from /Users/aaron/.rbenv/versions/2.1.0-dev/lib/ruby/site_ruby/2.1.0/rubygems/core_ext/kernel_require.rb:35:in `require'
    from -e:1:in `<main>'
$

This pull request has a test case. I haven't looked in to fixing it. :-)

@tenderlove
Copy link
Contributor Author

Hi, this patch fixes my test, but I don't know if it's The Right Way™:

diff --git a/lib/rubygems/core_ext/kernel_require.rb b/lib/rubygems/core_ext/kernel_require.rb
index 0416644..84bb03f 100755
--- a/lib/rubygems/core_ext/kernel_require.rb
+++ b/lib/rubygems/core_ext/kernel_require.rb
@@ -38,6 +38,8 @@ module Kernel
   def require path
     RUBYGEMS_ACTIVATION_MONITOR.enter

+    path = path.to_path if path.respond_to? :to_path
+
     spec = Gem.find_unresolved_default_spec(path)
     if spec
       Gem.remove_unresolved_default_spec(spec)

@tenderlove
Copy link
Contributor Author

I updated my PR with my fix. ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️

drbrain added a commit that referenced this pull request Oct 8, 2013
Kernel#require accept a Pathname like the built-in
@drbrain drbrain merged commit eeeb625 into rubygems:master Oct 8, 2013
drbrain added a commit that referenced this pull request Oct 8, 2013
drbrain added a commit that referenced this pull request Oct 8, 2013
drbrain added a commit that referenced this pull request Oct 8, 2013
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.

None yet

2 participants