Support for underscores in module namespaces #6105

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants

bploetz commented May 1, 2012

As noted in #5849, Rails does not currently support module namespaces containing underscores. This is largely due to how ActiveSupport::Inflector.camelize and ActiveSupport::Inflector.underscore currently work.

This pull request essentially introduces a new convention whereby module namespaces containing underscores are translated to paths with two underscores for each underscore, and vice versa. For example:

"API::V0_1_0" --> "api/v0__1__0"
"api/v0__1__0" --> "API::V0_1_0"

I'm not married to this convention by any stretch, I'm just throwing it out there as a straw man.

In addition to this fix for the 3-2-stable branch, I've also made this change for the 3-0-stable, 3-1-stable, and master branches as well. You can find them in the following branches in my repo (if you want a separate pull request for each just let me know):

underscores-in-namespaces-3-0-stable
underscores-in-namespaces-3-1-stable
underscores-in-namespaces-3-2-stable
underscores-in-namespaces-master

I can also fix this for the 2.x series branches as well if you want, just let me know.

Thanks,
Brian

Add-on: AS tests in master are running green.

fxn was assigned Jul 9, 2012

Owner

fxn commented Jul 9, 2012

I'd like to explore other conventions in the file name. I think an option that looks more natural to me would be a hyphen:

"Foo::Bar_Baz" <=> "foo/bar-baz"

Nowadays "bar-baz" is camelized as "Bar-baz", therefore nobody is using that as a file or directory name and would be backwards compatible given the fact that camelize and underscore are meant to solve this problem. (Of course any solution would be backwards incompatible if we take them as mere functions on strings, but they are not.)

What do you think?

bploetz commented Jul 9, 2012

"-" in stead of "__" in the file/directory names is fine with me. Do you want me to update the pull request using this dash convention, or do you want to gather more feedback first? Let me know.....

Owner

fxn commented Jul 9, 2012

Hi Brian. On a second thought I think this approach is not a good idea because since underscore respects underscores this change would be backwards incompatible.

That is, today one can autoload A_B::C_D, and have the class defined in a_b/c_d.rb.

On the other hand, the test suite has some test cases for camelized strings with underscores that people may be relying on.

All in all, this may be an edge case that perhaps does not justify breaking existing code. In some cases require_dependency may be a workaround.

bploetz commented Jul 9, 2012

Hey Xavier,

One could argue that the change introduced between Rails 3.1 and 3.2 (d38ca78 as outlined in #5849) broke backwards compatibility as well, but I understand your concerns.

Again, having underscores in module names is perfectly valid Ruby, but obviously I defer to you guys whether you want to support it in Rails or not.

Owner

fxn commented Jul 9, 2012

Hmmm... regarding acronyms see the comments in its pull request about backwards compatibility #1366.

Thing is modules with underscores work today:

$ cat app/models/c_d/e_f.rb
module C_D
  module E_F
  end
end
$ rr 'p C_D::E_F'
C_D::E_F

and that would be broken with this change. See my concern?

bploetz commented Jul 10, 2012

Sorry, maybe I'm missing something here. If modules with underscores work today, then why are the two following problems happening?

#5849
#5849 (comment)

Owner

fxn commented Jul 10, 2012

Sure, the original ticket shows modules with underscores do not work under some circumstances. At first sight seem to be those where Rails starts with a string to come up with a constant name via camelize.

So, the situation is that modules with underscores work today in some use cases, and do not work in some others.

But the proposed solution would break existing Rails applications where constant names with underscores are used in a way that works. I believe we need to think of another solution because of that.

bploetz commented Jul 10, 2012

OK. How do you propose that we proceed then? Do you have an approach in mind?

Owner

fxn commented Jul 10, 2012

@bploetz not sure. One approach to explore is to revise camelize to support some new convention (eg hyphen -> underscore), and leave underscore as it is. They wouldn't round-trip but they already don't, that seems fine to me.

Not supper happy with ad-hoc conventions like that for camelize though. Rails assumes standard naming conventions. While the underscore is a valid character in constant names, class and module names use camel case by convention, no underscores. Not very keen on introducing weird rules for camelize to support non-conventional constant names.

To be honest I don't have a solution that I like right now, but let's discuss and see whether we can do something. /cc @jeremy @josevalim

bploetz commented Jul 12, 2012

@fxn: There's currently a hook to allow you to add custom inflections, i.e.

ActiveSupport::Inflector.inflections do |inflect|
  inflect.uncountable "rails"
end

Perhaps there's a way we can do something similar to allow folks to add custom camelize and underscore rules to their app?

Member

schneems commented Aug 31, 2012

What are the next steps for this PR?

Member

steveklabnik commented Aug 31, 2012

I am personally 👎 on using modules with _ in general, so I'm not sure this is behavior I'd like to encourage.

Member

steveklabnik commented Nov 17, 2012

It's been four months with no new discussion, and so I'm giving this a close. Thanks for the patch, @bploetz , but it doesn't seem that we've come to any kind of consensus on how this feature should work. If you'd like to continue the conversation, please post to rails-core. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment