Pass handle to 'progress' callable #2173
Conversation
| array_shift($args); | ||
| } | ||
| call_user_func_array($progress, $args); | ||
| }; |
There was a problem hiding this comment.
If I am correct what guzzle library was doing here is to wrap one callable function inside another.
So this call_user_func_array($progress, $args); calls the developers callback function that he added from his options.
According to the docs the handler argument is not supposed to be set from the developer and that's why this part of the code exists:
if (is_resource($args[0])) {
array_shift($args);
}
If you ask why the handler does not exists. Most probably it's because guzzle provides an abstraction layer. Developers should not care about the handler that the guzzle internally uses.. So my guess is that guzzle maintainers didn't want when a developer is setting this option having to think about the handler that the client internally uses.
There was a problem hiding this comment.
Yes, but it's for compatibility purpose as it doesn't exists in old php versions. It's also documented there.
The handle is eg. accessible here
guzzle/src/Handler/EasyHandle.php
Line 17 in 0037574
There was a problem hiding this comment.
Can you leave this as unresolved? I believe that guzzle maintainers should share their toughs on this.
|
The handle however is not very handy as I found out https://stackoverflow.com/a/52930054/519333 |
|
Maybe we can reorder args and put handle as the last one. Then it won't even be BC |
|
Closing.. as this PR makes the consumers of If another maintainer feels differently we can re-discuss. |
The progress callable accepts 5 parameters where first is the handle as mentioned here http://php.net/manual/en/function.curl-setopt.php
Current state of the library passes only 4 arguments and removes the first argument containing the handle because it was not available in php versions <5.5. However this library requires 5.5+ so it can be added https://github.com/guzzle/guzzle/blob/master/composer.json#L16
I guess it's BC break.