commander-rb / commander

The complete solution for Ruby command-line executables
MIT License
822 stars 74 forks source link

Option argument parsed as a global option #80

Open jeffshantz opened 5 years ago

jeffshantz commented 5 years ago

I'm using Commander 4.4.7. I am noticing that if I have an option that takes an argument, and the argument starts with a global option, the argument is parsed as a global option.

require 'commander/import'

program :name, 'Name'
program :version, '0.1'
program :description, 'Issue with argument parsing.'

default_command :mount

command :mount do |c|
  c.option('--options=STRING', String) do |opts|
    puts "Parsed mount options: #{opts}"
  end
  c.action do
    puts 'Success'
  end
end

When I run the program, it interprets -vers=3.0 as the global option -v:

$ ruby test.rb --options "-vers=3.0"
Name 0.1

If I use = to separate --options from its argument, it works as expected:

$ ruby test.rb --options="-vers=3.0"
Parsed mount options: -vers=3.0
Success

So, we have a workaround, but I still argue that it's a bug. First, it's very common to omit the =. Second, it works in either case using optparse directly. Here is an example:

require 'optparse'

OptionParser.new do |opts|
  opts.banner = "Usage: example.rb [options]"

  opts.on("-v", "--version", "Show version") do |v|
    puts "-v received"
  end

  opts.on("-o OPTIONS", "--options OPTIONS", "Provide mount options") do |opts|
    puts "-o received with #{opts}"
  end

end.parse!

Observe that it works as expected in either case:

$ ruby test2.rb --options "-vers=3.0"
-o received with -vers=3.0

$ ruby test2.rb --options="-vers=3.0"
-o received with -vers=3.0
ggilder commented 4 years ago

Hmm, I'm not sure this is a bug so much as undefined behavior. From the shell's perspective, the invocation you mention is no different from the following:

$ ruby test.rb --options -vers=3.0

In other words, the quotes don't matter because they're interpreted away by the shell. That might clarify why in this situation, commander sees a global option.

I'm not totally sure what the correct behavior is here, but it is interesting that optparse behaves differently. However, since optparse doesn't have the same notion of global options it's a little difficult to directly compare.

jeffshantz commented 4 years ago

I understand that the quotes are removed by the shell, but I would argue that the parser should understand that --options takes an argument and therefore the next argument would be the argument to --options and not an option of its own.

orien commented 4 years ago

Looks like the problem is in the parsing of global options. In particular, the parse, rescue, remove and retry portion:

https://github.com/commander-rb/commander/blob/0abbac486ea7d87f6f680aaf17d0e3cd136dbf73/lib/commander/runner.rb#L380-L386

In normal usage, it'll raise on the command specific options, which will then be removed from the array of options before retrying the parse.

In the scenario detailed in this issue: test.rb --options -vers=3.0. Only the --options token will be removed, as -vers=3.0 satisfies the --version global. Hence in the retry, the options are only -vers=3.0 which triggers the --version behaviour.

ggilder commented 4 years ago

Exactly, global options are parsed in a first pass before parsing command options. The OptionParser example provided by @jeffshantz above works because there is only one pass on parsing the options.