From ab93ec4c74a45e19565ba0d8f8ad9a5a08b8b796 Mon Sep 17 00:00:00 2001 From: Zakir Dzhamaliddinov Date: Thu, 1 Aug 2024 10:53:37 +0300 Subject: [PATCH] Review fixes --- docs/commands.md | 4 ++-- lib/command/base.rb | 3 ++- lib/command/terraform/generate.rb | 11 +++-------- lib/cpflow.rb | 21 +++++++++++++-------- script/update_command_docs | 8 ++++++-- spec/cpflow_spec.rb | 11 +++++++---- spec/support/command_helpers.rb | 9 --------- 7 files changed, 33 insertions(+), 34 deletions(-) diff --git a/docs/commands.md b/docs/commands.md index 43e6fa61..14fa758a 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -444,9 +444,9 @@ cpflow run -a $APP_NAME --entrypoint /app/alternative-entrypoint.sh -- rails db: cpflow setup-app -a $APP_NAME ``` -### `generate` +### `terraform generate` -Generates terraform configuration files based on `controlplane.yml` and `templates/` config +- Generates terraform configuration files based on `controlplane.yml` and `templates/` config ```sh cpflow terraform generate diff --git a/lib/command/base.rb b/lib/command/base.rb index 47d14aae..7ef6411d 100644 --- a/lib/command/base.rb +++ b/lib/command/base.rb @@ -12,6 +12,8 @@ class Base # rubocop:disable Metrics/ClassLength VALIDATIONS_WITH_ADDITIONAL_OPTIONS = %w[templates].freeze ALL_VALIDATIONS = VALIDATIONS_WITHOUT_ADDITIONAL_OPTIONS + VALIDATIONS_WITH_ADDITIONAL_OPTIONS + # Used to call the command (`cpflow SUBCOMMAND_NAME NAME`) + SUBCOMMAND_NAME = nil # Used to call the command (`cpflow NAME`) # NAME = "" # Displayed when running `cpflow help` or `cpflow help NAME` (defaults to `NAME`) @@ -38,7 +40,6 @@ class Base # rubocop:disable Metrics/ClassLength WITH_INFO_HEADER = true # Which validations to run before the command VALIDATIONS = %w[config].freeze - SUBCOMMAND = nil def initialize(config) @config = config diff --git a/lib/command/terraform/generate.rb b/lib/command/terraform/generate.rb index f4ba7442..55141dd0 100644 --- a/lib/command/terraform/generate.rb +++ b/lib/command/terraform/generate.rb @@ -2,18 +2,13 @@ module Command module Terraform - class Generate < ::Command::Base - SUBCOMMAND = "terraform" + class Generate < Base + SUBCOMMAND_NAME = "terraform" NAME = "generate" DESCRIPTION = "Generates terraform configuration files" LONG_DESCRIPTION = <<~DESC - Generates terraform configuration files based on `controlplane.yml` and `templates/` config + - Generates terraform configuration files based on `controlplane.yml` and `templates/` config DESC - EXAMPLES = <<~EX - ```sh - cpflow terraform generate - ``` - EX WITH_INFO_HEADER = false VALIDATIONS = [].freeze diff --git a/lib/cpflow.rb b/lib/cpflow.rb index 5325dd56..e4123fb3 100644 --- a/lib/cpflow.rb +++ b/lib/cpflow.rb @@ -115,7 +115,7 @@ def self.fix_help_option matches = help_mappings & ARGV # Help option works correctly for subcommands - return if matches && (ARGV & subcommand_names).any? + return if matches && subcommand? matches.each do |match| ARGV.delete(match) @@ -123,6 +123,11 @@ def self.fix_help_option end end + def self.subcommand? + (subcommand_names & ARGV).any? + end + private_class_method :subcommand? + # Needed to silence deprecation warning def self.exit_on_failure? true @@ -154,7 +159,7 @@ def self.all_base_commands end def self.subcommand_names - ::Command::Base.all_commands.values.map { |command| command::SUBCOMMAND }.compact + Dir["#{__dir__}/command/*"].filter_map { |name| File.basename(name) if File.directory?(name) } end def self.process_option_params(params) @@ -165,13 +170,13 @@ def self.process_option_params(params) params end - def self.klass_for(subcommand) - klass_name = subcommand.to_s.split("-").collect(&:capitalize).join + def self.klass_for(subcommand_name) + klass_name = subcommand_name.to_s.split("-").map(&:capitalize).join return Cpflow.const_get(klass_name) if Cpflow.const_defined?(klass_name) Cpflow.const_set(klass_name, Class.new(BaseSubCommand)).tap do |subcommand_klass| - desc subcommand, "#{subcommand.capitalize} commands" - subcommand subcommand, subcommand_klass + desc(subcommand_name, "#{subcommand_name.capitalize} commands") + subcommand(subcommand_name, subcommand_klass) end end private_class_method :klass_for @@ -188,6 +193,7 @@ def self.klass_for(subcommand) deprecated = deprecated_commands[command_key] name = command_class::NAME + subcommand_name = command_class::SUBCOMMAND_NAME name_for_method = deprecated ? command_key : name.tr("-", "_") usage = command_class::USAGE.empty? ? name : command_class::USAGE requires_args = command_class::REQUIRES_ARGS @@ -200,7 +206,6 @@ def self.klass_for(subcommand) hide = command_class::HIDE || deprecated with_info_header = command_class::WITH_INFO_HEADER validations = command_class::VALIDATIONS - subcommand = command_class::SUBCOMMAND long_description += "\n#{examples}" if examples.length.positive? @@ -214,7 +219,7 @@ def self.klass_for(subcommand) @commands_with_extra_options.push(name_for_method.to_sym) if accepts_extra_options - klass = subcommand ? klass_for(subcommand) : self + klass = subcommand_name ? klass_for(subcommand_name) : self klass.class_eval do desc(usage, description, hide: hide) diff --git a/script/update_command_docs b/script/update_command_docs index 74e7dea2..8ab78c34 100755 --- a/script/update_command_docs +++ b/script/update_command_docs @@ -12,12 +12,16 @@ commands.keys.sort.each do |command_key| next if command_class::HIDE name = command_class::NAME - usage = command_class::USAGE.empty? ? name : command_class::USAGE + subcommand_name = command_class::SUBCOMMAND_NAME + + full_command = [subcommand_name, name].compact.join(" ") + + usage = command_class::USAGE.empty? ? full_command : command_class::USAGE options = command_class::OPTIONS long_description = command_class::LONG_DESCRIPTION examples = command_class::EXAMPLES - command_str = "### `#{name}`\n\n" + command_str = "### `#{full_command}`\n\n" command_str += "#{long_description.strip}\n\n" if examples.empty? diff --git a/spec/cpflow_spec.rb b/spec/cpflow_spec.rb index de57206f..761117d5 100644 --- a/spec/cpflow_spec.rb +++ b/spec/cpflow_spec.rb @@ -21,17 +21,20 @@ end it "handles subcommands correctly" do - result = run_cpflow_command("help") + result = run_cpflow_command("--help") expect(result[:status]).to eq(0) + # Temporary solution, will be fixed with https://github.com/rails/thor/issues/742 + basename = Cpflow::Cli.send(:basename) + Cpflow::Cli.subcommand_names.each do |subcommand| - expect(result[:stdout]).to include("#{package_name} #{subcommand}") + expect(result[:stdout]).to include("#{basename} #{subcommand}") - subcommand_result = run_cpflow_command(subcommand, "help") + subcommand_result = run_cpflow_command(subcommand, "--help") expect(subcommand_result[:status]).to eq(0) - expect(subcommand_result[:stdout]).to include("#{package_name} #{subcommand} help [COMMAND]") + expect(subcommand_result[:stdout]).to include("#{basename} #{subcommand} help [COMMAND]") end end end diff --git a/spec/support/command_helpers.rb b/spec/support/command_helpers.rb index 95afc82f..6e275083 100644 --- a/spec/support/command_helpers.rb +++ b/spec/support/command_helpers.rb @@ -157,9 +157,6 @@ def create_app_if_not_exists(app, deploy: false, image_before_deploy_count: 0, i end def run_cpflow_command(*args, raise_errors: false) # rubocop:disable Metrics/MethodLength - program_name_before = $PROGRAM_NAME - $PROGRAM_NAME = package_name - LogHelpers.write_command_to_log(args.join(" ")) result = { @@ -185,8 +182,6 @@ def run_cpflow_command(*args, raise_errors: false) # rubocop:disable Metrics/Met raise result.to_json if result[:status].nonzero? && raise_errors result - ensure - $PROGRAM_NAME = program_name_before end def run_cpflow_command!(*args) @@ -262,8 +257,4 @@ def spec_directory File.dirname(current_directory) end - - def package_name - @package_name ||= Cpflow::Cli.instance_variable_get("@package_name") - end end