Skip to content

Commit 230b809

Browse files
drwldrwl
committed
Refactor Parser
Refactored Parser to isolate changes being made to ENV. This way we have an intermediate step where we know the environment variables being set.
1 parent f95913b commit 230b809

File tree

2 files changed

+168
-155
lines changed

2 files changed

+168
-155
lines changed

‎lib/annotate/parser.rb

Lines changed: 166 additions & 153 deletions
Original file line numberDiff line numberDiff line change
@@ -3,220 +3,233 @@
33
module Annotate
44
# Class for handling command line arguments
55
class Parser # rubocop:disable Metrics/ClassLength
6-
def self.parse(args)
7-
new(args).parse
6+
def self.parse(args, env = {})
7+
new(args, env).parse
88
end
99

10-
attr_reader :args
10+
attr_reader :args, :options, :env
1111

1212
ANNOTATION_POSITIONS = %w[before top after bottom].freeze
1313
FILE_TYPE_POSITIONS = %w[position_in_class position_in_factory position_in_fixture position_in_test position_in_routes position_in_serializer].freeze
1414
EXCLUSION_LIST = %w[tests fixtures factories serializers].freeze
1515
FORMAT_TYPES = %w[bare rdoc markdown].freeze
1616

17-
def initialize(args)
17+
def initialize(args, env)
1818
@args = args
19+
@options = default_options
20+
@env = env
1921
end
2022

2123
def parse
22-
options = default_options
24+
# To split up because right now this method parses and commits
25+
parser.parse!(args)
2326

24-
parser(options).parse!(args)
27+
commit
2528

2629
options
2730
end
2831

2932
private
3033

31-
def parser(options) # rubocop:disable Metrics/MethodLength
34+
def commit
35+
env.each_pair do |key, value|
36+
ENV[key] = value
37+
end
38+
end
39+
40+
def parser
41+
OptionParser.new do |option_parser|
42+
add_options_to_parser(option_parser)
43+
end
44+
end
45+
46+
def add_options_to_parser(option_parser) # rubocop:disable Metrics/MethodLength
3247
has_set_position = {}
3348
positions = ANNOTATION_POSITIONS
3449

35-
OptionParser.new do |opts|
36-
opts.banner = 'Usage: annotate [options] [model_file]*'
50+
option_parser.banner = 'Usage: annotate [options] [model_file]*'
3751

38-
opts.on('-d', '--delete', 'Remove annotations from all model files or the routes.rb file') do
39-
options[:target_action] = :remove_annotations
40-
end
52+
option_parser.on('-d', '--delete', 'Remove annotations from all model files or the routes.rb file') do
53+
@options[:target_action] = :remove_annotations
54+
end
4155

42-
opts.on('-p', '--position [before|top|after|bottom]', positions,
43-
'Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s)') do |p|
44-
ENV['position'] = p
56+
option_parser.on('-p', '--position [before|top|after|bottom]', positions,
57+
'Place the annotations at the top (before) or the bottom (after) of the model/test/fixture/factory/route/serializer file(s)') do |p|
58+
env['position'] = p
4559

46-
FILE_TYPE_POSITIONS.each do |key|
47-
ENV[key] = p unless has_set_position[key]
48-
end
60+
FILE_TYPE_POSITIONS.each do |key|
61+
env[key] = p unless has_set_position[key]
4962
end
63+
end
5064

51-
opts.on('--pc', '--position-in-class [before|top|after|bottom]', positions,
52-
'Place the annotations at the top (before) or the bottom (after) of the model file') do |p|
53-
ENV['position_in_class'] = p
54-
has_set_position['position_in_class'] = true
55-
end
65+
option_parser.on('--pc', '--position-in-class [before|top|after|bottom]', positions,
66+
'Place the annotations at the top (before) or the bottom (after) of the model file') do |p|
67+
env['position_in_class'] = p
68+
has_set_position['position_in_class'] = true
69+
end
5670

57-
opts.on('--pf', '--position-in-factory [before|top|after|bottom]', positions,
58-
'Place the annotations at the top (before) or the bottom (after) of any factory files') do |p|
59-
ENV['position_in_factory'] = p
60-
has_set_position['position_in_factory'] = true
61-
end
71+
option_parser.on('--pf', '--position-in-factory [before|top|after|bottom]', positions,
72+
'Place the annotations at the top (before) or the bottom (after) of any factory files') do |p|
73+
env['position_in_factory'] = p
74+
has_set_position['position_in_factory'] = true
75+
end
6276

