ActiveSupport::Inflector.underscore messes up acronyms under namespaces #12510

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants

agios commented Oct 13, 2013

For example:

ActiveSupport::Inflector.inflections do |inflect|
  inflect.acronym 'ISP'
  inflect.acronym 'ISPs'
end

"ISP".pluralize.underscore 
  => "isps" (ok)
"SomeISP".pluralize.underscore 
  => "some_isps" (ok)
"Provider::ISP".pluralize.underscore
  => "provider/is_ps" (should be provider/isps)

agios commented Oct 26, 2013

Has anyone had a chance to look at this yet? It is a simple and straight-forward fix.

Currently this issue breaks url_for in our application if we use namespaced model names with an acronym, among other problems. Ie, if the model was named Provider::ISP, the helper method would be incorrectly named provider_is_ps_url and url_for(Provider::ISP) fails with undefined methodprovider_isps_url'`

Contributor

pftg commented Oct 26, 2013

Please add tests and changelog entry

agios commented Oct 26, 2013

The patch already includes the test. I'll add a changelog entry.

Contributor

pftg commented Oct 26, 2013

👍

Contributor

pftg commented Oct 26, 2013

Also please squash commits

agios commented Oct 28, 2013

@pftg does this look ok now?

Owner

rafaelfranca commented Nov 7, 2013

Cc @fxn

@ghost ghost assigned frodsan and fxn Nov 7, 2013

agios commented Dec 6, 2013

Is anyone still looking at this? Given it's a bug, albeit small, it would be nice if it could be fixed (and possibly backported too, allowing us to remove a monkey-patch 😉)

Contributor

jaggederest commented Dec 11, 2013

This is failing CI, can you update it and get it to pass?

agios commented Dec 11, 2013

The CI fail was on the master branch, if it is fixed I can rebase it.

agios commented Oct 3, 2014

I believe the regex in #14146 is slightly more elegant, closing in favor of that

@agios agios closed this Oct 3, 2014

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