rubymotion-community / BubbleWrap

Cocoa wrappers and helpers for RubyMotion (Ruby for iOS and OS X) - Making Cocoa APIs more Ruby like, one API at a time. Fork away and send your pull requests
Other
1.18k stars 208 forks source link

[query.rb] turn to string before testing for existence in #escape. added... #262

Closed bogardon closed 11 years ago

bogardon commented 11 years ago

... simple test.

fixes #261

clayallsopp commented 11 years ago

Hmm, a different way to fix this is to add another case in process_payload_hash that handles boolean values?

I think I'd rather be explicit about how different values are transformed than a catch-all to_s. Then code like this would still raise errors if you did this on accident:

my_obj = MyClass.new

BW::HTTP.get("google.com", {payload: {my_obj: my_obj}}) do |response|
end
# => Raises error, as my_obj is not a string

Is that reasonable?

bogardon commented 11 years ago

Would that actually raise an error or just try to append something like "#<Model:0xb4aafd0>"? Honestly I would rather avoid setting standards in this case unless there's an existing standard to go on. For example, if I pass false, should it be "false" or 0? If I pass a NSDate, is it some ISO string or unix time? I guess what it really comes down to is to never pass nil or a non String object inside of payload?

Either way, this PR is really saying, let's make sure we're at least passing a String to CFURLCreateStringByAddingPercentEscapes.

clayallsopp commented 11 years ago

Gah terribly sorry! I read the original code incorrectly (I didn't see the original implementation called string.to_s already)

Thanks, good call on the patch!