rails / thor

Thor is a toolkit for building powerful command-line interfaces.
http://whatisthor.com/
MIT License
5.12k stars 553 forks source link

Thor::Group registered as subcommand can't find arguments. #149

Open devboy opened 13 years ago

devboy commented 13 years ago

I want to register a Thor::Group as a subcommand to another Thor class which fails when introducing arguments to the Thor::Group. I am using the same technique to add regular Thor tasks as subcommands which works fine.

class Task < Thor::Group

  argument :name

  def first_method
    p name
  end

end

Thor::Runner.register Task, "task", "USAGE", "DESC"

running the command without argument fails (as expected) because of a missing argument:

$ runner task
No value provided for required arguments 'name'

but running the command with an argument fails as well:

$ runner task myname
"task" was called incorrectly. Call as "runner USAGE". 

Can anyone provide workarounds or solutions to this problem, I was going through the source but couldn't point it out.

devboy commented 13 years ago

I filed a pull request here: https://github.com/wycats/thor/pull/150

anithri commented 13 years ago

I don't think that patch fixes this problem, but then It's 4:00 am and I just finished figuring this out so I'm going to post a workaround instead.

class Foo < Thor::Group

  argument :name

  def first_method
    p name
  end

end

Thor::Runner.register Foo, "foo", "USAGE", "DESC"

#Hackery here!
Thor::Runner.tasks["foo"].options = Foo.class_options

Basically you're getting the class options from the Foo class, and assigning it as the options hash of the "foo" task.

I can't for the life of me determine what the correct behavior is supposed to be in this case, but this is the most DRY way I could find to propogate the options for subcommands.

devboy commented 13 years ago

I am using my patch for a quite a while now and it's working without a doubt.

anithri commented 13 years ago

I created a sample thor app called Thorax https://github.com/anithri/thorax. Using just this one patch, I can't get the correct behavior. So, there could be other things in the master branch that does fix this. I'll fork and apply and try it out. For the meantime, here's my results for simply applying your patch to Thor 0.14.6

  1. created it's own gemset on ruby-1.9.2-p190
  2. installed bundler
  3. bundle install --path=vendor to install all gems into a local dir
  4. edited vendor/ruby/1.9.1/gems/thor-0.14.6/lib/thor.rb to change #register as per your Pull request
  5. created a basic thor app with both a Thor::Group and a Thor added via #register
  6. Comment out the 2 lines that cop
#Command and output for help and help foo.  This is correct behavior
% bundle exec bin/thorax help
Tasks:
  thorax bar          # print bar
  thorax baz          # print stuff
  thorax foo          # print foo
  thorax help [TASK]  # Describe available tasks or one specific task

Options:
  [--config=CONFIG]  # configuration file.
                     # Default: ~/.thoraxrc

% bundle exec bin/thorax help foo
Usage:
  thorax foo

Options:
  [--rfoo=N]         # repeat greeting X times
                     # Default: 3
  [--config=CONFIG]  # configuration file.
                     # Default: ~/.thoraxrc

print foo
#Command and output for help bar and help baz.  This is NOT CORRECT behavior
% bundle exec bin/thorax help bar
Usage:
  thorax bar

Options:
  [--config=CONFIG]  # configuration file.
                     # Default: ~/.thoraxrc

print bar

% bundle exec bin/thorax help baz
Usage:
  thorax baz

Options:
  [--config=CONFIG]  # configuration file.
                     # Default: ~/.thoraxrc

print stuff
  1. uncomment #Thorax::Runner.tasks["bar"].options = Thorax::More.class_options and similar line for Stuff

The bundle exec bin/thorax help and bundle exec bin/thorax help foo continue to be correct, while bundle exec bin/thorax help bar and bundle exec bin/thorax help baz are now correct.

% bundle exec bin/thorax help bar
Usage:
  thorax bar

Options:
  [--rbar=N]         # repeat greeting X times
                     # Default: 3
  [--config=CONFIG]  # configuration file.
                     # Default: ~/.thoraxrc

print bar

% bundle exec bin/thorax help baz
Usage:
  thorax baz

Options:
  [--rbaz=N]         # repeat greeting X times
                     # Default: 3
  [--config=CONFIG]  # configuration file.
                     # Default: ~/.thoraxrc

print stuff
devboy commented 13 years ago

My patch fixes the "passing through" of arguments but doesn't touch the options, but I haven't experienced any problems with the options on my side. Thanks for the detailled info, I'll check this out properly tomorrow!

anithri commented 13 years ago

I think I found the discrepancy while playing with the Thor Specs.

Thor uses #register to add an entire class, and then uses that to provide subcommands

#lib/thorax/stuff.rb Line: ~10
Thorax::Runner.register Thorax::Stuff, :stuff, "stuff", "print more stuff"
#then you see...
% bundle exec bin/thorax
Tasks:
  thorax bar          # print bar
  thorax foo          # print foo
  thorax help [TASK]  # Describe available tasks or one specific task
  thorax stuff        # print more stuff

Options:
  [--config=CONFIG]  # configuration file.
                     # Default: ~/.thoraxrc

% bundle exec bin/thorax stuff
Tasks:
  thorax baz             # print baz
  thorax help [COMMAND]  # Describe subcommands or one specific subcommand

% bundle exec bin/thorax stuff baz
default default default 

While what I am doing is using #register to register a single task. Which works except for the method_options don't come across.

I think that my use of register as useful as the existing one. So the question is how best to codify it.

  1. def register_task to handle registering the new connection directly.
  2. add a new clause to the if statement in #register like elsif klass.instance_method(:subcommand_name) that catches the case where the Class being registered has a method of the appropriate name. May break existing code though.
  3. provide docs and code for a different way to do this entirely
  4. Leave it as a voodoo hack.

I'd be happy to write specs and code to implement this, given a nudge in the right direction.