Remove unused InputMethod#initialize

[st0012]

The constructor takes a file_name argument, but it is never used. The only input method that needs a file is FileInputMethod, which has its own constructor to take a file object directly.

So the constructor in InputMethod is not needed and its child classes don't need to call super in their constructors.

I also dropped the attribute accessors for file_name as they're not used either.

[ima1zumi]

In a search on GitHub, InputMethod.new was found to be used in two repositories.
I believe it would be better to show a deprecation warning before removal.

https://github.com/search?q=%22+InputMethod.new%22+language%3ARuby&type=code

[st0012]

It's thoughtful to think about external dependants when dropping things 👍 But in this case I think we should not be blocked by them because:

1. irb_remote hasn't received any update for 5 years and it already passes incorrect number of arguments to InputMethod.new. So it's already broken.
2. In irb-power_assert, it's used to build an IRB environment for testing incorrectly. I say incorrectly because InputMethod is an abstract class. So in practice it should have not been initialised directly. If the author switch to StdioInputMethod instead, that may be better and more stable.

But the root problem is that we and previous maintainers don't define what's official APIs, what's not, and ways/principles to achieve what they need without using private APIs.

My current categorisation is:

• Public APIs:
  • Methods to start an IRB session: binding.irb and IRB.start
  • IRB commands
  • (Maybe more)
• Should be private but unfortunately got exposed extensively APIs:
  • IRB::ExtendCommandBundle
    • I want to fix it with Support command extension APIs #513 and Support helper method registration #624
  • IRB::Irb
• Private APIs:
  • All the things that are not included in the above categories

So IMO InputMethod, or even its child classes like RelineInputMethod...etc., should be considered private. Otherwise, our work on refactoring IRB would be seriously affected.

What are your thoughts? @tompng @hasumikin

[ima1zumi]

Thanks for the explanation. I understand, and it does not seem to be a problem to delete InputMethod.new.
However, I think we should carefully migrate the other InputMethods since they are widely used.
Not all of the code is IRB-related, but there seems to be a lot there. (Not sure if they have been active recently)
https://github.com/search?q=InputMethod.new+language%3ARuby+&type=code

[tompng]

I think the refactor looks good. InputMethod should not have FileInputMethod specific attribute.

My opinion for public and private are:

Sub classes: IMO should be private forever. There are alternatives.

RelineInputMethod # Only used in ruby/irb and it's forked repository
StdioInputMethod # Some repository using this is too old. last update: 12 years ago

# Alternative of IRB::Irb.new(workspace, FileInputMethod.new(file_name))
IRB::Irb.new(workspace, file_name)

# Alternative of IRB::Irb.new(workspace, ReadlineInputMethod.new)
IRB.conf[:USE_MULTILINE] = false

InputMethod: Maybe we can make it public in the future. But now, there is no way to set output method. It's hard to implement something like remote IRB using socket because users can only change input. So the use case of custom input method is very limited.

IRB::Irb#initialize had ouput_method but it was unused/unimplemented and removed in 53f7769

[kachick]

Thanks for your consideration, I was facing CI failures with these changes, but this discussion helps me a lot.
I just replaced s/InputMethod/IRB::StdioInputMethod/ for now.

Feel free to leave care about irb-power-assert anymore, I will catch up IRB changes in #673 if needed.

[st0012]

@kachick Thank you for the feedback and sorry for breaking your project's CI.
After taking a closer look, I think the test setup actually doesn't need to specify an input method at all. So I opened kachick/irb-power_assert#114 and I hope it will simplify its maintenance in the long run 🙂