Merge pull request #304 from ctran/fix/245

bin/annotate

@@ -18,18 +18,16 @@ require 'optparse'
 require 'annotate'
 Annotate.bootstrap_rake
 
-target = {
-  :klass => AnnotateModels,
-  :task => :do_annotations,
-}
 has_set_position = {}
+target_action = :do_annotations
+
 OptionParser.new do |opts|
   opts.banner = "Usage: annotate [options] [model_file]*"
 
   opts.on('-d', '--delete',
           "Remove annotations from all model files or the routes.rb file") do
 
-    target[:task] = :remove_annotations
+    target_action = :remove_annotations
   end
 
   opts.on('-p', '--position [before|top|after|bottom]', ['before', 'top', 'after', 'bottom'],
@@ -93,10 +91,7 @@ OptionParser.new do |opts|
 
   opts.on('-r', '--routes',
           "Annotate routes.rb with the output of 'rake routes'") do
-    target = {
-      :klass => AnnotateRoutes,
-      :task => :do_annotations
-    }
+    ENV['routes'] = 'true'
   end
 
   opts.on('-v', '--version',
@@ -187,4 +182,6 @@ end.parse!
 
 options = Annotate.setup_options({ :is_rake => ENV['is_rake'] && !ENV['is_rake'].empty? })
 Annotate.eager_load(options)
-target[:klass].send(target[:task], options)
+
+AnnotateModels.send(target_action, options) if Annotate.include_models?
+AnnotateRoutes.send(target_action, options) if Annotate.include_routes?

lib/annotate.rb

@@ -30,7 +30,7 @@ module Annotate
     :exclude_scaffolds, :exclude_controllers, :exclude_helpers
   ]
   OTHER_OPTIONS=[
-    :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close, :wrapper
+    :ignore_columns, :skip_on_db_migrate, :wrapper_open, :wrapper_close, :wrapper, :routes
   ]
   PATH_OPTIONS=[
     :require, :model_dir, :root_dir
@@ -98,6 +98,14 @@ module Annotate
     ENV['skip_on_db_migrate'] =~ TRUE_RE
   end
 
+  def self.include_routes?
+    ENV['routes'] =~ TRUE_RE
+  end
+
+  def self.include_models?
+    true
+  end
+
   def self.loaded_tasks=(val); @loaded_tasks = val; end
   def self.loaded_tasks; return @loaded_tasks; end
 

lib/annotate/annotate_models.rb

@@ -530,7 +530,7 @@ module AnnotateModels
         annotate_model_file(annotated, File.join(file), header, options)
       end
       if annotated.empty?
-        puts "Nothing to annotate."
+        puts "Model files unchanged."
       else
         puts "Annotated (#{annotated.length}): #{annotated.join(', ')}"
       end
@@ -542,7 +542,7 @@ module AnnotateModels
         klass = get_model_class(file)
         if klass && klass < ActiveRecord::Base && !klass.abstract_class? && klass.table_exists?
           if annotate(klass, file, header, options)
-            annotated << klass
+            annotated << file
           end
         end
       rescue Exception => e

lib/annotate/annotate_routes.rb

@@ -37,7 +37,8 @@ module AnnotateRoutes
       "#"
     ] + routes_map.map { |line| "# #{line}".rstrip }
 
-    (content, where_header_found) = strip_annotations(File.read(routes_file))
+    existing_text = File.read(routes_file)
+    (content, where_header_found) = strip_annotations(existing_text)
     changed = where_header_found != 0 # This will either be :before, :after, or
                                       # a number.  If the number is > 0, the
                                       # annotation was found somewhere in the
@@ -60,21 +61,25 @@ module AnnotateRoutes
 
     content = position_after ? (content + header) : header + content
 
-    write_contents(content)
-
-    puts "Route file annotated."
+    if write_contents(existing_text, content)
+      puts "#{routes_file} annotated."
+    else
+      puts "#{routes_file} unchanged."
+    end
   end
 
   def self.remove_annotations(options={})
     return unless(routes_exists?)
-
-    (content, where_header_found) = strip_annotations(File.read(routes_file))
+    existing_text = File.read(routes_file)
+    (content, where_header_found) = strip_annotations(existing_text)
 
     content = strip_on_removal(content, where_header_found)
 
-    write_contents(content)
-
-    puts "Removed annotations from routes file."
+    if write_contents(existing_text, content)
+      puts "Removed annotations from #{routes_file}."
+    else
+      puts "#{routes_file} unchanged."
+    end
   end
 
 protected
@@ -89,11 +94,15 @@ protected
     return routes_exists
   end
 
-  def self.write_contents(content)
-    content << '' unless(content.last == '') # Make sure we end on a trailing
-                                             # newline.
+  def self.write_contents(existing_text, new_content)
+    # Make sure we end on a trailing newline.
+    new_content << '' unless(new_content.last == '')
+    new_text = new_content.join("\n")
+
+    return false if existing_text == new_text
 
-    File.open(routes_file, "wb") { |f| f.puts(content.join("\n")) }
+    File.open(routes_file, "wb") { |f| f.puts(new_text) }
+    return true
   end
 
   def self.strip_annotations(content)

lib/generators/annotate/templates/auto_annotate_models.rake

@@ -6,6 +6,7 @@ if Rails.env.development?
     # You can override any of these by setting an environment variable of the
     # same name.
     Annotate.set_defaults(
+      'routes'                  => 'false',
       'position_in_routes'      => 'before',
       'position_in_class'       => 'before',
       'position_in_test'        => 'before',

lib/tasks/migrate.rake

@@ -29,13 +29,23 @@ module Annotate
     def self.update_annotations
       unless @@working || Annotate.skip_on_migration?
         @@working = true
-        if Rake::Task.task_defined?("annotate_models")
-          Rake::Task["annotate_models"].invoke
-        elsif Rake::Task.task_defined?("app:annotate_models")
-          Rake::Task["app:annotate_models"].invoke
-        else
-          raise "Don't know how to build task 'annotate_models'"
-        end
+
+        self.update_models if Annotate.include_models?
+        self.update_routes if Annotate.include_routes?
+      end
+    end
+
+    def self.update_models
+      if Rake::Task.task_defined?("annotate_models")
+        Rake::Task["annotate_models"].invoke
+      elsif Rake::Task.task_defined?("app:annotate_models")
+        Rake::Task["app:annotate_models"].invoke
+      end
+    end
+
+    def self.update_routes
+      if Rake::Task.task_defined?("annotate_routes")
+        Rake::Task["annotate_routes"].invoke
       end
     end
   end

spec/annotate/annotate_routes_spec.rb

@@ -2,85 +2,114 @@ require File.dirname(__FILE__) + '/../spec_helper.rb'
 require 'annotate/annotate_routes'
 
 describe AnnotateRoutes do
+  ROUTE_FILE = "config/routes.rb"
+  ANNOTATION_ADDED = "#{ROUTE_FILE} annotated."
+  ANNOTATION_REMOVED = "Removed annotations from #{ROUTE_FILE}."
+  FILE_UNCHANGED = "#{ROUTE_FILE} unchanged."
 
   def mock_file(stubs={})
     @mock_file ||= double(File, stubs)
   end
 
   it "should check if routes.rb exists" do
-    expect(File).to receive(:exists?).with("config/routes.rb").and_return(false)
+    expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(false)
     expect(AnnotateRoutes).to receive(:puts).with("Can`t find routes.rb")
     AnnotateRoutes.do_annotations
   end
 
-  describe "When Annotating, with older Rake Versions" do
+  describe "When adding" do
+    before(:each) do
+      expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true)
+      expect(AnnotateRoutes).to receive(:`).with("rake routes").and_return("")
+    end
+
+    it "should insert annotations if file does not contain annotations" do
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("")
+      expect(File).to receive(:open).with(ROUTE_FILE, "wb").and_yield(mock_file)
+      expect(@mock_file).to receive(:puts).with("\n# == Route Map\n#\n")
+      expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED)
+
+      AnnotateRoutes.do_annotations
+    end
+
+    it "should skip annotations if file does already contain annotation" do
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("\n# == Route Map\n#\n")
+      expect(AnnotateRoutes).to receive(:puts).with(FILE_UNCHANGED)
+
+      AnnotateRoutes.do_annotations
+    end
+
+  end
+
+  describe "When adding with older Rake versions" do
 
     before(:each) do
-      expect(File).to receive(:exists?).with("config/routes.rb").and_return(true)
+      expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true)
       expect(AnnotateRoutes).to receive(:`).with("rake routes").and_return("(in /bad/line)\ngood line")
-      expect(File).to receive(:open).with("config/routes.rb", "wb").and_yield(mock_file)
-      expect(AnnotateRoutes).to receive(:puts).with("Route file annotated.")
+      expect(File).to receive(:open).with(ROUTE_FILE, "wb").and_yield(mock_file)
+      expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED)
     end
 
     it "should annotate and add a newline!" do
