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

Delete never used cli.rb #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Delete never used cli.rb #91

wants to merge 1 commit into from

Conversation

neves
Copy link

@neves neves commented Sep 26, 2015

cli.rb is loaded by pry at startup, looking for cli options to add to pry command options.
This file is slowing pry startup by 50%, but it's never used.
It's ok to not exist, since pry checks it:
https://github.com/pry/pry/blob/master/lib/pry/plugins.rb#L38

The same was proposed to pry-byebug:
deivid-rodriguez/pry-byebug#76

cli.rb is loaded by pry at startup, looking for cli options to add to pry command options.
This file is slowing pry startup by 50%, but it's never used.
It's ok to not exist, since pry checks it:
https://github.com/pry/pry/blob/master/lib/pry/plugins.rb#L38

The same was proposed to pry-byebug
@kyrylo
Copy link
Collaborator

kyrylo commented Sep 26, 2015

I don't remember why it was added, but I guess there's a good reason for it. How did you figure out that it slows the startup time by 50%?

@ConradIrwin
Copy link
Owner

I think it was to ensure that interception loaded ahead of time. In todays modern times we can probably rely on bundler rather than pry to do this for us.

@neves
Copy link
Author

neves commented Sep 26, 2015

@kyrylo I just copy/paste from byebug. Rescue add about 0.1s to load time on my machine.
But if every plugins adds 0.1s, you got the idea.

@kyrylo
Copy link
Collaborator

kyrylo commented Sep 26, 2015

So without requiring cli.rb it adds only 0.05s, doesn't it?

@neves
Copy link
Author

neves commented Sep 26, 2015

@kyrylo On my machine, pry takes 0.8s to load without plugins. With only rescue, it takes 0.9s.
So 0.1s.
With others plugins, its taking 4s.

@neves
Copy link
Author

neves commented Sep 27, 2015

Just to make it clear, cli.rb is required by pry even if you'll not use it.
For example, if you execute pry -h it will require cli.rb from all pry extensions installed on the system.
That's why the simple existence of this file, slow down pry shell commands.
I have 12 pry-* gems on my system, but only byebug, rescue and theme has cli.rb and none are used.

@ConradIrwin
Copy link
Owner

@neves The fact that it does a require "pry-rescue" is significant. It's being used (or arguably "abused") to ensure that pry-rescue loads early. It might be that we don't need to do this anymore, as this heralds back to the pre-bundler days (the whole pry plugins system doesn't work well with bundler) but someone would have to test that assumption.

@neves
Copy link
Author

neves commented Sep 27, 2015

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.

3 participants