rails / thor

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

Test for #755 #756

Closed KishiTheMechanic closed 2 years ago

KishiTheMechanic commented 2 years ago

Related this issue #627 , #755 Modified tests also.

dorner commented 2 years ago

@KishiTheMechanic this doesn't seem to be fixing the behavior you mentioned earlier. This seems to remove the thor part of the help output but doesn't fix it for doubly nested subcommands. Unless there's another missing test somewhere?

KishiTheMechanic commented 2 years ago

I think it's ok solution for now because if there's no thor part, we can see them as after the command of which you entered with help.

dorner commented 2 years ago

@KishiTheMechanic can you add a test demonstrating this behavior please?

KishiTheMechanic commented 2 years ago

What do you mean? I modified the tests related this changing?

dorner commented 2 years ago

The tests are all showing the main case of removing "thor" but they are not showing any new case that fixes the problem originally stated (where nested subcommands don't show the parent command).

KishiTheMechanic commented 2 years ago

I thought removing "thor" is the solution for the state (where nested subcommands don't show the parent command). Because the state's problem is confusing us about nested subcommands because it shows the head but not middle. If it shows only after part of which command user inputed, we can know it is the help for the sub commands. Doesn't it work?

dorner commented 2 years ago

This is why I'd like to see a test demonstrating the issue so we can confirm that the fix works for the original case. 😄

KishiTheMechanic commented 2 years ago

for example,

➜  souls-doc git:(main) souls api generate help 
Commands:
  souls generate connection [CLASS_NAME]                                 # Generate GraphQL Connection from schema.rb
  souls generate delete_all  [CLASS_NAME]                                # Generate Scaffold All Tables from schema.rb
  souls generate edge [CLASS_NAME]                                       # Generate GraphQL Edge from schema.rb
  souls generate help [COMMAND]                                          # Describe subcommands or one specific subcommand
  souls generate job [CLASS_NAME]                                        # Generate Job File in Worker
  souls generate mailer [MAILER_NAME]                                    # Generate Mailer Template in Worker
  souls generate manager [MANAGER_NAME] --mutation, --mutation=MUTATION  # Generate GraphQL Mutation Template
  souls generate model [CLASS_NAME]                                      # Generate Model Template
  souls generate mutation [CLASS_NAME]                                   # Generate GraphQL Mutation from schema.rb
  souls generate policy [CLASS_NAME]                                     # Generate Policy File Template
  souls generate query [CLASS_NAME]                                      # Generate GraphQL Query from schema.rb
  souls generate resolver [CLASS_NAME]                                   # Generate GraphQL Resolver from schema.rb
  souls generate rspec_factory [CLASS_NAME]                              # Generate Rspec Factory Test from schema.rb
  souls generate rspec_model [CLASS_NAME]                                # Generate Rspec Model Test from schema.rb
  souls generate rspec_mutation [CLASS_NAME]                             # Generate Rspec Mutation Test from schema.rb
  souls generate rspec_policy [CLASS_NAME]                               # Generate Rspec Policy Test from schema.rb
  souls generate rspec_query [CLASS_NAME]                                # Generate Rspec Query Test from schema.rb
  souls generate rspec_resolver [CLASS_NAME]                             # Generate Rspec Resolver Test from schema.rb
  souls generate scaffold [CLASS_NAME]                                   # Generate Scaffold from schema.rb
  souls generate scaffold_all                                            # Generate Scaffold All Tables from schema.rb
  souls generate type [CLASS_NAME]                                       # Generate GraphQL Type from schema.rb

➜  souls-doc git:(main) souls api help
Commands:
  souls api generate [COMMAND]  # souls api generate Commands
  souls api help [COMMAND]      # Describe subcommands or one specific subcommand
  souls api update [COMMAND]    # souls api update Commands

This is the problem.

Then I changed to below.

➜  souls-doc git:(main) souls api generate help 
Commands:
  generate connection [CLASS_NAME]                                 # Generate GraphQL Connection from schema.rb
  generate delete_all  [CLASS_NAME]                                # Generate Scaffold All Tables from schema.rb
  generate edge [CLASS_NAME]                                       # Generate GraphQL Edge from schema.rb
  generate help [COMMAND]                                          # Describe subcommands or one specific subcommand
  generate job [CLASS_NAME]                                        # Generate Job File in Worker
  generate mailer [MAILER_NAME]                                    # Generate Mailer Template in Worker
  generate manager [MANAGER_NAME] --mutation, --mutation=MUTATION  # Generate GraphQL Mutation Template
  generate model [CLASS_NAME]                                      # Generate Model Template
  generate mutation [CLASS_NAME]                                   # Generate GraphQL Mutation from schema.rb
  generate policy [CLASS_NAME]                                     # Generate Policy File Template
  generate query [CLASS_NAME]                                      # Generate GraphQL Query from schema.rb
  generate resolver [CLASS_NAME]                                   # Generate GraphQL Resolver from schema.rb
  generate rspec_factory [CLASS_NAME]                              # Generate Rspec Factory Test from schema.rb
  generate rspec_model [CLASS_NAME]                                # Generate Rspec Model Test from schema.rb
  generate rspec_mutation [CLASS_NAME]                             # Generate Rspec Mutation Test from schema.rb
  generate rspec_policy [CLASS_NAME]                               # Generate Rspec Policy Test from schema.rb
  generate rspec_query [CLASS_NAME]                                # Generate Rspec Query Test from schema.rb
  generate rspec_resolver [CLASS_NAME]                             # Generate Rspec Resolver Test from schema.rb
  generate scaffold [CLASS_NAME]                                   # Generate Scaffold from schema.rb
  generate scaffold_all                                            # Generate Scaffold All Tables from schema.rb
  generate type [CLASS_NAME]                                       # Generate GraphQL Type from schema.rb

➜  souls-doc git:(main) souls api help
Commands:
  api generate [COMMAND]  # souls api generate Commands
  api help [COMMAND]      # Describe subcommands or one specific subcommand
  api update [COMMAND]    # souls api update Commands
dorner commented 2 years ago

Ah OK - so rather than adding in the api piece, you're removing the souls piece. I don't think this is the direction I'd like to go - I'd prefer adding in the full path for the best understandability. This fix is a bit better than the current implementation but it's not what I'd want to see.

KishiTheMechanic commented 2 years ago

Yes, it's a small progress but actually better than now I think.

dorner commented 2 years ago

Yes... but if we're going to go ahead and try to fix this then I'd rather fix it the right way rather than doing a half-fix now and a full fix later.

KishiTheMechanic commented 2 years ago

Sounds great!

KishiTheMechanic commented 2 years ago

Could you please let me upload the folk version to the gem (like thor_temp) as temporary solution for now? I think it takes for a while to full fix it so. I will delete the gem after this issue closed.

dorner commented 2 years ago

@KishiTheMechanic not sure I understand the question. You're free to use your own fork in your projects if you like, just reference your Github repo:

gem thor, git: https://github.com/KishiTheMechanic/thor

KishiTheMechanic commented 2 years ago

Yes, but I can't use it on our gem... We need to specify it as dependency to publish our gem (SOULs) so. https://rubygems.org/gems/souls By the way, thank you always so much to contribute it.

dorner commented 2 years ago

Yeah... unfortunately there's no good way to specify a fork as a dependency on a gem. However, you can make a note in your README that to use it you need to add the fork of the thor gem to your Gemfile as specified. You can also add it to your gem's Gemfile (as opposed to its gemspec) so your tests pass.

Having said that, the best solution would indeed be to put in the "real" fix so we can merge 😄

KishiTheMechanic commented 2 years ago

After all, I think you should accept this temporary solution now. Because it seems very difficult to get nested commands in this architecture (If you know, please tell me). And the bug has taken almost 2 years to fix which means also the problem is hard.

All Thor made CLI keeps lying until now. I think it's huge problem.

After this solution, at least the CLIs stop lying.

KishiTheMechanic commented 2 years ago

I think we shouldn't nest commands too much in Thor and will refactor our gem to make shallow structure because it's not the only problem in too much nested commands. Thank you.