Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 156 - adding a default user agent. #176

Merged
merged 2 commits into from
Jun 12, 2012
Merged

Issue 156 - adding a default user agent. #176

merged 2 commits into from
Jun 12, 2012

Conversation

9z0b3t1c
Copy link
Contributor

@9z0b3t1c 9z0b3t1c commented Jun 8, 2012

No description provided.

@hanshasselberg
Copy link
Member

Thanks for your work! Is it possible to add the user agent without modifying the original options?

@9z0b3t1c
Copy link
Contributor Author

9z0b3t1c commented Jun 9, 2012

I don't see how I have modified the original options...? :)

@hanshasselberg
Copy link
Member

When there are headers already, the hash is assigned to @headers here: https://github.com/typhoeus/typhoeus/blob/master/lib/typhoeus/request.rb#L80. And the you modify the very same hash. Didn't tested it, but pretty sure.

@9z0b3t1c
Copy link
Contributor Author

I'm setting the User-Agent in @headers unless it is already set. I don't modify the original options. What else could we do here? :)

@hanshasselberg
Copy link
Member

Sorry for not being clear. This is what I mean:

def fubar(options)
  headers = options[:headers]
  headers['User-Agent'] = 'typhoeus'
end
options = {:headers => {}}
fubar(options)
p options
#=> {:headers=>{"User-Agent"=>"typhoeus"}}

And I wanted you to duplicate the headers before changing it. Or something similar.

@9z0b3t1c
Copy link
Contributor Author

Hi Hans,

Thanks for your explanation, that makes sense.

I'm not sure if the focus is to ensure the options hash isn't modified, or the @headers accessor.

I think that we have to set @headers['User-Agent'] in the headers directly, rather than in the user_agent method, because it is the headers which will actually be used by Curl. I don't even think that user_agent method is used...?

I'm pretty sure this isn't really what you're looking for, but I've marshalled the options to ensure they won't be modified and added some more specs.

Let me know what you think! :)

Cheers,
Steven

@hanshasselberg
Copy link
Member

Hi Steven,

sorry for the back and forth and thanks for your patience. This is what I was looking for. Thanks!

@9z0b3t1c
Copy link
Contributor Author

Cool. :)

hanshasselberg added a commit that referenced this pull request Jun 12, 2012
Issue 156 - adding a default user agent.
@hanshasselberg hanshasselberg merged commit 03ec0bd into typhoeus:master Jun 12, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants