Merge pull request #391 from anthonycrumley/fix-subcommand-edgecases

Fix dispatching of subcommands (concerning :help and *args) (#374)

Author: sferik

lib/thor.rb

@@ -321,23 +321,14 @@ def stop_on_unknown_option #:nodoc:
 
     # The method responsible for dispatching given the args.
     def dispatch(meth, given_args, given_opts, config) #:nodoc: # rubocop:disable MethodLength
-      # There is an edge case when dispatching from a subcommand.
-      # A problem occurs invoking the default command. This case occurs
-      # when arguments are passed and a default command is defined, and
-      # the first given_args does not match the default command.
-      # Thor use "help" by default so we skip that case.
-      # Note the call to retrieve_command_name. It's called with
-      # given_args.dup since that method calls args.shift. Then lookup
-      # the command normally. If the first item in given_args is not
-      # a command then use the default command. The given_args will be
-      # intact later since dup was used.
-      if config[:invoked_via_subcommand] && given_args.size >= 1 && default_command != 'help' && given_args.first != default_command
-        meth ||= retrieve_command_name(given_args.dup)
-        command = all_commands[normalize_command_name(meth)]
-        command ||= all_commands[normalize_command_name(default_command)]
-      else
-        meth ||= retrieve_command_name(given_args)
-        command = all_commands[normalize_command_name(meth)]
+      meth ||= retrieve_command_name(given_args)
+      command = all_commands[normalize_command_name(meth)]
+
+      if !command && config[:invoked_via_subcommand]
+        # We're a subcommand and our first argument didn't match any of our
+        # commands. So we put it back and call our default command.
+        given_args.unshift(meth)
+        command = all_commands[normalize_command_name(default_command)]
       end
 
       if command

spec/register_spec.rb

@@ -82,6 +82,20 @@ def say
   default_command :say
 end
 
+class SubcommandWithDefault < Thor
+  default_command :default
+
+  desc 'default', 'default subcommand'
+  def default
+    puts 'default'
+  end
+
+  desc 'with_args', 'subcommand with arguments'
+  def with_args(*args)
+    puts 'received arguments: ' + args.join(',')
+  end
+end
+
 BoringVendorProvidedCLI.register(
   ExcitingPluginCLI,
   'exciting',
@@ -125,6 +139,9 @@ def say
   'say message',
   'subcommands ftw')
 
+BoringVendorProvidedCLI.register(SubcommandWithDefault,
+  'subcommand', 'subcommand', 'Run subcommands')
+
 describe '.register-ing a Thor subclass' do
   it 'registers the plugin as a subcommand' do
     fireworks_output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[exciting fireworks]) }
@@ -136,20 +153,33 @@ def say
     expect(help_output).to include('do exciting things')
   end
 
-  it 'invokes the default command correctly' do
-    output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[say hello]) }
-    expect(output).to include('hello')
-  end
+  context 'with a default command,' do
+    it 'invokes the default command correctly' do
+      output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[say hello]) }
+      expect(output).to include('hello')
+    end
 
-  it 'invokes the default command correctly with multiple args' do
-    output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[say_multiple hello adam]) }
-    expect(output).to include('hello')
-    expect(output).to include('adam')
-  end
+    it 'invokes the default command correctly with multiple args' do
+      output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[say_multiple hello adam]) }
+      expect(output).to include('hello')
+      expect(output).to include('adam')
+    end
+
+    it 'invokes the default command correctly with a declared argument' do
+      output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[say_argument hello]) }
+      expect(output).to include('hello')
+    end
+
+    it "displays the subcommand's help message" do
+      output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[subcommand help]) }
+      expect(output).to include('default subcommand')
+      expect(output).to include('subcommand with argument')
+    end
 
-  it 'invokes the default command correctly with a declared argument' do
-    output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[say_argument hello]) }
-    expect(output).to include('hello')
+    it "invokes commands with their actual args" do
+      output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[subcommand with_args actual_argument]) }
+      expect(output.strip).to eql('received arguments: actual_argument')
+    end
   end
 
   context 'when $thor_runner is false' do