-
Notifications
You must be signed in to change notification settings - Fork 658
[Fix #630] do not re-annotate when wrapper matches column_pattern #668
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -512,23 +512,13 @@ def annotate_one_file(file_name, info_block, position, options = {}) | |
return false unless File.exist?(file_name) | ||
old_content = File.read(file_name) | ||
return false if old_content =~ /#{SKIP_ANNOTATION_PREFIX}.*\n/ | ||
|
||
# Ignore the Schema version line because it changes with each migration | ||
header_pattern = /(^# Table name:.*?\n(#.*[\r]?\n)*[\r]?)/ | ||
old_header = old_content.match(header_pattern).to_s | ||
new_header = info_block.match(header_pattern).to_s | ||
|
||
column_pattern = /^#[\t ]+[\w\*`]+[\t ]+.+$/ | ||
old_columns = old_header && old_header.scan(column_pattern).sort | ||
new_columns = new_header && new_header.scan(column_pattern).sort | ||
|
||
return false if old_columns == new_columns && !options[:force] | ||
return false if columns_unchanged?(old_content, info_block, options) && !options[:force] | ||
|
||
abort "annotate error. #{file_name} needs to be updated, but annotate was run with `--frozen`." if options[:frozen] | ||
|
||
# Replace inline the old schema info with the new schema info | ||
wrapper_open = options[:wrapper_open] ? "# #{options[:wrapper_open]}\n" : "" | ||
wrapper_close = options[:wrapper_close] ? "# #{options[:wrapper_close]}\n" : "" | ||
wrapper_open = options[:wrapper_open] ? "#{wrapper_line(options[:wrapper_open])}\n" : "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [94/80] |
||
wrapper_close = options[:wrapper_close] ? "#{wrapper_line(options[:wrapper_close])}\n" : "" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [97/80] |
||
wrapped_info_block = "#{wrapper_open}#{info_block}#{wrapper_close}" | ||
|
||
old_annotation = old_content.match(annotate_pattern(options)).to_s | ||
|
@@ -939,6 +929,34 @@ def mb_chars_ljust(string, length) | |
def non_ascii_length(string) | ||
string.to_s.chars.reject(&:ascii_only?).length | ||
end | ||
|
||
def wrapper_line(wrapper) | ||
"# #{wrapper}" | ||
end | ||
|
||
def columns_unchanged?(old_content, info_block, options) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/AbcSize: Assignment Branch Condition size for columns_unchanged? is too high. [26.93/15] |
||
# Ignore the Schema version line because it changes with each migration | ||
header_pattern = /(^# Table name:.*?\n(#.*[\r]?\n)*[\r]?)/ | ||
old_header = old_content.match(header_pattern).to_s | ||
new_header = info_block.match(header_pattern).to_s | ||
|
||
column_pattern = /^#[\t ]+[\w\*`]+[\t ]+.+$/ | ||
old_columns = [] | ||
new_columns = [] | ||
|
||
if old_header | ||
old_columns = old_header.scan(column_pattern).sort | ||
old_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [90/80] |
||
old_columns.delete(wrapper_line(options[:wrapper_close])) if options[:wrapper_close] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [92/80] |
||
end | ||
if new_header | ||
new_columns = new_header.scan(column_pattern).sort | ||
new_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [90/80] |
||
new_columns.delete(wrapper_line(options[:wrapper_close])) if options[:wrapper_close] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [92/80] |
||
end | ||
|
||
old_columns == new_columns | ||
end | ||
end | ||
|
||
class BadModelFileError < LoadError | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1711,13 +1711,14 @@ def write_model(file_name, file_content) | |
def annotate_one_file(options = {}) | ||
Annotate.set_defaults(options) | ||
options = Annotate.setup_options(options) | ||
AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options) | ||
result = AnnotateModels.annotate_one_file(@model_file_name, @schema_info, :position_in_class, options) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [108/80] |
||
|
||
# Wipe settings so the next call will pick up new values... | ||
Annotate.instance_variable_set('@has_set_defaults', false) | ||
Annotate::POSITION_OPTIONS.each { |key| ENV[key.to_s] = '' } | ||
Annotate::FLAG_OPTIONS.each { |key| ENV[key.to_s] = '' } | ||
Annotate::PATH_OPTIONS.each { |key| ENV[key.to_s] = '' } | ||
result | ||
end | ||
|
||
def magic_comments_list_each | ||
|
@@ -1882,6 +1883,21 @@ class User < ActiveRecord::Base | |
end | ||
end | ||
|
||
describe 'wrapper value matches column pattern' do | ||
let(:wrapper) { 'Schema generated with `annotate`' } | ||
|
||
it 'does not consider this as a new column and does not try to re-annotate' do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [84/80] |
||
res = annotate_one_file wrapper_open: wrapper, wrapper_close: wrapper | ||
expect(res).to eq(true) | ||
|
||
expect(File.read(@model_file_name)) | ||
.to eq("# #{wrapper}\n#{@schema_info}# #{wrapper}\n\n#{@file_content}") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Metrics/LineLength: Line is too long. [81/80] |
||
|
||
res = annotate_one_file wrapper_open: wrapper, wrapper_close: wrapper | ||
expect(res).to eq(false) # Model file unchanged | ||
end | ||
end | ||
|
||
describe "if a file can't be annotated" do | ||
before do | ||
allow(AnnotateModels).to receive(:get_loaded_model_by_path).with('user').and_return(nil) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Metrics/LineLength: Line is too long. [94/80]