Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Updated: Comment #0
Part of #2105863: [meta] Images, toolkits and operations.
Problem/Motivation
Because starting with #2073759: Convert toolkit operations to plugins toolkit operations will receive arguments in a keyed array rathe than directly in the method interface there's no way right now to provide sane defaults for missed optional arguments.
Proposed resolution
Declare a method in ImageToolkitOperationInterface (named defaultOperationArguments()?) and force implementations to return sane values for default. Normalize arguments in Image::apply() before sending to toolkit.
Remaining tasks
N/A
User interface changes
No changes.
API changes
New method in ImageToolkitOperationInterface named TBD.
Comments
Comment #1
fietserwinNot necessary: pass in the arguments you can, and let the toolkit operation merge in defaults if it wants to.
Moreover:
- it is hardly thinkable that when ImageEffect calls ImageToolkitOperation, that there will be arguments that could not get a value.
- ImageEffect is the interface to the user and may provide defaults where the user did not enter them. For an ImageToolkitOperation all arguments should be required. And if they are not provided: fail fast, fail loud. E.g.:do not surround array access with checks but access directly.
Comment #2
claudiu.cristeaNo, they are not necessary required. E.g. "scale" is now an operations (it was not) and may have only one dimension. Defaults were provided by the fact that arguments were directly in the interface:
scale($width = NULL, $height = NULL). That made sure that variables are at leas initialized.Please, do not close this event if you don't agree. Let the discussion go.
Comment #3
claudiu.cristeaI'm working now on #2073759: Convert toolkit operations to plugins and I just had to write
$data += array('width' => NULL, 'height' => NULL, 'upscale' => FALSE);in GDScale and then in TestScale :(Comment #4
fietserwinThe tests are testing situations that in practice will not happen. E.g. the scale effect always passes both width and height because that is already in its own data
Example:
public function scale($width = NULL, $height = NULL, $upscale = FALSE) {- This code contains a bug, not a feature. it iis unthinkable that someone would pass 0 arguments, that would make it a no-op (or scale the image to 0x0)
- You can pass in only width or height, but to express that you want only to scale to one side the caller should pass in null for the other. So not requiring the 2nd argument is also faulty.
- Only the 3rd argument can be made optional. And the operation should add that value itself.
What is wrong with your solution:
It cannot be guaranteed that someone will retrieve the sane defaults before calling the operation itself. So the operation itself should still merge the defaults in. (thus duplicating your efforts).
Looked at it in another way, merging in defaults is common practice: Any other function/method does so, if it has optional arguments. The only difference being that we can't specify it in the method signature, so we will have to document it.
Still a won't fix.
Comment #5
claudiu.cristeaThey don't have to. We'll do that for them in Image::apply() before calling the toolkit apply(). But putting that on the interface we still force them (developers) to think on that.
What I'm thinking now is that if we can merge this with "image geometry stuff" to have all in one. Not sure, let's discuss more this issue.
Comment #6
claudiu.cristeaThe scope of this issues has moved in #2073759: Convert toolkit operations to plugins.
The arguments sane defaults are provided now in
ImageToolkitOperationInterface::prepare().Closing this as duplicate of #2073759: Convert toolkit operations to plugins.