Prettify output of schema_dumper by making things line up. Closes #4241 [Caio Chassot <caio@v2studio.com>]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4273 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 50 insertions, 8 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 67c44f339a..0f7804a896 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,6 +1,9 @@
 *SVN*
 
+* Prettify output of schema_dumper by making things line up. Closes #4241 [Caio  Chassot <caio@v2studio.com>]
+
 * Make build_postgresql_databases task make databases owned by the postgres user. Closes #4790. [mlaster@metavillage.com]
+
 * Sybase Adapter type conversion cleanup. Closes #4736. [dev@metacasa.net]
 
 * Fix bug where calculations with long alias names return null. [Rick]
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 06fdd3f813..718669a1b7 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -82,13 +82,25 @@ HEADER
           tbl.print ", :force => true"
           tbl.puts " do |t|"
 
-          columns.each do |column|
+          column_specs = columns.map do |column|
             raise StandardError, "Unknown type '#{column.sql_type}' for column '#{column.name}'" if @types[column.type].nil?
             next if column.name == pk
-            tbl.print "    t.column #{column.name.inspect}, #{column.type.inspect}"
-            tbl.print ", :limit => #{column.limit.inspect}" if column.limit != @types[column.type][:limit] 
-            tbl.print ", :default => #{column.default.inspect}" if !column.default.nil?
-            tbl.print ", :null => false" if !column.null
+            spec = {}
+            spec[:name]    = column.name.inspect
+            spec[:type]    = column.type.inspect
+            spec[:limit]   = column.limit.inspect if column.limit != @types[column.type][:limit] 
+            spec[:default] = column.default.inspect if !column.default.nil?
+            spec[:null]    = 'false' if !column.null
+            (spec.keys - [:name, :type]).each{ |k| spec[k].insert(0, "#{k.inspect} => ")}
+            spec
+          end.compact
+          keys = [:name, :type, :limit, :default, :null] & column_specs.map{ |spec| spec.keys }.inject([]){ |a,b| a | b }
+          lengths = keys.map{ |key| column_specs.map{ |spec| spec[key] ? spec[key].length + 2 : 0 }.max }
+          format_string = lengths.map{ |len| "%-#{len}s" }.join("")
+          column_specs.each do |colspec|
+            values = keys.zip(lengths).map{ |key, len| colspec.key?(key) ? colspec[key] + ", " : " " * len }
+            tbl.print "    t.column "
+            tbl.print((format_string % values).gsub(/,\s*$/, ''))
             tbl.puts
           end
 
diff --git a/activerecord/test/schema_dumper_test.rb b/activerecord/test/schema_dumper_test.rb
index 5dd0d4e9a2..37e8a99048 100644
--- a/activerecord/test/schema_dumper_test.rb
+++ b/activerecord/test/schema_dumper_test.rb
@@ -5,16 +5,43 @@ require 'stringio'
 if ActiveRecord::Base.connection.respond_to?(:tables)
 
   class SchemaDumperTest < Test::Unit::TestCase
-    def test_schema_dump
+    def standard_dump
       stream = StringIO.new
       ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)
-      output = stream.string
-
+      stream.string
+    end
+    
+    def test_schema_dump
+      output = standard_dump
       assert_match %r{create_table "accounts"}, output
       assert_match %r{create_table "authors"}, output
       assert_no_match %r{create_table "schema_info"}, output
     end
     
+    def assert_line_up(lines, pattern, required = false)
+      return assert(true) if lines.empty?
+      matches = lines.map { |line| line.match(pattern) }
+      assert matches.all? if required
+      matches.compact!
+      return assert(true) if matches.empty?
+      assert_equal 1, matches.map{ |match| match.offset(0).first }.uniq.length
+    end
+    
+    def test_arguments_line_up
+      output  = standard_dump
+      output.scan(/^( *)create_table.*?\n(.*?)^\1end/m).map{ |m| m.last.split(/\n/) }.each do |column_set|
+        assert_line_up(column_set, /:(?:integer|float|datetime|timestamp|time|date|text|binary|string|boolean)/, true)
+        assert_line_up(column_set, /:default => /)
+        assert_line_up(column_set, /:limit => /)
+        assert_line_up(column_set, /:null => /)
+      end
+    end
+    
+    def test_no_dump_errors
+      output = standard_dump
+      assert_no_match %r{\# Could not dump table}, output
+    end
+    
     def test_schema_dump_includes_not_null_columns
       stream = StringIO.new