This project is archived and is in readonly mode.

#4725 ✓resolved
Caleb Land

[PATCH] Allow ActiveModel attribute names with spaces and other characters

Reported by Caleb Land | May 29th, 2010 @ 06:52 PM | in 3.1

I'm working on a library that uses ActiveModel and CouchDB and it would be nice for the AttributeMethods plugin in ActiveModel to work with attributes that have spaces (and other characters) in their names.

Ruby supports method names that don't fit the usual naming conventions if you use the define_method method, so by just changing some 'def's to 'define_method' calls, and adding some quotes around symbols in dynamically generated code, you can use attribute names with whatever characters you want.

Semantically, this patch doesn't change anything, but it makes working with more interesting field names a lot easier.

Comments and changes to this ticket

  • Caleb Land

    Caleb Land May 29th, 2010 @ 08:06 PM

    • Tag set to patch
  • Caleb Land

    Caleb Land May 31st, 2010 @ 07:29 PM

    • Title changed from “Allow attribute names with spaces and other characters (PATCH)” to “[PATCH] Allow attribute names with spaces and other characters”
  • Caleb Land

    Caleb Land May 31st, 2010 @ 08:18 PM

    • 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

    Jason Gignac May 31st, 2010 @ 09:42 PM

    -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

    Caleb Land May 31st, 2010 @ 10:38 PM

    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

    Simon Hafner June 1st, 2010 @ 10:58 AM

    Why don't remove the String eval part as you're using define_method instead of def?

  • Caleb Land

    Caleb Land June 1st, 2010 @ 07:12 PM

    That's a good idea, I'll update the patch without the eval.

  • Caleb Land

    Caleb Land June 2nd, 2010 @ 12:50 AM

    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

    Santiago Pastorino February 2nd, 2011 @ 04:51 PM

    • 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

    Santiago Pastorino February 2nd, 2011 @ 04:51 PM

    • State changed from “open” to “stale”
  • Caleb Land

    Caleb Land February 2nd, 2011 @ 07:46 PM

    • 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

    Santiago Pastorino February 2nd, 2011 @ 08:39 PM

    • 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

    Caleb Land February 3rd, 2011 @ 07:00 PM

    Here is the patch rebased to master.

    I ran AR tests against sqlite3, postgresql, and mysql, and everything looks good.

  • Repository

    Repository February 3rd, 2011 @ 09:15 PM

    • 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...

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

Tags

Referenced by

Pages