63-
opts.on('--px', '--position-in-fixture [before|top|after|bottom]', positions,
64-
'Place the annotations at the top (before) or the bottom (after) of any fixture files') do |p|
65-
ENV['position_in_fixture'] = p
66-
has_set_position['position_in_fixture'] = true
67-
end
77+
option_parser.on('--px', '--position-in-fixture [before|top|after|bottom]', positions,
78+
'Place the annotations at the top (before) or the bottom (after) of any fixture files') do |p|
79+
env['position_in_fixture'] = p
80+
has_set_position['position_in_fixture'] = true
81+
end
6882

69-
opts.on('--pt', '--position-in-test [before|top|after|bottom]', positions,
70-
'Place the annotations at the top (before) or the bottom (after) of any test files') do |p|
71-
ENV['position_in_test'] = p
72-
has_set_position['position_in_test'] = true
73-
end
83+
option_parser.on('--pt', '--position-in-test [before|top|after|bottom]', positions,
84+
'Place the annotations at the top (before) or the bottom (after) of any test files') do |p|
85+
env['position_in_test'] = p
86+
has_set_position['position_in_test'] = true
87+
end
7488

75-
opts.on('--pr', '--position-in-routes [before|top|after|bottom]', positions,
76-
'Place the annotations at the top (before) or the bottom (after) of the routes.rb file') do |p|
77-
ENV['position_in_routes'] = p
78-
has_set_position['position_in_routes'] = true
79-
end
89+
option_parser.on('--pr', '--position-in-routes [before|top|after|bottom]', positions,
90+
'Place the annotations at the top (before) or the bottom (after) of the routes.rb file') do |p|
91+
env['position_in_routes'] = p
92+
has_set_position['position_in_routes'] = true
93+
end
8094

81-
opts.on('--ps', '--position-in-serializer [before|top|after|bottom]', positions,
82-
'Place the annotations at the top (before) or the bottom (after) of the serializer files') do |p|
83-
ENV['position_in_serializer'] = p
84-
has_set_position['position_in_serializer'] = true
85-
end
95+
option_parser.on('--ps', '--position-in-serializer [before|top|after|bottom]', positions,
96+
'Place the annotations at the top (before) or the bottom (after) of the serializer files') do |p|
97+
env['position_in_serializer'] = p
98+
has_set_position['position_in_serializer'] = true
99+
end
86100

87-
opts.on('--w', '--wrapper STR', 'Wrap annotation with the text passed as parameter.',
88-
'If --w option is used, the same text will be used as opening and closing') do |p|
89-
ENV['wrapper'] = p
90-
end
101+
option_parser.on('--w', '--wrapper STR', 'Wrap annotation with the text passed as parameter.',
102+
'If --w option is used, the same text will be used as opening and closing') do |p|
103+
env['wrapper'] = p
104+
end
91105

92-
opts.on('--wo', '--wrapper-open STR', 'Annotation wrapper opening.') do |p|
93-
ENV['wrapper_open'] = p
94-
end
106+
option_parser.on('--wo', '--wrapper-open STR', 'Annotation wrapper opening.') do |p|
107+
env['wrapper_open'] = p
108+
end
95109

96-
opts.on('--wc', '--wrapper-close STR', 'Annotation wrapper closing') do |p|
97-
ENV['wrapper_close'] = p
98-
end
110+
option_parser.on('--wc', '--wrapper-close STR', 'Annotation wrapper closing') do |p|
111+
env['wrapper_close'] = p
112+
end
99113

100-
opts.on('-r', '--routes', "Annotate routes.rb with the output of 'rake routes'") do
101-
ENV['routes'] = 'true'
102-
end
114+
option_parser.on('-r', '--routes', "Annotate routes.rb with the output of 'rake routes'") do
115+
env['routes'] = 'true'
116+
end
103117

104-
opts.on('-a', '--active-admin', 'Annotate active_admin models') do
105-
ENV['active_admin'] = 'true'
106-
end
118+
option_parser.on('-a', '--active-admin', 'Annotate active_admin models') do
119+
env['active_admin'] = 'true'
120+
end
107121

108-
opts.on('-v', '--version', 'Show the current version of this gem') do
109-
puts "annotate v#{Annotate.version}"
110-
options[:exit] = true
111-
end
122+
option_parser.on('-v', '--version', 'Show the current version of this gem') do
123+
puts "annotate v#{Annotate.version}"
124+
@options[:exit] = true
125+
end
112126

113-
opts.on('-m', '--show-migration', 'Include the migration version number in the annotation') do
114-
ENV['include_version'] = 'yes'
115-
end
127+
option_parser.on('-m', '--show-migration', 'Include the migration version number in the annotation') do
128+
env['include_version'] = 'yes'
129+
end
116130

