This project is archived and is in readonly mode.
[PATCH] Allow ActiveModel attribute names with spaces and other characters
-
Caleb Land- Tag set to patch
-
Caleb Land
- Title changed from Allow attribute names with spaces and other characters (PATCH) to [PATCH] Allow attribute names with spaces and other characters
-
Caleb Land
- Title changed from [PATCH] Allow attribute names with spaces and other characters to [PATCH] Allow ActiveModel attribute names with spaces and other characters
-
Jason Gignac
-1 Though I can see the argument for this, I think it would end up adding confusion for what is a somewhat slender use case - the number of situations in which "phone number" is inherently superior to "phone_number" seems pretty low to me? I could see this being useful for certain individuals, but it has a broad reaching enough effect, I'd think it might be better as a gem that one can install if one wishes, maybe?
-
Caleb Land
This does target a narrow use case, but currently ActiveModel just crashes when you use the name "phone number". I guess you could argue that by crashing you catch typos more easily, but like fancy, long filenames, I can see that more expressive field names could be a nice feature.
I wouldn't argue to make activerecord "space safe" but ActiveModel is a nice library for library builders, and some databases support this kind of thing.
Another way around this would be to just not declare the attributes and let method_missing take care of it...
-
Simon Hafner
Why don't remove the String eval part as you're using define_method instead of def?
-
Caleb Land
That's a good idea, I'll update the patch without the eval.
-
Caleb Land
I've added two new patches. One uses eval and one does not.
I think I prefer the string eval version, to tell you the truth. It seems easier to read.
-
Santiago Pastorino
- State changed from new to open
This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.
Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.
-
Santiago Pastorino
- State changed from open to stale
-
Caleb Land
- State changed from stale to open
This bug is still present in the master branch of Rails.
When you try to define an attribute with spaces and other non-valid Ruby identifier characters, an exception is thrown because an invalid Ruby block is eval'd.
This patch simply adds quotes to the appropriate places and changes an occurrence of "def" to define_method.
Are there any problems with this patch that I could address?
I know it's not critical, but document oriented databases allow more flexible field naming, and ruby does allow spaces and other characters as methods so long as they're defined properly.
[state:open]
-
Santiago Pastorino
- Milestone set to 3.1
- Assigned user set to Santiago Pastorino
- Importance changed from to Low
Caleb can you rebase your patch? thanks.
-
Caleb Land
Here is the patch rebased to master.
I ran AR tests against sqlite3, postgresql, and mysql, and everything looks good.
-
Repository
- State changed from open to resolved
(from [bca070ef2ddbbe7e093c340ec7722e4dca0f37a5]) allow spaces and other characters in attribute names [#4725 state:resolved]
- define the dynamically defined methods with 'define_method' instead of def
- wrap some string injected method names in quotes
Signed-off-by: Santiago Pastorino santiago@wyeworks.com
https://github.com/rails/rails/commit/bca070ef2ddbbe7e093c340ec7722...
