String extensions should be on NSString about motion-support

[tkadauke]

I agree. I didn't do it yet, since I didn't need it so far.
There are basically two different ways to do that. One, like you said, add everything to NSString/NSMutableString. Two, leave things in String, and add a to_s method to NSString, giving the responsibility back to the user. Two point five would be to also add all the methods to NSString/NSMutableString, but delegate the calls to to_s. An example of the last one can be found in motion/core_ext/ns_dictionary.rb

What do you think?

[clayallsopp]

Hmmm, I don't like just using to_s because it can lead to a lot of unexpected behavior (i.e. you create a User object with String fields, serialize it, deserialize it, everything is now an NSString and you have to remember to coerce the fields back with to_s at some point). Would lead to a lot of surprises just because of a technical detail in how RubyMotion is implemented, and folks without a Cocoa background won't immediately understand why. I can already see the Github Issues popping up :)

But the delegation scheme could work...I guess the drawback is when new extensions are added, you have to remember to also add a corresponding method to the right core_ext file? Instead of just writing it in one place and being done with it. And that's not necessarily a bad thing, since new extensions won't be super common; just trying to make sure that's the only downside.

So yeah tl;dr either solution 1 or solution 2.5 sound good

[tkadauke]

Check out my commit in branch ns_string. This has the additional advantage that whenever you use an extension to class String that returns a different string, you're guaranteed to end up with a Ruby String instance instead of a NSString instance. What do you think?

[clayallsopp]

Very nice 👍

[tkadauke]

Pushed the changes to master

[clayallsopp]

Hmmm, I think there's a bug in the implementation for multiple-argument methods:

(main)> "camel_case".camelize(true)
=> nil
(main)> "camel_case".camelize(false)
=> nil

# vs.
(main)> MotionSupport::Inflector.camelize("camel_case", true)
=> "CamelCase"
(main)> MotionSupport::Inflector.camelize("camel_case", false)
=> "camelCase"

[tkadauke]

Hmm I agree it is unexpected, but it's just like in ActiveRecord. It should be

"camel_case".camelize(:lower)
"camel_case".camelize(:upper)

[clayallsopp]

Ahhh gotcha, makes sense.