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

Commands should support arguments #72

Closed
kyrylo opened this issue Aug 13, 2012 · 7 comments · Fixed by #76
Closed

Commands should support arguments #72

kyrylo opened this issue Aug 13, 2012 · 7 comments · Fixed by #76

Comments

@kyrylo
Copy link
Contributor

kyrylo commented Aug 13, 2012

Currently, there is no way to implement parsing of commands that accept arguments. Let's consider a basic Slop v3 command set:

commands = Slop::Commands.new do
  on 'install'
end

Let's try to parse some input:

commands.parse %w( install geronimo )
#=> ["geronimo"]

The problem with current code is that it doesn't support arguments for install subcommand. Of course, you can save the return value of Commands#parse and use it in your program, but let's have a look at the situation when our command set have more than one command:

commands = Slop::Commands.new do
  on :install
  on :list
end

commands.parse %w( install geronimo )
#=> ["geronimo"]

commands.parse %w( list geronimo )
#=> ["geronimo"]

Both calls return the same value. Given a stroppy user of our CLI tool, it's pretty obvious that we can't tell, which of the subcommands has been executed from program's point of view: install or list?

It seems like the only way to go is to use command options:

commands = Slop::Commands.new do
  on :install do
    on :n, :name
  end

  on :list do
    on :n, :name
  end
end

commands.parse %w( install --name geronimo )
#=> ["--name", "geronimo"]
commands[:install].present? :name
#=> true

commands.parse %w( list --name geronimo )
#=> ["--name", "geronimo"]
commands[:list].present? :name
#=> true

Quite frankly, this pretty verbose and simply sucks.

Possible solutions

Add :with_argument parameter hash to Commands#on

This parameter should be specific for each command.

commands = Slop::Commands.new do
  on :install, :with_argument => true
  on :list
end

commands.parse %w( install geronimo )
#=> ["install", "geronimo"]
commands.present? :install
#=> true
commands.present? :list
#=> false
commands[:list].argument
#=> "geronimo"

Treat one of subcommand options as a default option

commands = Slop::Commands.new do
  on :install do
    as_default(on(:n, :name))
    on :v, :verbose
  end

  on :list
end

commands.parse %w( install --name geronimo )
#=> ["--name", "geronimo"]
commands.parse %w( install geronimo )
#=> ["install", "geronimo"]
commands[:list].argument
#=> "geronimo"
@leejarvis
Copy link
Owner

Thanks for reporting this. I should have some time tomorrow to look into it a little more.

@leejarvis
Copy link
Owner

How about something like this:

require 'slop'

opts = Slop::Commands.new do
  on :install
end

opts.parse %w(install geronimo)

p opts.arguments #=> ["geronimo"]

@kyrylo
Copy link
Contributor Author

kyrylo commented Aug 13, 2012

Looks good, but still there is no way to tell, which command has been triggered:

opts = Slop::Commands.new do
  on :install
  on :list
end

opts.parse %w(install geronimo)

p opts.arguments #=> ["geronimo"]
# Is this an argument of `install` or maybe `list`?

@leejarvis
Copy link
Owner

I can't see why you would ever care which command was used. I see something like this:

if opts[:install]
  run_installer(opts.arguments)
end

And just pass around opts.arguments to any sub command you need

@kyrylo
Copy link
Contributor Author

kyrylo commented Aug 13, 2012

I don't know, maybe I don't understand something, but this if check doesn't work as you showed in your example. opts[:install] always returns a Slop instance, whatever happens.

opts = Slop::Commands.new do
  on :install
  on :list
end

opts.parse %w(install geronimo)

p opts[:install]
#=> #<Slop {:strict=>false, :help=>false, :banner=>nil, :ignore_case=>false, :autocreate=>false, :arguments=>false, :optional_arguments=>false, :multiple_switches=>true, :longest_flag=>0} []>

p opts[:list]
#=> #<Slop {:strict=>false, :help=>false, :banner=>nil, :ignore_case=>false, :autocreate=>false, :arguments=>false, :optional_arguments=>false, :multiple_switches=>true, :longest_flag=>0} []>

P.S.: Also, keep in mind the situation when we want to pass multiple arguments (like we can do it with options: --install geronimo,banzai,huzzah)

@leejarvis
Copy link
Owner

Yeah you're right. I think temporarily something like this will have to do:

opts = Slop::Commands.new do
  on :install
  on :list
end

opts.parse %w(install geronimo)

if opts.present?(:install)
  # do something with opts.arguments
end

Which I guess is just short for

if ARGV.first == 'install'
  # ...

That is, I think the command interface probably needs an overhaul but I'd rather plan and spend the time getting it right for Slop v4.

@leejarvis
Copy link
Owner

Closed by #76

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 a pull request may close this issue.

2 participants