Right now, if you accidentally give a migration filename an upper-case
character (say, 001-initialMigration.rb), you get a crash:
You have a nil object when you didn''t expect it!
You might have expected an instance of Array.
The error occurred while evaluating nil.first
/home/bronson/workspace-hobo/hobo-docs/vendor/rails/activerecord/lib/active_record/migration.rb:367:in
`migration_files''
I would like to fix this.
Isn''t this an artificial limitation? Why not just add "A-Z"
to the regex in
migration.rb so that migration names can contain capitals?
def migration_version_and_name(migration_file)
- return *migration_file.scan(/([0-9]+)_([_a-z0-9]*).rb/).first
+ return *migration_file.scan(/([0-9]+)_([_A-Za-z0-9]*).rb/).first
end
Either way, should I submit a patch that does something like this?
- migration_version_and_name(f).first.to_i
+ m = migration_version_and_name(f)
+ raise IllegalMigrationNameError.new(f) if m.nil?
+ m.first.to_i
(and, of course, create the IllegalMigrationNameError object)
That way, at least, Rails will spit out an intelligible error message
instead of bombing out when it calls nil.first.
Does this make sense?
- Scott
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core-unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---
On 10/17/07, Scott Bronson <bronson@rinspin.com> wrote:> Right now, if you accidentally give a migration filename an upper-case > character (say, 001-initialMigration.rb), you get a crash: > > You have a nil object when you didn''t expect it! > You might have expected an instance of Array. > The error occurred while evaluating nil.first > > /home/bronson/workspace-hobo/hobo-docs/vendor/rails/activerecord/lib/active_record/migration.rb:367:in > `migration_files'' > > I would like to fix this. > > > Isn''t this an artificial limitation? Why not just add "A-Z" to the regex in > migration.rb so that migration names can contain capitals?> def migration_version_and_name(migration_file) > - return > *migration_file.scan(/([0-9]+)_([_a-z0-9]*).rb/).first > + return > *migration_file.scan(/([0-9]+)_([_A-Za-z0-9]*).rb/).first > end >I wrote about this some time ago: http://talklikeaduck.denhaven2.com/articles/2007/09/11/conventions-uber-alles prompted by this, Ben Scofield proposed a similar patch: http://dev.rubyonrails.org/ticket/9542 Which was rejected because it would violate roundtrips from classname to filename.> Either way, should I submit a patch that does something like this? > > - migration_version_and_name(f).first.to_i > + m = migration_version_and_name(f) > + raise IllegalMigrationNameError.new(f) if m.nil? > + m.first.to_i > > (and, of course, create the IllegalMigrationNameError object) > > That way, at least, Rails will spit out an intelligible error message > instead of bombing out when it calls nil.first.Well why not submit it and see what the maintainers have to say. -- Rick DeNatale My blog on Ruby http://talklikeaduck.denhaven2.com/ --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
Jack Danger Canty
2007-Oct-17 17:37 UTC
Re: Baffling error when migration includes upper-case
On 10/16/07, Scott Bronson <bronson@rinspin.com> wrote:> > Right now, if you accidentally give a migration filename an upper-case > character (say, 001-initialMigration.rb), you get a crash:Was this file made from the migration generator or by hand? It seems like we should fix this if it was somehow generated improperly but hand-written migrations should probably need to follow convention.> Either way, should I submit a patch that does something like this? > > - migration_version_and_name(f).first.to_i > + m = migration_version_and_name(f) > + raise IllegalMigrationNameError.new(f) if m.nil? > + m.first.to_i > > (and, of course, create the IllegalMigrationNameError object) > > That way, at least, Rails will spit out an intelligible error message > instead of bombing out when it calls nil.first.That''s a great idea. Well-named errors (especially on the command line) are a real help to new Railers (and old ones too). ::Jack Danger --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---
> > On 10/17/07, Rick DeNatale <rick.denatale@gmail.com> wrote: > prompted by this, Ben Scofield proposed a similar patch: > http://dev.rubyonrails.org/ticket/9542 > Which was rejected because it would violate roundtrips from classname > to filename.That makes total sense. Given that, I agree that migrations filenames should be restricted to lower case. On 10/17/07, Jack Danger Canty <dangeronrails@gmail.com> wrote:> Was this file made from the migration generator or by hand? It seems like > we should fix this if it was somehow generated improperly but hand-written > migrations should probably need to follow convention. >It was made using the Hobo migration generator which apparently doesn''t sanitize the filenames. I''ll open a Hobo ticket. Either way, should I submit a patch...?> > > That''s a great idea. Well-named errors (especially on the command line) > are a real help to new Railers (and old ones too). >Ticket: http://dev.rubyonrails.org/ticket/9909 Please vote for it! :) - Scott --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@googlegroups.com For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en -~----------~----~----~----~------~----~------~--~---