1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Move comment after newline in installed migrations

When an engine's migration is installed in a rails application, a
comment is inserted after any magic comments indicating the migration's
source. However, the current implementation does not take into
account whether there is an empty line after magic comments, and the
generated migration will not pass rubocop's
Layout/EmptyLineAfterMagicComment even if the engine's migration did.
This commit changes the implementation to insert the new comment after
a newline occuring after magic comments, if it exists.

Example Engine Migration:
```ruby
# frozen_string_literal: true
# coding: ISO-8859-15

class CurrenciesHaveSymbols < ActiveRecord::Migration::Current
end
```

Before change:
```ruby
# frozen_string_literal: true
# coding: ISO-8859-15
# This migration comes from bukkits (originally 1)

class CurrenciesHaveSymbols < ActiveRecord::Migration::Current
end
```

After change:
```ruby
# frozen_string_literal: true
# coding: ISO-8859-15

# This migration comes from bukkits (originally 1)
class CurrenciesHaveSymbols < ActiveRecord::Migration::Current
end
```
This commit is contained in:
Hartley McGuire 2021-08-04 00:30:05 -04:00
parent c85d24c05d
commit 43f756778f
2 changed files with 9 additions and 3 deletions

View file

@ -957,6 +957,12 @@ module ActiveRecord
magic_comments << magic_comment; "" magic_comments << magic_comment; ""
end || break end || break
end end
if !magic_comments.empty? && source.start_with?("\n")
magic_comments << "\n"
source = source[1..-1]
end
source = "#{magic_comments}#{inserted_comment}#{source}" source = "#{magic_comments}#{inserted_comment}#{source}"
if duplicate = destination_migrations.detect { |m| m.name == migration.name } if duplicate = destination_migrations.detect { |m| m.name == migration.name }

View file

@ -1426,7 +1426,7 @@ class CopyMigrationsTest < ActiveRecord::TestCase
assert_equal [@migrations_path + "/4_people_have_hobbies.bukkits.rb", @migrations_path + "/5_people_have_descriptions.bukkits.rb"], copied.map(&:filename) assert_equal [@migrations_path + "/4_people_have_hobbies.bukkits.rb", @migrations_path + "/5_people_have_descriptions.bukkits.rb"], copied.map(&:filename)
expected = "# This migration comes from bukkits (originally 1)" expected = "# This migration comes from bukkits (originally 1)"
assert_equal expected, IO.readlines(@migrations_path + "/4_people_have_hobbies.bukkits.rb")[1].chomp assert_equal expected, IO.readlines(@migrations_path + "/4_people_have_hobbies.bukkits.rb")[2].chomp
files_count = Dir[@migrations_path + "/*.rb"].length files_count = Dir[@migrations_path + "/*.rb"].length
copied = ActiveRecord::Migration.copy(@migrations_path, bukkits: MIGRATIONS_ROOT + "/to_copy") copied = ActiveRecord::Migration.copy(@migrations_path, bukkits: MIGRATIONS_ROOT + "/to_copy")
@ -1529,8 +1529,8 @@ class CopyMigrationsTest < ActiveRecord::TestCase
assert File.exist?(@migrations_path + "/4_currencies_have_symbols.bukkits.rb") assert File.exist?(@migrations_path + "/4_currencies_have_symbols.bukkits.rb")
assert_equal [@migrations_path + "/4_currencies_have_symbols.bukkits.rb"], copied.map(&:filename) assert_equal [@migrations_path + "/4_currencies_have_symbols.bukkits.rb"], copied.map(&:filename)
expected = "# frozen_string_literal: true\n# coding: ISO-8859-15\n# This migration comes from bukkits (originally 1)" expected = "# frozen_string_literal: true\n# coding: ISO-8859-15\n\n# This migration comes from bukkits (originally 1)"
assert_equal expected, IO.readlines(@migrations_path + "/4_currencies_have_symbols.bukkits.rb")[0..2].join.chomp assert_equal expected, IO.readlines(@migrations_path + "/4_currencies_have_symbols.bukkits.rb")[0..3].join.chomp
files_count = Dir[@migrations_path + "/*.rb"].length files_count = Dir[@migrations_path + "/*.rb"].length
copied = ActiveRecord::Migration.copy(@migrations_path, bukkits: MIGRATIONS_ROOT + "/magic") copied = ActiveRecord::Migration.copy(@migrations_path, bukkits: MIGRATIONS_ROOT + "/magic")