-      expect(File).to receive(:read).with("config/routes.rb").and_return("ActionController::Routing...\nfoo")
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("ActionController::Routing...\nfoo")
       expect(@mock_file).to receive(:puts).with(/ActionController::Routing...\nfoo\n\n# == Route Map\n#\n# good line\n/)
       AnnotateRoutes.do_annotations
     end
 
     it "should not add a newline if there are empty lines" do
-      expect(File).to receive(:read).with("config/routes.rb").and_return("ActionController::Routing...\nfoo\n")
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("ActionController::Routing...\nfoo\n")
       expect(@mock_file).to receive(:puts).with(/ActionController::Routing...\nfoo\n\n# == Route Map\n#\n# good line\n/)
       AnnotateRoutes.do_annotations
     end
 
   end
 
-  describe "When Annotating, with newer Rake Versions" do
+  describe "When adding with newer Rake versions" do
 
     before(:each) do
-      expect(File).to receive(:exists?).with("config/routes.rb").and_return(true)
+      expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true)
       expect(AnnotateRoutes).to receive(:`).with("rake routes").and_return("another good line\ngood line")
-      expect(File).to receive(:open).with("config/routes.rb", "wb").and_yield(mock_file)
-      expect(AnnotateRoutes).to receive(:puts).with("Route file annotated.")
+      expect(File).to receive(:open).with(ROUTE_FILE, "wb").and_yield(mock_file)
+      expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_ADDED)
     end
 
+
     it "should annotate and add a newline!" do
-      expect(File).to receive(:read).with("config/routes.rb").and_return("ActionController::Routing...\nfoo")
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("ActionController::Routing...\nfoo")
       expect(@mock_file).to receive(:puts).with(/ActionController::Routing...\nfoo\n\n# == Route Map\n#\n# another good line\n# good line\n/)
       AnnotateRoutes.do_annotations
     end
 
     it "should not add a newline if there are empty lines" do
-      expect(File).to receive(:read).with("config/routes.rb").and_return("ActionController::Routing...\nfoo\n")
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("ActionController::Routing...\nfoo\n")
       expect(@mock_file).to receive(:puts).with(/ActionController::Routing...\nfoo\n\n# == Route Map\n#\n# another good line\n# good line\n/)
       AnnotateRoutes.do_annotations
     end
 
     it "should add a timestamp when :timestamp is passed" do
-      expect(File).to receive(:read).with("config/routes.rb").and_return("ActionController::Routing...\nfoo")
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("ActionController::Routing...\nfoo")
       expect(@mock_file).to receive(:puts).with(/ActionController::Routing...\nfoo\n\n# == Route Map \(Updated \d{4}-\d{2}-\d{2} \d{2}:\d{2}\)\n#\n# another good line\n# good line\n/)
       AnnotateRoutes.do_annotations :timestamp => true
     end
 
   end
 
-  describe "When Removing Annotation" do
+  describe "When removing" do
 
     before(:each) do
-      expect(File).to receive(:exists?).with("config/routes.rb").and_return(true)
-      expect(File).to receive(:open).with("config/routes.rb", "wb").and_yield(mock_file)
-      expect(AnnotateRoutes).to receive(:puts).with("Removed annotations from routes file.")
+      expect(File).to receive(:exists?).with(ROUTE_FILE).and_return(true)
+      expect(File).to receive(:open).with(ROUTE_FILE, "wb").and_yield(mock_file)
+      expect(AnnotateRoutes).to receive(:puts).with(ANNOTATION_REMOVED)
     end
 
     it "should remove trailing annotation and trim trailing newlines, but leave leading newlines alone" do
-      expect(File).to receive(:read).with("config/routes.rb").and_return("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n# == Route Map\n#\n# another good line\n# good line\n")
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n# == Route Map\n#\n# another good line\n# good line\n")
       expect(@mock_file).to receive(:puts).with(/\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n/)
       AnnotateRoutes.remove_annotations
     end
 
     it "should remove prepended annotation and trim leading newlines, but leave trailing newlines alone" do
-      expect(File).to receive(:read).with("config/routes.rb").and_return("# == Route Map\n#\n# another good line\n# good line\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n")
+      expect(File).to receive(:read).with(ROUTE_FILE).and_return("# == Route Map\n#\n# another good line\n# good line\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\n\nActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n")
       expect(@mock_file).to receive(:puts).with(/ActionController::Routing...\nfoo\n\n\n\n\n\n\n\n\n\n\n/)
       AnnotateRoutes.remove_annotations
     end