117-
opts.on('-k', '--show-foreign-keys',
118-
"List the table's foreign key constraints in the annotation") do
119-
ENV['show_foreign_keys'] = 'yes'
120-
end
131+
option_parser.on('-k', '--show-foreign-keys',
132+
"List the table's foreign key constraints in the annotation") do
133+
env['show_foreign_keys'] = 'yes'
134+
end
121135

122-
opts.on('--ck',
123-
'--complete-foreign-keys', 'Complete foreign key names in the annotation') do
124-
ENV['show_foreign_keys'] = 'yes'
125-
ENV['show_complete_foreign_keys'] = 'yes'
126-
end
136+
option_parser.on('--ck',
137+
'--complete-foreign-keys', 'Complete foreign key names in the annotation') do
138+
env['show_foreign_keys'] = 'yes'
139+
env['show_complete_foreign_keys'] = 'yes'
140+
end
127141

128-
opts.on('-i', '--show-indexes',
129-
"List the table's database indexes in the annotation") do
130-
ENV['show_indexes'] = 'yes'
131-
end
142+
option_parser.on('-i', '--show-indexes',
143+
"List the table's database indexes in the annotation") do
144+
env['show_indexes'] = 'yes'
145+
end
132146

133-
opts.on('-s', '--simple-indexes',
134-
"Concat the column's related indexes in the annotation") do
135-
ENV['simple_indexes'] = 'yes'
136-
end
147+
option_parser.on('-s', '--simple-indexes',
148+
"Concat the column's related indexes in the annotation") do
149+
env['simple_indexes'] = 'yes'
150+
end
137151

138-
opts.on('--model-dir dir',
139-
"Annotate model files stored in dir rather than app/models, separate multiple dirs with commas") do |dir|
140-
ENV['model_dir'] = dir
141-
end
152+
option_parser.on('--model-dir dir',
153+
"Annotate model files stored in dir rather than app/models, separate multiple dirs with commas") do |dir|
154+
env['model_dir'] = dir
155+
end
142156

143-
opts.on('--root-dir dir',
144-
"Annotate files stored within root dir projects, separate multiple dirs with commas") do |dir|
145-
ENV['root_dir'] = dir
146-
end
157+
option_parser.on('--root-dir dir',
158+
"Annotate files stored within root dir projects, separate multiple dirs with commas") do |dir|
159+
env['root_dir'] = dir
160+
end
147161

148-
opts.on('--ignore-model-subdirects',
149-
"Ignore subdirectories of the models directory") do |_dir|
150-
ENV['ignore_model_sub_dir'] = 'yes'
151-
end
162+
option_parser.on('--ignore-model-subdirects',
163+
"Ignore subdirectories of the models directory") do |_dir|
164+
env['ignore_model_sub_dir'] = 'yes'
165+
end
152166

153-
opts.on('--sort',
154-
"Sort columns alphabetically, rather than in creation order") do |_dir|
155-
ENV['sort'] = 'yes'
156-
end
167+
option_parser.on('--sort',
168+
"Sort columns alphabetically, rather than in creation order") do |_dir|
169+
env['sort'] = 'yes'
170+
end
157171

158-
opts.on('--classified-sort',
159-
"Sort columns alphabetically, but first goes id, then the rest columns, then the timestamp columns and then the association columns") do |_dir|
160-
ENV['classified_sort'] = 'yes'
161-
end
172+
option_parser.on('--classified-sort',
173+
"Sort columns alphabetically, but first goes id, then the rest columns, then the timestamp columns and then the association columns") do |_dir|
174+
env['classified_sort'] = 'yes'
175+
end
162176

163-
opts.on('-R', '--require path',
164-
"Additional file to require before loading models, may be used multiple times") do |path|
165-
ENV['require'] = if !ENV['require'].blank?
166-
ENV['require'] + ",#{path}"
167-
else
168-
path
169-
end
170-
end
177+
option_parser.on('-R', '--require path',
178+
"Additional file to require before loading models, may be used multiple times") do |path|
179+
env['require'] = if !env['require'].blank?
180+
env['require'] + ",#{path}"
181+
else
182+
path
183+
end
184+
end
171185

172-
opts.on('-e', '--exclude [tests,fixtures,factories,serializers]', Array, "Do not annotate fixtures, test files, factories, and/or serializers") do |exclusions|
173-
exclusions ||= EXCLUSION_LIST
174-
exclusions.each { |exclusion| ENV["exclude_#{exclusion}"] = 'yes' }
175-
end
186+
option_parser.on('-e', '--exclude [tests,fixtures,factories,serializers]', Array, "Do not annotate fixtures, test files, factories, and/or serializers") do |exclusions|
187+
exclusions ||= EXCLUSION_LIST
188+
exclusions.each { |exclusion| env["exclude_#{exclusion}"] = 'yes' }
189+
end
176190

