Skip to content

Loading…

Possible fix for the subcommand help messages using the klass name instead of command name #330

Open
wants to merge 1 commit into from

2 participants

@rberger

Changed spec/subcommand and spec/fixtures/script.thor to expose the problem of when the subcommand name is different than the klass.

Fixed it by:

Updating lib/thor.rb metaprogramming magic that adds the help methods
to the subcommand klass to include a class instance variable that is
initialized to the subcommand_name and a method to access it.

Then modified command#formatted_usage to make use of that method if
its processing a subcommand and the subcommand_method exists.

Also had to "fix" the spec/register_spec.rb
context "when $thor_runner is false"
that had a test that seemed to accecpt the fact that subcommand names
got foobar'd if the klass name is different than the subcommand name.
Or I didn't understand that test, but I changed it to what made more
sense to me.

I believe this addresses #306, #169, #170, #196, #261, #128, #288

@rberger rberger Changed spec/subcommand and spec/fixtures/script.thor to expose the
problem of when the subcommand name is different than the klass.

Fixed it by:

Updating lib/thor.rb metaprogramming magic that adds the help methods
to the subcommand klass to include a class instance variable that is
initialized to the subcommand_name and a method to access it.

Then modified command#formatted_usage to make use of that method if
its processing a subcommand and the subcommand_method exists.

Also had to "fix" the spec/register_spec.rb
   context "when $thor_runner is false"
that had a test that seemed to accecpt the fact that subcommand names
got foobar'd if the klass name is different than the subcommand name.
Or I didn't understand that test, but I changed it to what made more
sense to me.
562a9af
@rberger

Ok, I've already discovered one problem with this. It breaks the namespace override.

Got another possible fix comming, though now that I looked into the namespace issue, I now understand one of the workarounds people mentioned for using namespaces to update the help output. That still seems like a hack

@rberger

I've created a new branch at https://github.com/rberger/thor named subcommand_help_issue_2 that fixes the bug introduced in the earlier patch. So now if you set the namespace in the subcommand class, it will "win" and be the name displayed.

I'm not pushing it here as there still is a problem that if you ask for help on the subcommand's commands it still shows the snake case of the Class name instead of the subcommand's name.

That branch does have an added test to subcommand_spec that demonstrates the problem.

I don't have any more time to spend on it, and not quite sure what other ripples me trying to fix this will create, as the current thor rspec test suite did not detect the failure my last patch introduced.

It does look like the "work around" described in #261 (comment) does work with the current 0.18.1 thor gem.

It does seem like the proper solution would be to somehow make the command name be accesible as a first class method in the Thor cli classes. And a bonus would be to make the formatting more manageable programmatically at the application layer and not just on a per class basis (or I need to understand the documentation better if that mechanism is already there)

Wish I had more time to futz with this. Learned a lot by diving into the thor code, but have to pop my stack back to the other Yak's I'm shaving and maybe eventually finishing my real work :-)

@henderea

I was having this issue, and this is the solution I came up with (handles multi-level nesting):

class Thor
  class << self
    attr_accessor :parent_class

    def basename2(subcommand = false)
      bn  = parent_class && parent_class.basename2
      bn2 = basename
      ns  = self.namespace.split(':').last
      bn ? (subcommand ? bn : "#{bn} #{ns}") : bn2
    end

    def banner(command, namespace = nil, subcommand = false)
      "#{basename2(subcommand)} #{command.formatted_usage(self, $thor_runner, subcommand)}"
    end

    alias :old_subcommand :subcommand

    def subcommand(subcommand, subcommand_class)
      subcommand_class.parent_class = self
      old_subcommand(subcommand, subcommand_class)
    end
  end
end

I figure someone can find a way to use something like this directly in the Thor library.

I have tested this with nesting beyond one subcommand, and it worked just fine.

@jmoody jmoody referenced this pull request in calabash/run_loop
Open

CLI: subcommand help is not printing correctly #226

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 9, 2013
  1. @rberger

    Changed spec/subcommand and spec/fixtures/script.thor to expose the

    rberger committed
    problem of when the subcommand name is different than the klass.
    
    Fixed it by:
    
    Updating lib/thor.rb metaprogramming magic that adds the help methods
    to the subcommand klass to include a class instance variable that is
    initialized to the subcommand_name and a method to access it.
    
    Then modified command#formatted_usage to make use of that method if
    its processing a subcommand and the subcommand_method exists.
    
    Also had to "fix" the spec/register_spec.rb
       context "when $thor_runner is false"
    that had a test that seemed to accecpt the fact that subcommand names
    got foobar'd if the klass name is different than the subcommand name.
    Or I didn't understand that test, but I changed it to what made more
    sense to me.
Showing with 12 additions and 7 deletions.
  1. +5 −1 lib/thor.rb
  2. +2 −1 lib/thor/command.rb
  3. +1 −1 spec/base_spec.rb
  4. +2 −2 spec/fixtures/script.thor
  5. +1 −1 spec/register_spec.rb
  6. +1 −1 spec/subcommand_spec.rb
View
6 lib/thor.rb
@@ -454,7 +454,11 @@ def find_command_possibilities(meth)
def subcommand_help(cmd)
desc "help [COMMAND]", "Describe subcommands or one specific subcommand"
- class_eval <<-RUBY
+ class_eval <<-RUBY
+ @subcommand_name = "#{cmd.to_s}"
+ def self.subcommand_name()
+ @subcommand_name
+ end
def help(command = nil, subcommand = true); super; end
RUBY
end
View
3 lib/thor/command.rb
@@ -45,7 +45,8 @@ def formatted_usage(klass, namespace = true, subcommand = false)
namespace = klass.namespace
formatted = "#{namespace.gsub(/^(default)/,'')}:"
end
- formatted = "#{klass.namespace.split(':').last} " if subcommand
+
+ formatted = (klass.respond_to? :subcommand_name) ? "#{klass.subcommand_name} " : "#{klass.namespace.split(':').last} " if subcommand
formatted ||= ""
View
2 spec/base_spec.rb
@@ -192,7 +192,7 @@ def hello
it "returns tracked subclasses, grouped by the files they come from" do
thorfile = File.join(File.dirname(__FILE__), "fixtures", "script.thor")
expect(Thor::Base.subclass_files[File.expand_path(thorfile)]).to eq([
- MyScript, MyScript::AnotherScript, MyChildScript, Barn,
+ MyScript, MyScript::AnotherScript, MyChildScript, BarnCli,
PackageNameScript, Scripts::MyScript, Scripts::MyDefaults,
Scripts::ChildDefault, Scripts::Arities
])
View
4 spec/fixtures/script.thor
@@ -145,7 +145,7 @@ class MyChildScript < MyScript
remove_command :boom, :undefine => true
end
-class Barn < Thor
+class BarnCli < Thor
desc "open [ITEM]", "open the barn door"
def open(item = nil)
if item == "shotgun"
@@ -192,7 +192,7 @@ module Scripts
end
desc "barn", "commands to manage the barn"
- subcommand "barn", Barn
+ subcommand "barn", BarnCli
end
class ChildDefault < Thor
View
2 spec/register_spec.rb
@@ -157,7 +157,7 @@ def say
begin
$thor_runner = false
help_output = capture(:stdout) { BoringVendorProvidedCLI.start(%w[exciting]) }
- expect(help_output).to include('thor exciting_plugin_c_l_i fireworks')
+ expect(help_output).to include('thor exciting fireworks')
ensure
$thor_runner = true
end
View
2 spec/subcommand_spec.rb
@@ -6,7 +6,7 @@
it "maps a given subcommand to another Thor subclass" do
barn_help = capture(:stdout) { Scripts::MyDefaults.start(%w[barn]) }
- expect(barn_help).to include("barn help [COMMAND] # Describe subcommands or one specific subcommand")
+ expect(barn_help).to include("thor barn help [COMMAND] # Describe subcommands or one specific subcommand")
end
it "passes commands to subcommand classes" do
Something went wrong with that request. Please try again.