-
Notifications
You must be signed in to change notification settings - Fork 121
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
IRB crashing on sorting completions if $LOAD_PATH
contains Pathname
#394
Comments
$LOAD_PATH
contains Pathname
In case it's confusing, the |
Yeah, I probably should've attach logs from a clean rails app. I described above, the issue is in having an instance of Pathname in $LOAD_PATH, instead of a string. |
I just bumped into the same issue. It can be reproduced by adding a e.g. # config/application.rb
class Application < Rails::Application
config.eager_load_paths << config.root.join('lib')
end |
Ruby will happily accept any entry in the class X
def to_str
__dir__
end
end
$LOAD_PATH << X.new
require 'some_file_in_current_directory' I think Line 63 in b248520
(gem_paths.to_a | $LOAD_PATH).sort_by { |path| String(path) rescue "" } . As far as I can tell that, matches what MRI does when processing the $LOAD_PATH with an extra bit of error handling so entries that can't be coerced to a String don't break IRB.
I can pull this into a PR, but I need to familiarize myself with the tests first. Having to modify |
Thanks for look into it. I think in this case something like this should be acceptable? $LOAD_PATH << pathname_path
# test
ensure
$LOAD_PATH.pop We can just dup and reassign the |
FYI, I created #400 to address this |
Description
Expected behavior: IRB proposes options to autocomplete the "require" method argument (or just doesn't crash).
Actual behavior: IRB crashes once I add any symbol after
require '
In my case, Rails
config.eager_load_paths
contains an instance ofPathname
and when I replace it with a String, the error disappear. Probably, we should convert all options to String before sorting.Result of irb_info
and in Docker:
Terminal Emulator
iTerm
Setting Files
No
The text was updated successfully, but these errors were encountered: