tj / commander

The complete solution for Ruby command-line executables
http://visionmedia.github.com/commander
MIT License
1.09k stars 64 forks source link

Use HighLine menu system #79

Closed dcode closed 10 years ago

dcode commented 10 years ago

I was trying to use a menu as a component to a command and there appears to be a conflict between the commander version of "choose" and the HighLine version of "choose".

Example:

require 'commander/import'

command :foo do |c|
  c.action do |args, options|
    choose do |menu|
      menu.prompt = "Select an option:  "
      menu("Do this one") { puts "I did the first one." }
      menu("Do the other one") { puts "I did the second one." }
    end
  end
end

This results in the following error:

/usr/share/gems/gems/commander-4.2.0/lib/commander/user_interaction.rb:43:in `choose': wrong number of arguments (0 for 1+) (ArgumentError)
ggilder commented 10 years ago

There are a couple of workarounds you could use here.

Call the method directly from the class:

HighLine.method(:choose).call do |menu|
  menu.prompt = "Select an option:"
  menu.choice("Do this one") { puts "I did the first one." }
  menu.choice("Do the other one") { puts "I did the second one." }
end

Or alias it:

require 'highline/import'
alias :highline_choose :choose
require 'commander/import'

command :foo do |c|
  c.action do |args, options|
    highline_choose do |menu|
      menu.prompt = "Select an option:  "
      menu.choice("Do this one") { puts "I did the first one." }
      menu.choice("Do the other one") { puts "I did the second one." }
    end
  end
end

It's an unfortunate naming conflict... the only thing I can think of as an acceptable workaround would be for Commander to provide an alias to the original HighLine method, like highline_choose. Otherwise we could break existing code. What do you think?

ggilder commented 10 years ago

Actually, I see that our version of choose delegates to HighLine's version, so we can probably improve this. I'll take a stab at it.

dcode commented 10 years ago

Cool. Otherwise I think the alias is reasonable. I'm an experienced programmer, but new to Ruby and all its idioms.

ggilder commented 10 years ago

I've released the fix in 4.2.1. Thanks for reporting the problem!