curl-init.xml: replace 'resource' to 'session' #3623
Conversation
I think that in the context of the curl_init() function, it is appropriate to replace the term "resource" with the term "session", since the resource is no longer relevant since PHP 8.0.0, and the session is relevant for both newer versions of PHP and before PHP 8.0.0
| // close cURL resource, and free up system resources | ||
| // Close cURL session, and free up system resources | ||
| curl_close($ch); |
There was a problem hiding this comment.
The new comment doesn't seem to be right. The old comment would at least be correct prior to PHP 8.0.0. Not sure how to improve the comment; see also the related #3622 (comment).
There was a problem hiding this comment.
I seem to be confused about the times before PHP 8.0.0 and after)
I don't understand why we can't change the current comment:
close cURL resource, and free up system resources
with a new one:
Close cURL session, and free up system resources
for the call curl_close($ch).
I'll leave it here so I don't forget what the fuctions are doing:
- curl_init — Initialize a cURL session
- curl_close — Close a cURL session
— Prior to PHP 8.0.0, the part "close the cURL resource" was relevant, but since version 8.0.0 it has not been, since resource was replaced by CurlHandle class.
— The phrase "close the cURL session" is relevant for both version.
===
I think it's better to move on to the second part of the sentence after we've dealt with the first one. Because the wording
and free up system resources
worked only prior to PHP 8.0.0 and lost its relevance as of PHP 8.0.0.
And my main question in #3622 was how to replace the outdated curl_close call as of PHP 8.0.0?
I remember about the refcounter and the garbage collector, which destroys symbols that have no references left. That's not the main question. The main question is that once the curl_close call was specified in the examples, it was probably necessary for some reason. Calling curl_close no longer has an effect, but remains in the example. The documentation mentions the lack of effect as of PHP 8.0, but does not say what to do for those who, as of PHP 8.0.0, for some reason, still need to free up system resources occupied by easy handles (e.g. in multi handle contained 5000 easy handles) manually (e.g. after reading of easy handle data).
But help me first understand the first part of the sentence: why we can't replace the word resource with the word session?
There was a problem hiding this comment.
You are right that this change would already be an improvement, and as such might be good as is. But maybe we can even do better:
// Close cURL session, and free up system resources (prior to PHP 8.0.0)
There was a problem hiding this comment.
You are right that this change would already be an improvement, and as such might be good as is. But maybe we can even do better:
// Close cURL session, and free up system resources (prior to PHP 8.0.0)
That would still be misleading. If I understand correctly, this function does nothing as of PHP 8.0. It doesn't close a session and it doesn't free up resources. Prior to PHP 8.0, it closed the resource.
I propose a simpler solution, just remove curl_close and its comment from this example. It's useless anyway.
There was a problem hiding this comment.
Yeah, maybe we should just remove it.
There was a problem hiding this comment.
I have removed every mention of curl_close from the page in f33d219 commit. At the same time, I removed the trailing whitespace characters and added the echo construction to curl_exec($ch) so that the output would actually be passed to the browser
|
Doesn't it still use a resource under the hood? I think cUrl resource still makes sense. |
|
I wish I could agree with you, but I can't. cUrl resource still makes sense... for what? I think, it make sense only prior to PHP 8.0.0. In the meantime, I'm trying to realize a) why the |
Co-authored-by: Kamil Tekiela <[email protected]>
I have removed every mention of `curl_close` from the page. At the same time, I removed the trailing whitespace characters and added the `echo` construction to `curl_exec($ch)` so that the output would actually be passed to the browser
Co-authored-by: Kamil Tekiela <[email protected]>
| // grab URL and pass it to the browser | ||
| curl_exec($ch); | ||
| // Grab URL and pass it to the browser | ||
| echo curl_exec($ch); |
There was a problem hiding this comment.
Correct me if I'm wrong, but I thought that curl_exec() already does echo internally. You'd need to explicitly disable it with CURLOPT_RETURNTRANSFER
There was a problem hiding this comment.
You are right, curl_exec display result without echo, if CURLOPT_RETURNTRANSFER not provided; I was just playing with cURL and used this option))
I looked at the curl_exec page and found no mention of the function having an internal output. I think we should go back to curl_exec a little later and add one sentence about it.
Yes, I almost forgot. Maybe it makes sense to add a mention of easy to the wording for a simple descriptor? For example, like this:
Initializes a new session and returns a cURL easy handle.
There was a problem hiding this comment.
No, just leave it as I suggested and remove the rest of the sentence.
Co-authored-by: Gina Peter Banyard <[email protected]>
I think that in the context of the curl_init() function, it is appropriate to replace the term "resource" with the term "session", since the resource is no longer relevant since PHP 8.0.0, and the session is relevant for both newer versions of PHP and before PHP 8.0.0