Does the response implement #to_str? This way, the response can be converted to a String very easily.

According to Ruby's documentation, #to_str allows the receiver to be treated as a String where it makes sense.

Bye!
François

I see that as of 1.4, the response is no longer a string.  As near as
I can tell, this breaks almost every existing use of RestClient.  e.g.
the very common codepath like this:

hash = JSON.parse(RestClient.get(url))

Sorry if I haven't been paying attention on the list, but what
motivated this change?  Returning a string that can be immediately
parsed is what I think of as the most fundamental feature of
RestClient.

Adam

I see that as of 1.4, the response is no longer a string.  As near as
I can tell, this breaks almost every existing use of RestClient.  e.g.
the very common codepath like this:

hash = JSON.parse(RestClient.get(url))

Sorry if I haven't been paying attention on the list, but what
motivated this change?  Returning a string that can be immediately
parsed is what I think of as the most fundamental feature of
RestClient.

Adam

The motivation was a proposition from you that started in a private
exchange that I then transmaitted on the mailing list (mail 29.01.2010
21:30) :

###

RestClient::Reponse is a little wonky right now - there's this weird
mixin thing, in general it needs some cleanup and normalization.

could you give me some hints of what parts need cleanup or
normalization ?

The thing that seems weird to me is that it's a mixin.  You've got
RawResponse and Response, which both include
RestClient::Mixin::Response.  It seems like this should be unified
somehow.  If nothing else, having "Mixin" in a class name (and a weird
subdirectory named mixin with exactly one file) seems very odd to me.
All of this code was contributed by others so I'm not exactly sure
what the thinking was here - I suspect they were trying to preserve
existing behavior, but the result is wonky code.

Also, the fact that Response is a subclass of String isn't great
practice.  Again, this reflects this history - the response was a
string for the first many verisons of restclient.  But perhaps we can
start to move away from that now - use duck typing to make sure it
behaves like a string, rather than having it be inherited.

If you want to discuss further let's move to the mailing list.  I
think it's really important that everyone be able to see these
discussions.

As the 1.3.0 is released this seems a good way to start the 1.4.0, so
I commited a proposed solution on the master branch.

###

Sorry if I misunderstood what you meant

The updated required some changes in RestClient usages (I provided fix to
couchrest and heroku by myself) but at the end people seemed ok so I think
your point seemed valid.



@François : Response currently implements #to_s but adding a #to_str seems
a good idea,  in the same way should I add a #to_i that returns the code ?

@Blake : it's strange as
http://github.com/archiloque/rest-client/blob/1.4.2/lib/restclient/response.rb#L25
should make the 'RestClient.get("http://foobar.heroku.com/") == "Hello
World!"' works (if reponse == foo fail the code then try response.body
with a warning if it matches)



Bien à vous

A.

I'm not against returning a String. I was against returning a class
that inherited from a string.

Strings are so flexible because they are Strings and Ruby has a
special place in it's heart for them. They are almost 100%
first-class citizens. When you inherit from a string, only to add a
few simple attributes, you remove this relationship.

I propose a simple mixin:

module RestClient
module Response
attr_accessor :code, ....

def body ; self ; end
end

def get(...)
response = make_request(...) # returns a String
response.extend Response
end
end

This keeps it a String while gaining the additional desired behavior.

That is all. I'm not using RestClient as much as I used to so take
this with a grain of salt.

BTW: A module is a mixin. No need to namespace with Mixin IMHO. ;)

-blake

On Fri, Mar 19, 2010 at 12:31 AM, <code@...> wrote:

I see that as of 1.4, the response is no longer a string. As near as
I can tell, this breaks almost every existing use of RestClient. e.g.
the very common codepath like this:

hash = JSON.parse(RestClient.get(url))

Sorry if I haven't been paying attention on the list, but what
motivated this change? Returning a string that can be immediately
parsed is what I think of as the most fundamental feature of
RestClient.

Adam

I see that as of 1.4, the response is no longer a string. As near as
I can tell, this breaks almost every existing use of RestClient. e.g.
the very common codepath like this:

hash = JSON.parse(RestClient.get(url))

Sorry if I haven't been paying attention on the list, but what
motivated this change? Returning a string that can be immediately
parsed is what I think of as the most fundamental feature of
RestClient.

Adam

The motivation was a proposition from you that started in a private
exchange that I then transmaitted on the mailing list (mail 29.01.2010
21:30) :

###

RestClient::Reponse is a little wonky right now - there's this weird
mixin thing, in general it needs some cleanup and normalization.

could you give me some hints of what parts need cleanup or
normalization ?

The thing that seems weird to me is that it's a mixin. You've got
RawResponse and Response, which both include
RestClient::Mixin::Response. It seems like this should be unified
somehow. If nothing else, having "Mixin" in a class name (and a weird
subdirectory named mixin with exactly one file) seems very odd to me.
All of this code was contributed by others so I'm not exactly sure
what the thinking was here - I suspect they were trying to preserve
existing behavior, but the result is wonky code.

Also, the fact that Response is a subclass of String isn't great
practice. Again, this reflects this history - the response was a
string for the first many verisons of restclient. But perhaps we can
start to move away from that now - use duck typing to make sure it
behaves like a string, rather than having it be inherited.

If you want to discuss further let's move to the mailing list. I
think it's really important that everyone be able to see these
discussions.

As the 1.3.0 is released this seems a good way to start the 1.4.0, so
I commited a proposed solution on the master branch.

###

Sorry if I misunderstood what you meant

The updated required some changes in RestClient usages (I provided fix to
couchrest and heroku by myself) but at the end people seemed ok so I think
your point seemed valid.



@François : Response currently implements #to_s but adding a #to_str seems
a good idea, in the same way should I add a #to_i that returns the code ?

@Blake : it's strange as
http://github.com/archiloque/rest-client/blob/1.4.2/lib/restclient/response.rb#L25
should make the 'RestClient.get("http://foobar.heroku.com/") == "Hello
World!"' works (if reponse == foo fail the code then try response.body
with a warning if it matches)



Bien à vous

A.

--
Blake Mizerany
blake.mizerany@...

Hi,

I've sorted it out with Adams on the irc, Adams:
- perhaps that's where the misunderstanding was - what I was saying is that moving away from that approach internally would be good, *if* we can keep the external interface the same
- perhaps it's just not possible, in which case I think the string inheritence is ok
- as far as I know it's never caused any real problems, it's just wonky when you read the code
- here's the case that absolutely positively must work, regardless of anything else:

###
require 'restclient'
require 'json'

hash = JSON.parse RestClient.get('http://json-test.heroku.com/', :accept => :json)

if hash['hello'] != 'world'
puts "Failed"
exit 1
end

puts "Success"
###

With the 1.4.2 code it fails
/Library/Ruby/Gems/1.8/gems/json_pure-1.2.0/lib/json/pure/parser.rb:103:in `initialize': can't convert RestClient::Response into String (TypeError)

And after following François suggestion and adding Response.to_str (and Response.to_i), it works with some warnings ("The Response is no more a String and the Response content is now accessed through Response.body, please update your code") because json call the #[] method on the Response to do the parsing.

The current behavior is:
- #to_i returns the reponse code
- #body, #to_s and #to_str return the response body
- calling a method missing on Response but available on Response.body returns the expected result but prints a warning

So the critical issue is fixed, but we need your opinion:
- I think the current behavior is a good thing because the semantic of Response.body is better and makes the code cleaner.
- Adam thinks that using Response directly is really handy, and would prefer to remove the warning

What do you think ?


Regards

A.