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

Return subcommand if its already registered #883

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bradgessler
Copy link

When running Thor in a development environment on a server a code reloader, I noticed the subcommand would register duplicates like this:

tinyzap/server [cli-puma] → bin/tinyzap
Commands:
  tinyzap echo                                                          # Echos the input from stdin
  tinyzap help [COMMAND]                                                # Describe available commands or one specific command
  tinyzap links                                                         # Manage links
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links list                                                    # Lists links
  tinyzap links list                                                    # Lists links
  tinyzap links list                                                    # Lists links
  tinyzap links list                                                    # Lists links
  tinyzap links list                                                    # Lists links
  tinyzap links list                                                    # Lists links
  tinyzap links list                                                    # Lists links
  tinyzap login                                                         # Login to TinyZap
  tinyzap whoami                                                        # Prints your current login information

tinyzap/server [cli-puma] →

This PR fixes that problem by checking to see if the command is registered already. If it is, it returns the existing subcommand.

tinyzap/server [cli-puma] → bin/tinyzap
Commands:
  tinyzap echo                                                          # Echos the input from stdin
  tinyzap help [COMMAND]                                                # Describe available commands or one specific command
  tinyzap links                                                         # Manage links
  tinyzap links create -d, --description=DESCRIPTION -t, --title=TITLE  # Create a new link
  tinyzap links help [COMMAND]                                          # Describe subcommands or one specific subcommand
  tinyzap links list                                                    # Lists links
  tinyzap login                                                         # Login to TinyZap
  tinyzap whoami                                                        # Prints your current login information

@@ -327,6 +327,8 @@ def subcommand_classes
end

def subcommand(subcommand, subcommand_class)
return subcommand_classes[subcommand.to_s] if subcommands.include? subcommand.to_s
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to return the command class? It seems we only need to not register the command again. But what happens if people wants to override the subcommand to have different code? Let's say a library trying to enhance an already existing class. It seems that would break that behavior.

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.

2 participants