From 407ace641c637567372c98f32a6af10bbb1bc901 Mon Sep 17 00:00:00 2001 From: Shu Fujita Date: Tue, 28 Jan 2020 02:30:43 +0900 Subject: [PATCH] Refactor Annotate::Parser (#742) # Summary * Removed unnecessary variables * Renamed variables that have same name as reserved word (`p`) * Other tiny fixes --- lib/annotate/parser.rb | 186 ++++++++++++++++++++++++++--------------- 1 file changed, 119 insertions(+), 67 deletions(-) diff --git a/lib/annotate/parser.rb b/lib/annotate/parser.rb index 82d80161e..cb27b8b5d 100644 --- a/lib/annotate/parser.rb +++ b/lib/annotate/parser.rb @@ -9,6 +9,11 @@ def self.parse(args, env = {}) attr_reader :args, :options, :env + DEFAULT_OPTIONS = { + target_action: :do_annotations, + exit: false + }.freeze + ANNOTATION_POSITIONS = %w[before top after bottom].freeze FILE_TYPE_POSITIONS = %w[position_in_class position_in_factory position_in_fixture position_in_test position_in_routes position_in_serializer].freeze EXCLUSION_LIST = %w[tests fixtures factories serializers].freeze @@ -16,7 +21,7 @@ def self.parse(args, env = {}) def initialize(args, env) @args = args - @options = default_options + @options = DEFAULT_OPTIONS.dup @env = env end @@ -45,114 +50,150 @@ def commit def add_options_to_parser(option_parser) # rubocop:disable Metrics/MethodLength has_set_position = {} - positions = ANNOTATION_POSITIONS option_parser.banner = 'Usage: annotate [options] [model_file]*' - option_parser.on('--additional-file-patterns path1,path2,path3', Array, "Additional file paths or globs to annotate, separated by commas (e.g. `/foo/bar/%model_name%/*.rb,/baz/%model_name%.rb`)") do |additional_file_patterns| + option_parser.on('--additional-file-patterns path1,path2,path3', + Array, + "Additional file paths or globs to annotate, separated by commas (e.g. `/foo/bar/%model_name%/*.rb,/baz/%model_name%.rb`)") do |additional_file_patterns| ENV['additional_file_patterns'] = additional_file_patterns end - option_parser.on('-d', '--delete', 'Remove annotations from all model files or the routes.rb file') do + option_parser.on('-d', + '--delete', + 'Remove annotations from all model files or the routes.rb file') do @options[:target_action] = :remove_annotations end - option_parser.on('-p', '--position [before|top|after|bottom]', positions, - 'Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s)') do |p| - env['position'] = p + option_parser.on('-p', + '--position [before|top|after|bottom]', + ANNOTATION_POSITIONS, + 'Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s)') do |position| + env['position'] = position FILE_TYPE_POSITIONS.each do |key| - env[key] = p unless has_set_position[key] + env[key] = position unless has_set_position[key] end end - option_parser.on('--pc', '--position-in-class [before|top|after|bottom]', positions, - 'Place the annotations at the top (before) or the bottom (after) of the model file') do |p| - env['position_in_class'] = p + option_parser.on('--pc', + '--position-in-class [before|top|after|bottom]', + ANNOTATION_POSITIONS, + 'Place the annotations at the top (before) or the bottom (after) of the model file') do |position_in_class| + env['position_in_class'] = position_in_class has_set_position['position_in_class'] = true end - option_parser.on('--pf', '--position-in-factory [before|top|after|bottom]', positions, - 'Place the annotations at the top (before) or the bottom (after) of any factory files') do |p| - env['position_in_factory'] = p + option_parser.on('--pf', + '--position-in-factory [before|top|after|bottom]', + ANNOTATION_POSITIONS, + 'Place the annotations at the top (before) or the bottom (after) of any factory files') do |position_in_factory| + env['position_in_factory'] = position_in_factory has_set_position['position_in_factory'] = true end - option_parser.on('--px', '--position-in-fixture [before|top|after|bottom]', positions, - 'Place the annotations at the top (before) or the bottom (after) of any fixture files') do |p| - env['position_in_fixture'] = p + option_parser.on('--px', + '--position-in-fixture [before|top|after|bottom]', + ANNOTATION_POSITIONS, + 'Place the annotations at the top (before) or the bottom (after) of any fixture files') do |position_in_fixture| + env['position_in_fixture'] = position_in_fixture has_set_position['position_in_fixture'] = true end - option_parser.on('--pt', '--position-in-test [before|top|after|bottom]', positions, - 'Place the annotations at the top (before) or the bottom (after) of any test files') do |p| - env['position_in_test'] = p + option_parser.on('--pt', + '--position-in-test [before|top|after|bottom]', + ANNOTATION_POSITIONS, + 'Place the annotations at the top (before) or the bottom (after) of any test files') do |position_in_test| + env['position_in_test'] = position_in_test has_set_position['position_in_test'] = true end - option_parser.on('--pr', '--position-in-routes [before|top|after|bottom]', positions, - 'Place the annotations at the top (before) or the bottom (after) of the routes.rb file') do |p| - env['position_in_routes'] = p + option_parser.on('--pr', + '--position-in-routes [before|top|after|bottom]', + ANNOTATION_POSITIONS, + 'Place the annotations at the top (before) or the bottom (after) of the routes.rb file') do |position_in_routes| + env['position_in_routes'] = position_in_routes has_set_position['position_in_routes'] = true end - option_parser.on('--ps', '--position-in-serializer [before|top|after|bottom]', positions, - 'Place the annotations at the top (before) or the bottom (after) of the serializer files') do |p| - env['position_in_serializer'] = p + option_parser.on('--ps', + '--position-in-serializer [before|top|after|bottom]', + ANNOTATION_POSITIONS, + 'Place the annotations at the top (before) or the bottom (after) of the serializer files') do |position_in_serializer| + env['position_in_serializer'] = position_in_serializer has_set_position['position_in_serializer'] = true end - option_parser.on('--w', '--wrapper STR', 'Wrap annotation with the text passed as parameter.', - 'If --w option is used, the same text will be used as opening and closing') do |p| - env['wrapper'] = p + option_parser.on('--w', + '--wrapper STR', + 'Wrap annotation with the text passed as parameter.', + 'If --w option is used, the same text will be used as opening and closing') do |wrapper| + env['wrapper'] = wrapper end - option_parser.on('--wo', '--wrapper-open STR', 'Annotation wrapper opening.') do |p| - env['wrapper_open'] = p + option_parser.on('--wo', + '--wrapper-open STR', + 'Annotation wrapper opening.') do |wrapper_open| + env['wrapper_open'] = wrapper_open end - option_parser.on('--wc', '--wrapper-close STR', 'Annotation wrapper closing') do |p| - env['wrapper_close'] = p + option_parser.on('--wc', + '--wrapper-close STR', + 'Annotation wrapper closing') do |wrapper_close| + env['wrapper_close'] = wrapper_close end - option_parser.on('-r', '--routes', "Annotate routes.rb with the output of 'rake routes'") do + option_parser.on('-r', + '--routes', + "Annotate routes.rb with the output of 'rake routes'") do env['routes'] = 'true' end - option_parser.on('--models', "Annotate ActiveRecord models") do + option_parser.on('--models', + "Annotate ActiveRecord models") do env['models'] = 'true' end - option_parser.on('-a', '--active-admin', 'Annotate active_admin models') do + option_parser.on('-a', + '--active-admin', + 'Annotate active_admin models') do env['active_admin'] = 'true' end - option_parser.on('-v', '--version', 'Show the current version of this gem') do + option_parser.on('-v', + '--version', + 'Show the current version of this gem') do puts "annotate v#{Annotate.version}" @options[:exit] = true end - option_parser.on('-m', '--show-migration', 'Include the migration version number in the annotation') do + option_parser.on('-m', + '--show-migration', + 'Include the migration version number in the annotation') do env['include_version'] = 'yes' end - option_parser.on('-k', '--show-foreign-keys', + option_parser.on('-k', + '--show-foreign-keys', "List the table's foreign key constraints in the annotation") do env['show_foreign_keys'] = 'yes' end option_parser.on('--ck', - '--complete-foreign-keys', 'Complete foreign key names in the annotation') do + '--complete-foreign-keys', + 'Complete foreign key names in the annotation') do env['show_foreign_keys'] = 'yes' env['show_complete_foreign_keys'] = 'yes' end - option_parser.on('-i', '--show-indexes', + option_parser.on('-i', + '--show-indexes', "List the table's database indexes in the annotation") do env['show_indexes'] = 'yes' end - option_parser.on('-s', '--simple-indexes', + option_parser.on('-s', + '--simple-indexes', "Concat the column's related indexes in the annotation") do env['simple_indexes'] = 'yes' end @@ -168,84 +209,95 @@ def add_options_to_parser(option_parser) # rubocop:disable Metrics/MethodLength end option_parser.on('--ignore-model-subdirects', - "Ignore subdirectories of the models directory") do |_dir| + "Ignore subdirectories of the models directory") do env['ignore_model_sub_dir'] = 'yes' end option_parser.on('--sort', - "Sort columns alphabetically, rather than in creation order") do |_dir| + "Sort columns alphabetically, rather than in creation order") do env['sort'] = 'yes' end option_parser.on('--classified-sort', - "Sort columns alphabetically, but first goes id, then the rest columns, then the timestamp columns and then the association columns") do |_dir| + "Sort columns alphabetically, but first goes id, then the rest columns, then the timestamp columns and then the association columns") do env['classified_sort'] = 'yes' end - option_parser.on('-R', '--require path', + option_parser.on('-R', + '--require path', "Additional file to require before loading models, may be used multiple times") do |path| env['require'] = if env['require'].present? - env['require'] + ",#{path}" + "#{env['require']},#{path}" else path end end - option_parser.on('-e', '--exclude [tests,fixtures,factories,serializers]', Array, "Do not annotate fixtures, test files, factories, and/or serializers") do |exclusions| + option_parser.on('-e', + '--exclude [tests,fixtures,factories,serializers]', + Array, + "Do not annotate fixtures, test files, factories, and/or serializers") do |exclusions| exclusions ||= EXCLUSION_LIST exclusions.each { |exclusion| env["exclude_#{exclusion}"] = 'yes' } end - option_parser.on('-f', '--format [bare|rdoc|yard|markdown]', FORMAT_TYPES, 'Render Schema Infomation as plain/RDoc/Yard/Markdown') do |fmt| - env["format_#{fmt}"] = 'yes' + option_parser.on('-f', + '--format [bare|rdoc|yard|markdown]', + FORMAT_TYPES, + 'Render Schema Infomation as plain/RDoc/Yard/Markdown') do |format_type| + env["format_#{format_type}"] = 'yes' end - option_parser.on('--force', 'Force new annotations even if there are no changes.') do |_force| + option_parser.on('--force', + 'Force new annotations even if there are no changes.') do env['force'] = 'yes' end - option_parser.on('--frozen', 'Do not allow to change annotations. Exits non-zero if there are going to be changes to files.') do + option_parser.on('--frozen', + 'Do not allow to change annotations. Exits non-zero if there are going to be changes to files.') do env['frozen'] = 'yes' end - option_parser.on('--timestamp', 'Include timestamp in (routes) annotation') do + option_parser.on('--timestamp', + 'Include timestamp in (routes) annotation') do env['timestamp'] = 'true' end - option_parser.on('--trace', 'If unable to annotate a file, print the full stack trace, not just the exception message.') do |_value| + option_parser.on('--trace', + 'If unable to annotate a file, print the full stack trace, not just the exception message.') do env['trace'] = 'yes' end - option_parser.on('-I', '--ignore-columns REGEX', "don't annotate columns that match a given REGEX (i.e., `annotate -I '^(id|updated_at|created_at)'`") do |regex| + option_parser.on('-I', + '--ignore-columns REGEX', + "don't annotate columns that match a given REGEX (i.e., `annotate -I '^(id|updated_at|created_at)'`") do |regex| env['ignore_columns'] = regex end - option_parser.on('--ignore-routes REGEX', "don't annotate routes that match a given REGEX (i.e., `annotate -I '(mobile|resque|pghero)'`") do |regex| + option_parser.on('--ignore-routes REGEX', + "don't annotate routes that match a given REGEX (i.e., `annotate -I '(mobile|resque|pghero)'`") do |regex| env['ignore_routes'] = regex end - option_parser.on('--hide-limit-column-types VALUES', "don't show limit for given column types, separated by commas (i.e., `integer,boolean,text`)") do |values| + option_parser.on('--hide-limit-column-types VALUES', + "don't show limit for given column types, separated by commas (i.e., `integer,boolean,text`)") do |values| env['hide_limit_column_types'] = values.to_s end - option_parser.on('--hide-default-column-types VALUES', "don't show default for given column types, separated by commas (i.e., `json,jsonb,hstore`)") do |values| + option_parser.on('--hide-default-column-types VALUES', + "don't show default for given column types, separated by commas (i.e., `json,jsonb,hstore`)") do |values| env['hide_default_column_types'] = values.to_s end - option_parser.on('--ignore-unknown-models', "don't display warnings for bad model files") do |_values| + option_parser.on('--ignore-unknown-models', + "don't display warnings for bad model files") do env['ignore_unknown_models'] = 'true' end - option_parser.on('--with-comment', "include database comments in model annotations") do |_values| + option_parser.on('--with-comment', + "include database comments in model annotations") do env['with_comment'] = 'true' end end - - def default_options - { - target_action: :do_annotations, - exit: false - } - end end end