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

Drop hard dependency on RDoc. #393

Merged
merged 1 commit into from Aug 23, 2022
Merged

Conversation

voxik
Copy link
Contributor

@voxik voxik commented Aug 23, 2022

This has been introduced in 0267004, but it seems that this is just be mistake, otherwise the later handling of LoadError would not be needed.

Of course I might misunderstand the code. But if that is the case, I still think that RDoc should not be loaded by default during startup, but later when really needed.

This has been introduced in 0267004,
but it seems that this is just be mistake, otherwise the later handling
of `LoadError` would not be needed.
@hsbt
Copy link
Member

hsbt commented Aug 23, 2022

👍 Nice catch. I agree to remove the top of require "rdoc".

@hsbt hsbt merged commit 54c8df0 into ruby:master Aug 23, 2022
@voxik voxik deleted the drop-rdoc-hard-dependency branch August 23, 2022 19:35
@junaruga
Copy link
Member

junaruga commented Aug 25, 2022

I think this PR's change is wrong, because there is no logic for begin .. require 'rdoc' .. rescue LoadError .. end any more on the "latest master"'s. input-method.rb. The logic was deleted by 5f749c6 .
And this file is using RDoc.

$ grep -ri rdoc lib/irb/input-method.rb
      driver = RDoc::RI::Driver.new(options)
        rescue RDoc::RI::Driver::NotFoundError
      rescue RDoc::RI::Driver::NotFoundError
        doc = RDoc::Markup::Document.new
        rescue RDoc::RI::Driver::NotFoundError
      formatter = RDoc::Markup::ToAnsi.new

@junaruga
Copy link
Member

Searching parts requiring rdoc, there are 4 files requiring it.

$ grep -ri rdoc lib/
lib/irb/cmd/help.rb:        require 'rdoc/ri/driver'
lib/irb/cmd/help.rb:        opts = RDoc::RI::Driver.process_args([])
lib/irb/cmd/help.rb:        IRB::ExtendCommand::Help.const_set(:Ri, RDoc::RI::Driver.new(opts))
lib/irb/cmd/help.rb:            rescue RDoc::RI::Error
lib/irb/completion.rb:        require 'rdoc'
lib/irb/completion.rb:      RDocRIDriver ||= RDoc::RI::Driver.new
lib/irb/completion.rb:        out = RDoc::Markup::Document.new
lib/irb/completion.rb:            RDocRIDriver.add_method(out, m)  
lib/irb/completion.rb:          rescue RDoc::RI::Driver::NotFoundError
lib/irb/completion.rb:        RDocRIDriver.display(out)
lib/irb/completion.rb:          RDocRIDriver.display_names([namespace])
lib/irb/completion.rb:        rescue RDoc::RI::Driver::NotFoundError
lib/irb/easter-egg.rb:          require "rdoc"
lib/irb/easter-egg.rb:          RDoc::RI::Driver.new.page do |io|
lib/irb/input-method.rb:      driver = RDoc::RI::Driver.new(options)
lib/irb/input-method.rb:        rescue RDoc::RI::Driver::NotFoundError
lib/irb/input-method.rb:      rescue RDoc::RI::Driver::NotFoundError
lib/irb/input-method.rb:        doc = RDoc::Markup::Document.new
lib/irb/input-method.rb:        rescue RDoc::RI::Driver::NotFoundError
lib/irb/input-method.rb:      formatter = RDoc::Markup::ToAnsi.new

And in lib/irb/cmd/help.rb, lib/irb/completion.rb and lib/irb/easter-egg.rb, the require "rdoc" or require 'rdoc/ri/driver' are executed in a small scope handling the error by rescue LoadError. So, I think the 5f749c6 might be mistake. And this PR's commit is ok.

@voxik
Copy link
Contributor Author

voxik commented Aug 25, 2022

Oh, wow, where I left my eyes? 🙈 Maybe I got confused by the commit I have referenced without checking further? Sorry.

But still, I'd love to see the hard dependency removed.

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.

None yet

3 participants