[Fix #630] do not re-annotate when wrapper matches column_pattern

Author: sashabelozerov

lib/annotate/annotate_models.rb

@@ -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" : ""
+      wrapper_close = options[:wrapper_close] ? "#{wrapper_line(options[:wrapper_close])}\n" : ""
       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)
+      # 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]
+        old_columns.delete(wrapper_line(options[:wrapper_close])) if options[:wrapper_close]
+      end
+      if new_header
+        new_columns = new_header.scan(column_pattern).sort
+        new_columns.delete(wrapper_line(options[:wrapper_open])) if options[:wrapper_open]
+        new_columns.delete(wrapper_line(options[:wrapper_close])) if options[:wrapper_close]
+      end
+
+      old_columns == new_columns
+    end
   end
 
   class BadModelFileError < LoadError

spec/lib/annotate/annotate_models_spec.rb

@@ -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)
 
       # 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
+        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}")
+
+        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)