-
Notifications
You must be signed in to change notification settings - Fork 589
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
[Fix #570] Change of foreign key should be considered as a column change #678
[Fix #570] Change of foreign key should be considered as a column change #678
Conversation
@@ -515,7 +515,7 @@ def annotate_one_file(file_name, info_block, position, options = {}) | |||
old_header = old_content.match(header_pattern).to_s | |||
new_header = info_block.match(header_pattern).to_s | |||
|
|||
column_pattern = /^#[\t ]+[\w\*`]+[\t ]+.+$/ | |||
column_pattern = /^#[\t ]+[\w\*\.`]+[\t ]+.+$/ |
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.
Could you leave a note as to why this would fix the problem?
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.
@ctran Line with a foreign key info in an annotation text looks like:
# fk_rails_... (foreign_thing_id => foreign_things.id) ON DELETE => restrict
It didn't match the previous pattern because of dots at the end of the foreign key name (we have dots there because we truncate generated foreign key names at https://github.com/ctran/annotate_models/blob/develop/lib/annotate/annotate_models.rb#L477).
In turn, because a foreign key line didn't match the pattern, we didn't check it for changes at https://github.com/ctran/annotate_models/blob/develop/lib/annotate/annotate_models.rb#L525
Changes in the pattern solves this case.
Hope this helps.
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.
Does it mean if the name doesn't end with "...", this pattern won't match anymore?
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.
@ctran No. [\w\*\.`]
- this part of the regexp represents characters which a name can consist of, in any order. It will match both names with "..." and without them.
Thanks! |
Fixes #570