This project is archived and is in readonly mode.

#1489 ✓resolved
Henrik Nyh

Make Inflector#parameterize correctly squeeze multi-character separators

Reported by Henrik Nyh | November 28th, 2008 @ 11:52 AM | in 2.x

This has been bugging me a while, so I patched it. See attached.

Inflector#parameterize allows the custom separator to be any string, but it uses squeeze which acts on a per-character (or byte, I suppose) basis, not a string basis.

Personally I prefer what Slugalizer does: only allow a small set of one-character separators ("+", "-", "_") and raise if it's not one of those. But if separators can be any string like in current Rails, squeeze is wrong.

Comments and changes to this ticket

  • Christoffer Sawicki

    Christoffer Sawicki November 28th, 2008 @ 01:21 PM

    Henrik is right; if parameterize is supposed to handle multi-char separators, then the current implementation is buggy and Henrik's patch should be applied. In that case: +1

    However, as Henrik also says, if only one-char separators are to be supported, then that pre-condition should be checked. In this case, another fix should be created and applied.

  • Pratik

    Pratik March 8th, 2009 @ 01:22 PM

    • Assigned user set to “Pratik”
    • State changed from “new” to “incomplete”

    I get the following test failure after applying the patch.

    
  1) Error:
test_string_parameterized_no_separator(StringInflectionsTest):
RegexpError: invalid regular expression; there's no previous pattern, to which '{' would define cardinality at 1: /{2,}/
    ./test/../lib/active_support/inflector.rb:266:in `parameterize'
    ./test/../lib/active_support/core_ext/string/inflections.rb:106:in `parameterize'
    ./test/core_ext/string_ext_test.rb:88:in `test_string_parameterized_no_separator'
    ./test/core_ext/string_ext_test.rb:87:in `each'
    ./test/core_ext/string_ext_test.rb:87:in `test_string_parameterized_no_separator'
    /opt/local/lib/ruby/gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `__send__'
    /opt/local/lib/ruby/gems/1.8/gems/mocha-0.9.5/lib/mocha/test_case_adapter.rb:69:in `run'

    Thanks.

  • Henrik Nyh

    Henrik Nyh March 10th, 2009 @ 09:16 PM

    Thanks, Pratik. Fixed in second patch.

  • Henrik Nyh

    Henrik Nyh March 10th, 2009 @ 09:18 PM

    Moved comment, re-attaching second patch.

  • Repository

    Repository March 10th, 2009 @ 10:04 PM

    • State changed from “incomplete” to “resolved”

    (from [ea0e41d8fa5a132a2d2771e9785833b7663203ac]) Make Inflector#parameterize correctly squeeze multi-character separators [#1489 state:resolved]

    Signed-off-by: Pratik Naik pratiknaik@gmail.com http://github.com/rails/rails/co...

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>

People watching this ticket

Attachments

Tags

Referenced by

Pages