177-
opts.on('-f', '--format [bare|rdoc|markdown]', FORMAT_TYPES, 'Render Schema Infomation as plain/RDoc/Markdown') do |fmt|
178-
ENV["format_#{fmt}"] = 'yes'
179-
end
191+
option_parser.on('-f', '--format [bare|rdoc|markdown]', FORMAT_TYPES, 'Render Schema Infomation as plain/RDoc/Markdown') do |fmt|
192+
env["format_#{fmt}"] = 'yes'
193+
end
180194

181-
opts.on('--force', 'Force new annotations even if there are no changes.') do |_force|
182-
ENV['force'] = 'yes'
183-
end
195+
option_parser.on('--force', 'Force new annotations even if there are no changes.') do |_force|
196+
env['force'] = 'yes'
197+
end
184198

185-
opts.on('--frozen', 'Do not allow to change annotations. Exits non-zero if there are going to be changes to files.') do
186-
ENV['frozen'] = 'yes'
187-
end
199+
option_parser.on('--frozen', 'Do not allow to change annotations. Exits non-zero if there are going to be changes to files.') do
200+
env['frozen'] = 'yes'
201+
end
188202

189-
opts.on('--timestamp', 'Include timestamp in (routes) annotation') do
190-
ENV['timestamp'] = 'true'
191-
end
203+
option_parser.on('--timestamp', 'Include timestamp in (routes) annotation') do
204+
env['timestamp'] = 'true'
205+
end
192206

193-
opts.on('--trace', 'If unable to annotate a file, print the full stack trace, not just the exception message.') do |_value|
194-
ENV['trace'] = 'yes'
195-
end
207+
option_parser.on('--trace', 'If unable to annotate a file, print the full stack trace, not just the exception message.') do |_value|
208+
env['trace'] = 'yes'
209+
end
196210

197-
opts.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|
198-
ENV['ignore_columns'] = regex
199-
end
211+
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|
212+
env['ignore_columns'] = regex
213+
end
200214

201-
opts.on('--ignore-routes REGEX', "don't annotate routes that match a given REGEX (i.e., `annotate -I '(mobile|resque|pghero)'`") do |regex|
202-
ENV['ignore_routes'] = regex
203-
end
215+
option_parser.on('--ignore-routes REGEX', "don't annotate routes that match a given REGEX (i.e., `annotate -I '(mobile|resque|pghero)'`") do |regex|
216+
env['ignore_routes'] = regex
217+
end
204218

205-
opts.on('--hide-limit-column-types VALUES', "don't show limit for given column types, separated by commas (i.e., `integer,boolean,text`)") do |values|
206-
ENV['hide_limit_column_types'] = values.to_s
207-
end
219+
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|
220+
env['hide_limit_column_types'] = values.to_s
221+
end
208222

209-
opts.on('--hide-default-column-types VALUES', "don't show default for given column types, separated by commas (i.e., `json,jsonb,hstore`)") do |values|
210-
ENV['hide_default_column_types'] = values.to_s
211-
end
223+
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|
224+
env['hide_default_column_types'] = values.to_s
225+
end
212226

213-
opts.on('--ignore-unknown-models', "don't display warnings for bad model files") do |_values|
214-
ENV['ignore_unknown_models'] = 'true'
215-
end
227+
option_parser.on('--ignore-unknown-models', "don't display warnings for bad model files") do |_values|
228+
env['ignore_unknown_models'] = 'true'
229+
end
216230

217-
opts.on('--with-comment', "include database comments in model annotations") do |_values|
218-
ENV['with_comment'] = 'true'
219-
end
231+
option_parser.on('--with-comment', "include database comments in model annotations") do |_values|
232+
env['with_comment'] = 'true'
220233
end
221234
end
222235

‎spec/annotate/parser_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -345,9 +345,9 @@ module Annotate # rubocop:disable Metrics/ModuleLength
345345
context "when ENV['require'] is already set" do
346346
let(:preset_require_value) { 'some_dir/' }
347347
it "appends the path to ENV['require']" do
348-
allow(ENV).to receive(:[]).and_return(preset_require_value)
348+
env = { 'require' => preset_require_value }
349349
expect(ENV).to receive(:[]=).with(env_key, "#{preset_require_value},#{set_value}")
350-
Parser.parse([option, set_value])
350+
Parser.parse([option, set_value], env)
351351
end
352352
end
353353
end

0 commit comments

Comments
 (0)