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 RDoc in input-method.rb again in a limited scope. #395

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

junaruga
Copy link
Member

@junaruga junaruga commented Aug 25, 2022

This PR is a follow up by #393 (comment).

RDoc is implemented as soft dependency in IRB. See how the rdoc is required in the files. I reverted the commit below.

$ grep -ril rdoc lib/
lib/irb/cmd/help.rb
lib/irb/completion.rb
lib/irb/easter-egg.rb
lib/irb/input-method.rb

Ideally I wanted to run test the case of 'no rdoc' with the logic in test/lib/helper.rb below. But I couldn't do it. Perhaps the Kernel#require was overridden by Rubygems or Bundler in the process of running unit-test.

if ENV["NO_RDOC"] == "true"
  module Kernel
    alias_method :old_require, :require
    def require(name)
      raise LoadError, "cannot load such file -- rdoc (test)" if name == "rdoc"
      old_require(name)
    end
  end
end

require 'irb'

Revert "Remove require in signal handler to avoid ThreadError"

This reverts commit 5f749c6.

RDoc is implemented as soft dependency in IRB. See how the rdoc is required in
the files. I reverted the commit below.

```
$ grep -ril rdoc lib/
lib/irb/cmd/help.rb
lib/irb/completion.rb
lib/irb/easter-egg.rb
lib/irb/input-method.rb
```

---

Revert "Remove `require` in signal handler to avoid ThreadError"

This reverts commit 5f749c6.
@junaruga junaruga requested a review from aycabta August 25, 2022 23:15
@hsbt hsbt merged commit b248520 into ruby:master Aug 26, 2022
@hsbt
Copy link
Member

hsbt commented Aug 26, 2022

👍 Nice catch, Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants