Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

rberger
Copy link

@rberger rberger commented May 9, 2013

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

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.
@rberger
Copy link
Author

rberger commented May 9, 2013

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
Copy link
Author

rberger commented May 9, 2013

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
Copy link

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.

@rafaelfranca
Copy link
Member

Closing since this is too outdated. Thank you for the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants