use the correct class for shared namespaces

[Gerst20051]

check commands to find the correct class when multiple classes have the same namespace

this is related to https://github.com/rails/thor/issues/246 and https://github.com/rails/thor/pull/247

I'm assuming the intention was to allow multiple classes to use the same namespace. I guess I don't understand the reasoning for the namespace if it can only be used for a single class.

🌈

[dorner]

@Gerst20051 can you add a unit test for this?

[jb08]

I'm assuming the intention was to allow multiple classes to use the same namespace. I guess I don't understand the reasoning for the namespace if it can only be used for a single class.

+1

[hmistry]

@Gerst20051 I couldn't push these updates to your fork+branch. Branch is ok to rebase to current Rails:Thor:main branch - no conflicts. Add this test and then PR is ready to merge. No updates to documentation needed as this is a bug fix.

Here is the test for this fix:

# File: spec/fixtures/script.thor
# add this at the bottom
class Apple < Thor
  namespace :fruits
 desc 'apple', 'apple'; def apple; end
end

class Pear < Thor
  namespace :fruits
 desc 'pear', 'pear'; def pear; end
end

# File: spec/util_spec.rb in L91
# add this test in the #find_class_and_command_by_namespace describe block
  describe "#find_class_and_command_by_namespace" do
    # .... existing tests
    it "returns correct Thor class and the command name when shared namespaces" do
      expect(Thor::Util.find_class_and_command_by_namespace("fruits:apple")).to eq([Apple, "apple"])
      expect(Thor::Util.find_class_and_command_by_namespace("fruits:pear")).to eq([Pear, "pear"])
    end
  end

# File: spec/base_spec.rb in L193
# add `Apple`, `Pear` to the expected list
    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,
        PackageNameScript, Scripts::MyScript, Scripts::MyDefaults,
        Scripts::ChildDefault, Scripts::Arities, Apple, Pear
      ])
    end

LGTM 🚀

[hmistry]

@dorner Add unit tests, please review... LGTM

[hmistry]

Hi @rafaelfranca, @dorner, @deivid-rodriguez. Any chance this can be reviewed soon and merged so it's in the next release? This bug impacts our ability to organize thor tasks in our project into manageable grouped namespaces.

[Gerst20051]

@dorner can you help move this along? how should we proceed to get this released?

[dorner]

Unfortunately I'm not an admin here - @rafaelfranca is the closest thing we have to an active admin and he's sadly pretty busy. :(

[hmistry]

@rafaelfranca Thank you for merging this in and refactoring it. We look forward to seeing it in the next release and start using namespaces across classes. 🙏