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.

Sign up for GitHub

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

Jump to bottom

Use existing curl_opt equest methods instead of custom request #283

Merged
merged 1 commit into from Aug 6, 2012
Merged

Use existing curl_opt equest methods instead of custom request #283

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 26 additions & 1 deletion classes/kohana/request/client/curl.php
View file
Open in desktop
Expand Up @@ -24,8 +24,10 @@ public function _send_message(Request $request)
// Response headers // Response headers
$response_headers = array(); $response_headers = array();


$options = array();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these two lines is unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although they aren't necessary, I do always add them for clarity. It tells someone reading the code there's going to be an array.


// Set the request method // Set the request method
$options[CURLOPT_CUSTOMREQUEST] = $request->method(); $options = $this->_set_curl_request_method($request, $options);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CURLOPT_CUSTOMREQUEST is ok for things like "GET", "POST","PUT", "DELETE", "CONNECT" and so on

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you read the issue linked to this pull request, you'll see that it actually gave a problem to someone.



// Set the request body. This is perfectly legal in CURL even // Set the request body. This is perfectly legal in CURL even
// if using a request other than POST. PUT does support this method // if using a request other than POST. PUT does support this method
Expand Down Expand Up @@ -107,4 +109,27 @@ public function _send_message(Request $request)
return $response; return $response;
} }


/**
* Sets the appropriate curl request options. Uses the responding options
* for POST and PUT, uses CURLOPT_CUSTOMREQUEST otherwise
* @param Request $request
* @param array $options
* @return array
*/
public function _set_curl_request_method(Request $request, array $options)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it's public, shouldn't start with _

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just following the style of this file. Most methods that start with a _ are public.

{
switch ($request->method()) {
case Request::POST:
$options[CURLOPT_POST] = TRUE;
break;
case Request::PUT:
$options[CURLOPT_PUT] = TRUE;
break;
default:
$options[CURLOPT_CUSTOMREQUEST] = $request->method();
break;
}
return $options;
}

} // End Kohana_Request_Client_Curl } // End Kohana_Request_Client_Curl