Use motion-support for the string inflector

[clayallsopp]

We should probably use motion-support where appropriate, starting with string inflection.

The problem is BW's original camelize method actually accepted true and false as arguments, which the Motion/ActiveSupport variant does not (these only accept :upper and :lower, which BW also supports).

And because of how BubbleWrap monkey patches the build order, if we wanted to write a version of camelize which issued a deprecation warning and converted the boolean parameter to the appropriate symbol, it require several changes to the build system. (as I'm understanding it, BubbleWrap will add its own files to the build order first and then insert any other files added to the app afterwards, so by default motion-support is getting included after our string.rb)

so, I think we can:

1. Make changes to the build system and properly deprecate the boolean version of camelize
2. Have no deprecation and just break the true/false version of camelize (again, BW already supports :lower and :upper as arguments)
3. Any other ideas?

cc @tkadauke

[ryansobol]

How much work would option 1 take?

[tkadauke]

How about using motion-require for BubbleWrap?

[clayallsopp]

@ryansobol it wouldnt take much work; I already hacked a solution together, like 20 LOC. my beef is that Bubblewrap already does a lot of unexpected things to the build system that I'd like to see removed, and I'm hesitant to add more stuff to it for this one case =\

@tkadauke I'd be all for that and plan on doing it, but I don't think it would solve this case since motion-require is a separate gem and not in the BW project? since motion-require == require_relative, kind of a pain to find the relative path to a gem right?

[ryansobol]

my beef is that Bubblewrap already does a lot of unexpected things to the build system that I'd like to see removed, and I'm hesitant to add more stuff to it for this one case =\

Makes sense. What would you like to see removed? How much work would it take?

[clayallsopp]

mmm, I think

• BW.require and all that code should be replaced by motion-require, or if HipByte will add something similar to its build system then we should use that. Doing this would break existing projects that depend on BW.require, so that's like a 2.0-ish thing to fix. There's a lot of layers to BW::Requirement, and right now it's non-trivial for someone to dig into how it works =\
• This patch (https://github.com/rubymotion/BubbleWrap/blob/master/lib/bubble-wrap/ext/motion_project_config.rb) can be removed when the next version of RubyMotion ships (was fixed by HipByte recently HipByte/RubyMotion@b05fb73)

[ryansobol]

BW.require and all that code should be replaced by motion-require, or if HipByte will add something similar to its build system then we should use that.

So how different is BW.require compared to motion-require? Also, what are the chances HipByte will add something similar?

Doing this would break existing projects that depend on BW.require, so that's like a 2.0-ish thing to fix. There's a lot of layers to BW::Requirement, and right now it's non-trivial for someone to dig into how it works =\

I agree with everything here.

This patch (https://github.com/rubymotion/BubbleWrap/blob/master/lib/bubble-wrap/ext/motion_project_config.rb) can be removed when the next version of RubyMotion ships (was fixed by HipByte recently HipByte/RubyMotion@b05fb73)

Excellent.

(Sorry if you get two emails. That'll teach me to reply by email.)

[clayallsopp]

So how different is BW.require compared to motion-require?

Mmm pretty big and basically incompatible. it's a different but more sensible and natural way of declaring RubyMotion dependencies https://github.com/clayallsopp/motion-require

Also, what are the chances HipByte will add something similar?

I'll check; lrz mentioned it on twitter when motion-require came out.

[clayallsopp]

Well, I found a smaller work-around to include motion-support: 4ad615d

Tests pass now; it still supports the old true/false style, but outputs a deprecation warning before converting the argument.

Will merge soon if no one has any objects

cc @ryansobol

[supermarin]

I'm +1 on implementing it manually, not a fan of including external deps in BW :)

[clayallsopp]

@mneorr what's the downside of an external dependency? i.e. if we wanted to use with_indifferent_access (which we probably should in some places, like Persistence), should we copy/paste that whole file and stick it in BW?

[supermarin]

@clayallsopp the only downside is dependenciesception

[ryansobol]

Well, I found a smaller work-around to include motion-support: 4ad615d

Tests pass now; it still supports the old true/false style, but outputs a deprecation warning before converting the argument.

Well done!

[ryansobol]

I'm +1 on implementing it manually, not a fan of including external deps in BW :)

@mneorr what's the downside of an external dependency? i.e. if we wanted to use with_indifferent_access (which we probably should in some places, like Persistence), should we copy/paste that whole file and stick it in BW?

I'm pumped to start using motion-support (and motion-require). But now that you mention it, I have a few concerns and comments about adding external dependencies to BubbleWrap.

Each external dependency should be safe to depend on.

BubbleWrap is a post 1.0 library. As a contributor and an end-user, this implies a sense of stability and maturity. I believe the responsible thing to do is to apply the same measuring stick to external dependencies.

How mature and stable are motion-support and motion-require? If not very, why and what can we do to help? I haven't had a chance to look them over myself but I trust your judgement.

BubbleWrap should use the majority of APIs loaded from each external dependency.

I see that this patch requires motion-support/inflector. I like this piece-meal approach you've taken.

Are there any other APIs loaded into memory that we can't see?

BubbleWrap should be compatible with libraries it doesn't depend on.

BubbleWrap is probably the most popular RubyMotion library to date. Partly because it's a one-stop-shop for essential, productivity-boosting APIs. Inevitably, other libraries will rise and implement certain APIs better than BubbleWrap. I look at passing the torch to these libraries, like motion-support, as a good thing. (Passing the torch when they're ready.)

BubbleWrap pollutes a handful of core classes (e.g. String). It's in the best interest of the community if we start phasing this out, whether or not motion-support becomes an dependency for BubbleWrap. That's another reason why I'm excited about this patch.

@clayallsopp the only downside is dependenciesception

In my experience, using Bundler makes this a non-issue. Maybe I'm missing something?

[clayallsopp]

All good points.

motion-support is pretty stable and API-safe, since it's just ActiveSupport. I guess @tkadauke can comment on that better than I.

motion-support/inflector adds these files to the build step:

   Compile gems/motion-support-0.2.0/motion/inflector/inflections.rb
   Compile gems/motion-support-0.2.0/motion/inflections.rb
   Compile gems/motion-support-0.2.0/motion/core_ext/string/inflections.rb
   Compile gems/motion-require-0.0.6/motion/ext.rb
   Compile gems/motion-support-0.2.0/motion/core_ext/array/prepend_and_append.rb
   Compile gems/motion-support-0.2.0/motion/inflector/methods.rb

ext and prepend_and_app are very small files (< 5 lines) and don't add significant time to the build process.

Though perhaps we should hold on integrating this until HipByte/RubyMotion#81 is resolved; basically RubyMotion's build system needs more fine-grained options for turning off detect_dependencies in gems. The lack of that option is already causing problems for gems like BW (see #217 and #214) and will probably increase for those using motion-require, which motion-support does.

[ryansobol]

As a side note, I'd like to point out all the internal uses of String inflection within BubbleWrap. It's much fewer than I expected.

String#camelize

https://github.com/rubymotion/BubbleWrap/blob/master/motion/core/string.rb#L50
https://github.com/rubymotion/BubbleWrap/blob/master/motion/util/constants.rb#L34

String#underscore

https://github.com/rubymotion/BubbleWrap/blob/master/motion/core/device/camera.rb#L144
https://github.com/rubymotion/BubbleWrap/blob/master/motion/ui/gestures.rb#L35
https://github.com/rubymotion/BubbleWrap/blob/master/motion/ui/gestures.rb#L36

[tkadauke]

motion-support is indeed pretty stable. The version number (0.2) is because not everything is ported over, not because of concerns over stability (there are so many tests, it's ridiculous). Also, there are many open questions about the usefulness of certain parts that can only be answered as people start using the library. Since the last point means that useless parts of the library might be removed at any time, a major version of 0 seems appropriate to me. Of course I won't remove stuff that people actually use, so that's nothing to worry about.

The fact that camelize is only used at a few places in BubbleWrap doesn't mean that a better implementation won't help. People use camelize outside of BubbleWrap and could make use of the more powerful inflector behind it. Just as an example, motion-support's camelize can correctly camelize xml_http_api as XMLHTTPAPI and the underscore method does the same in reverse (provided you define the acronyms XML, HTTP and API). That acronym support also allows to camelize Cocoa-style class names like ui_view or ns_string.

[ryansobol]

Requiring motion-support/inflector will load the following methods into memory.

String#pluralize
String#singularize
String#constantize
String#safe_constantize
String#camelize and an alias #camelcase
String#titleize and an alias #titlecase
String#underscore
String#dasherize
String#demodulize
String#deconstantize
String#parameterize
String#tableize
String#classify
String#humanize
String#foreign_key
Array#append
Array#prepend

Plus some methods in MotionSupport::Inflector that allow users to create additional inflection rules and encapsulate the inflection logic.

From the motion-support docs:

• String#parameterize is missing because it needs to transliterate the string, which is dependent on the locale
• String#constantize is very slow. Cache the result if you use it.

---

I can live with these additions. I wish String#parameterize was at least a no-op, if not commented out, until MotionSupport::Inflector.parameterize is implemented. And I recommend avoiding String#constantize for long, nested constant strings. (e.g. "AReally::Really::Really::Long::Constant::Name")

[tkadauke]

@ryansobol oops, that must've slipped through. Of course String#parameterize should be removed.

Constantize is slow because Object#const_get is slow. My theory is that this is because there are just so many constants defined in RubyMotion (all the Cocoa classes plus all the enum values). So constantize should be cached whenever possible.

[ryansobol]

Constantize is slow because Object#const_get is slow. My theory is that this is because there are just so many constants defined in RubyMotion (all the Cocoa classes plus all the enum values). So constantize should be cached whenever possible.

Thanks for clarifying. I assumed that String#constantize was slow solely because of the loop. Good to know about #const_get.

[ryansobol]

Well, String#parameterize has been removed from motion-support. Anything else blocking this?