Fix: String#underscore with non-letter char #4278 #4281
Conversation
The String#underscore method does not behave correctly when handling strings containing non-letter characters such as digits (i.e. "FOO123")
|
Yes, sorry I didn't see you PR before pushing, I've put a message in the issue's page ... |
| @@ -3314,7 +3314,7 @@ class String | |||
| if mem | |||
| if char == '_' | |||
| # case 3 | |||
| else | |||
| elsif upcase || downcase | |||
There was a problem hiding this comment.
You intended last_is_upcase instead of upcase correctly, didn't you?
This is not the perfect though. For example, "3.14IsPi" should be converted to "3.14_is_pi". But your implementation converts `"3.14is_pi".
There was a problem hiding this comment.
Ah sorry. It is my mistake. I was misled ||.
There was a problem hiding this comment.
I'm not really sure of how "123ABC" should be converted (if "ABC123" has to be converted as "abc123", why make a difference for "123ABC" ?).
The best to do for now is probably to wait for an answer of the core devs ?
There was a problem hiding this comment.
The best to do for now is probably to wait for an answer of the core devs ?
Yes.
I imitated rails (activesupport) behavior:
$ ruby -rrails -e 'p "123ABC".underscore'
"123_abc"There was a problem hiding this comment.
Yup, I saw that Rails behaves this way too ...
Anyway, I'm not sure it's consistent in Rails either, the doc says As a rule of thumb you can think of underscore as the inverse of camelize so in that case, the camelized version of "123abc" should be "123Abc" but it is "123abc".
That's a lot of discussions for such a small problem :)
I prefer the way it's done in Rails but it feels strange in some ways too ...
(Sorry again for the double PR, I'll close this one as soon as we have an answer from the devs)
|
@olbat I can say 'yes'
|
|
Closing in favor of #4278 |
The String#underscore method does not behave correctly when handling
strings containing non-letter characters such as digits (i.e. "FOO123")