TYPO3.Fluid

Hi Bastian,

I think it'd be good to have the arguments working streamlined across the standard ViewHelpers; and this includes the $arguments argument behaving the same everywhere and i.e. accepting NULL as a value.

Personally I've stumbled across this when I was implementing a partial that uses the FormViewHelper and has an optional argument to set the $arguments argument of the FormViewHelper, which may or may not be set from the outside. I expected the same behaviour as with the other mentioned ViewHelpers like Link\ActionViewHelper, but instead I got an error thrown at me.

Best regards.

PS: The amount of arguments is too damn high!

Kevin Fischer wrote:

I think it'd be good to have the arguments working streamlined across the standard ViewHelpers

I agree. But we usually try to avoid mixed types as they're really error prone. So I'd suggest to fix the other ViewHelpers instead - but this would be a breaking change.

Personally I've stumbled across this when I was implementing a partial that uses the FormViewHelper and has an optional argument to set the $arguments argument of the FormViewHelper [...]

I see, that's a valid scenario of course..
But actually I think it might be a bug of the Fluid core if it really behaves this way. For:

1<f:form arguments="{null}" />

I would expect render() to be called with the default value of the "arguments" parameter, which is "array()".

PS: The amount of arguments is too damn high!

What do you suggest?

Bastian Waidelich wrote:

I agree. But we usually try to avoid mixed types as they're really error prone. So I'd suggest to fix the other ViewHelpers instead - but this would be a breaking change.

I see your point, but wouldn't moving the arguments defintion to initializeArguments() solve this problem as well, while allowing NULL values? Not accepting NULL for hinted parameters is a terrible, terrible mistake made by PHP.
Let me know if I should submit another patch that moves the argument definitions to initializeArgument().

I would expect render() to be called with the default value of the "arguments" parameter, which is "array()".

Imho this shouldn't be the case. What if you have a default value but want to override it with NULL? This wouldn't work anymore in this case. I don't know if this happens when arguments are defined using registerArgument(). I wouldn't mind it happening, but I also wouldn't expect it. This is a whole other story tho :)

PS: The amount of arguments is too damn high!

What do you suggest?

I was just kidding because of the high occurance of "argument" in my posts :)
http://knowyourmeme.com/memes/the-rent-is-too-damn-high-jimmy-mcmillan

Kevin Fischer wrote:

Let me know if I should submit another patch that moves the argument definitions to initializeArgument().

The patch is not needed, but if you could quickly test whether this really fixes the issue - that would be great!
We already discussed moving all arguments of the Fluid ViewHelpers to the initializeArgument(). This would make subclassing easier - but it also has some drawbacks (worse readability of the code (no type inference) and it's again a breaking change...)

I would expect render() to be called with the default value of the "arguments" parameter, which is "array()".

Imho this shouldn't be the case. What if you have a default value but want to override it with NULL?

True.. But didn't you suggest the same thing with "Not accepting NULL for hinted parameters is a terrible"?

PS: The amount of arguments is too damn high!

What do you suggest?

I was just kidding because of the high occurance of "argument" in my posts :)
http://knowyourmeme.com/memes/the-rent-is-too-damn-high-jimmy-mcmillan

:D

Bastian Waidelich wrote:

The patch is not needed, but if you could quickly test whether this really fixes the issue - that would be great!

Yes, this would solve the problem. I've removed the $arguments argument from render() and added the following line to initializeArguments():

$this->registerArgument('arguments', 'array', 'Additional arguments', false, array());

We already discussed moving all arguments of the Fluid ViewHelpers to the initializeArgument(). This would make subclassing easier - but it also has some drawbacks (worse readability of the code (no type inference) and it's again a breaking change...)

On the one hand, you're totally right there. Having all arguments in an outline or similiar is handy. But I don't dislike having them listed in the initializeArguments() either. But this everyone's personal opinion anyway.

True.. But didn't you suggest the same thing with "Not accepting NULL for hinted parameters is a terrible"?

No, what I meant is that in other languages, say Java, you can define a method like public void foobar(ArrayList arguments) and then call that method with foobar(NULL), which is a totally legit way as any value can be NULL in Java and most other OOP-languages. PHP though seems to think that it's not and will throw the metioned Catchable Fatal Error when passing NULL to a type-hinted parameter like public function render(array $arguments), as it expects an array, "but null is given".

Hi all,

I'd really like to see all arguments of all fluid VHs moved to initializeArguments().
It would solve this problem, quite some other problems with calling VHs we have currently and would make extending VHs a lot easier.

At which state of discussion is this change currently?

By the way: Just removing the "array" type hint as in the OP's .diff file wouldn't break anything, would it? It should be totally backward compatible, and as all the other mentioned ViewHelpers already do it like this, I'd highly suggest applying the same to the FormViewHelper.