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

CommandSet shouldn't spill its internal state. #980

Closed
ghost opened this issue Sep 23, 2013 · 8 comments
Closed

CommandSet shouldn't spill its internal state. #980

ghost opened this issue Sep 23, 2013 · 8 comments
Labels
Milestone

Comments

@ghost
Copy link

ghost commented Sep 23, 2013

@commands is the object Pry::CommandSet uses to keep track of its commands. We expose it for everyone at Pry::CommandSet#commands but we shouldn't.

Pry::CommandSet is Enumerable already and anything that mutates @commands should be a method on Pry::CommandSet. exposing also makes this weird API possible:

Pry.commands.commands.each do
  # WAT?
end

Pry.commands.each do 
  # ok.
end
@ghost
Copy link
Author

ghost commented Sep 23, 2013

There's also this:

Pry.commands.commands["help"] = "hahaha!"
Pry.commands["help"] = "hahaha!" # TypeError.

@ghost
Copy link
Author

ghost commented Sep 23, 2013

Pry.commands.commands["help"] = "hahaha!" also totally fucks pry. ^D is the only way to fix it/exit.
CommandSet#[]= tries to stop you shooting yourself in the foot.

@kyrylo
Copy link
Member

kyrylo commented Oct 4, 2013

Fixed by #979.

@kyrylo kyrylo closed this as completed Oct 4, 2013
@ghost
Copy link
Author

ghost commented Oct 4, 2013

@kyrylo that PR didn't fix this.

@ghost ghost reopened this Oct 4, 2013
@kyrylo
Copy link
Member

kyrylo commented Oct 4, 2013

Sorry, my bad!

@rf- rf- added the refactor label Apr 29, 2014
@rf- rf- added this to the v1.0.0 milestone Apr 29, 2014
@yui-knk
Copy link
Member

yui-knk commented Apr 30, 2014

Now

[12] pry(main)> Pry.commands.commands["help"]
NoMethodError: undefined method `commands' for #<Pry::CommandSet:0x007fe2d4194dd0>
from (pry):11:in `__pry__'
[15] pry(main)> Pry.commands["help"]  = "hhh"
TypeError: command is not a subclass of Pry::Command
from pry/lib/pry/command_set.rb:346:in `[]='

Maybe this issue is fixed ?

@kdaigle
Copy link

kdaigle commented May 18, 2014

On, pry (0.9.12.6), I get the following while running the same thing @yui-knk did:

[2] pry(main)> Pry.commands.commands["help"]
=> Pry::Command::Help
[3] pry(main)> Pry.commands["help"] = "hhh"
NoMethodError: undefined method `[]=' for #<Pry::CommandSet:0x00000109d2bc70>
from (pry):3:in `<main>'
[4] pry(main)>

But, I get the same response while on master. So it looks like this is fixed on master:

[1] pry(main)> Pry.commands.commands["help"]
NoMethodError: undefined method `commands' for #<Pry::CommandSet:0x00000104272428>
from (pry):1:in `<main>'
[2] pry(main)> Pry.commands["help"] = "hhh"
TypeError: command is not a subclass of Pry::Command
from /opt/rubies/2.0.0/lib/ruby/gems/2.0.0/bundler/gems/pry-ce952527cd5f/lib/pry/command_set.rb:346:in `[]='
[3] pry(main)>

It looks like c05c8f3 and 77a9c00 by @johnny5- fixed this. 👍

@Ferdy89
Copy link
Contributor

Ferdy89 commented Sep 13, 2016

@ghost @kyrylo @yui-knk as @kdaigle mentioned, this looks like it has been solved for a while. Could you close the issue?

@0x1eef 0x1eef closed this as completed Sep 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants