Permalink
Browse files

add support for closure view variables

  • Loading branch information...
WanWizard committed Dec 6, 2014
1 parent 1ee009b commit 30643ab08bc07d9f8ae75ca8a337166aec5f117b
Showing with 5 additions and 0 deletions.
  1. +5 −0 classes/view.php
View
@@ -272,6 +272,11 @@ protected function get_data($scope = 'all')
$filter = array_key_exists($key, $rules) ? $rules[$key] : null;
$filter = is_null($filter) ? $auto_filter : $filter;
+ if ($value instanceOf \Closure)
+ {
+ $value = $value();

This comment has been minimized.

Show comment
Hide comment
@jpudney

jpudney Aug 18, 2015

Contributor

What happens if the closure requires params?

@jpudney

jpudney Aug 18, 2015

Contributor

What happens if the closure requires params?

This comment has been minimized.

Show comment
Hide comment
@WanWizard

WanWizard Aug 18, 2015

Member

You're referring to old code, this issue has been addressed in #1881

@WanWizard

WanWizard Aug 18, 2015

Member

You're referring to old code, this issue has been addressed in #1881

+ }
+
$value = $filter ? \Security::clean($value, null, 'security.output_filter') : $value;
}

11 comments on commit 30643ab

@tominon

This comment has been minimized.

Show comment
Hide comment
@tominon

tominon Jan 12, 2015

Contributor

Try to add closure to Presenter's view() method:

$this->echo_upper = function($string) {
    echo strtoupper($string);
};

and you will get this exception:

Fuel\Core\PhpErrorException [ Warning ]:
Missing argument 1 for Presenter_Welcome_Hello::{closure}(), called in fuel\core\classes\view.php on line 277 and defined
Contributor

tominon replied Jan 12, 2015

Try to add closure to Presenter's view() method:

$this->echo_upper = function($string) {
    echo strtoupper($string);
};

and you will get this exception:

Fuel\Core\PhpErrorException [ Warning ]:
Missing argument 1 for Presenter_Welcome_Hello::{closure}(), called in fuel\core\classes\view.php on line 277 and defined
@WanWizard

This comment has been minimized.

Show comment
Hide comment
@WanWizard

WanWizard Jan 12, 2015

Member

What exactly do you want to achieve by this?

You can't pass closures to a view at the moment, only values. So if you define a closure, it will be converted at runtime by this commit, which obviously fails with your example, since that is not designed to return a value, but to act as a modifier.

Member

WanWizard replied Jan 12, 2015

What exactly do you want to achieve by this?

You can't pass closures to a view at the moment, only values. So if you define a closure, it will be converted at runtime by this commit, which obviously fails with your example, since that is not designed to return a value, but to act as a modifier.

@tominon

This comment has been minimized.

Show comment
Hide comment
@tominon

tominon Jan 12, 2015

Contributor

Where should be this modifier used then? If I read the docs right I can use it in View. This commit broke my code which relied on this.

Contributor

tominon replied Jan 12, 2015

Where should be this modifier used then? If I read the docs right I can use it in View. This commit broke my code which relied on this.

@emlynwest

This comment has been minimized.

Show comment
Hide comment
@emlynwest

emlynwest Jan 13, 2015

Member

I agree with @tominon with this. We should be able to pass closures to the view. In the past I have used an object that extends Closure and given it a __toString method to get the behaviour this commit implements but without the restriction.

Member

emlynwest replied Jan 13, 2015

I agree with @tominon with this. We should be able to pass closures to the view. In the past I have used an object that extends Closure and given it a __toString method to get the behaviour this commit implements but without the restriction.

@tominon

This comment has been minimized.

Show comment
Hide comment
Contributor

tominon replied Jan 15, 2015

@WanWizard

This comment has been minimized.

Show comment
Hide comment
@WanWizard

WanWizard Jan 15, 2015

Member

Trying to remember why this commit was made, and therefore what will break if this gets reverted...

Member

WanWizard replied Jan 15, 2015

Trying to remember why this commit was made, and therefore what will break if this gets reverted...

@tominon

This comment has been minimized.

Show comment
Hide comment
@tominon

tominon Jan 27, 2015

Contributor

Any news about this @WanWizard as it breaks backwards compatibility?

Contributor

tominon replied Jan 27, 2015

Any news about this @WanWizard as it breaks backwards compatibility?

@WanWizard

This comment has been minimized.

Show comment
Hide comment
@WanWizard

WanWizard Feb 2, 2015

Member

Sorry, health problems. 😭 it's on my todo list...

Member

WanWizard replied Feb 2, 2015

Sorry, health problems. 😭 it's on my todo list...

@kenjis

This comment has been minimized.

Show comment
Hide comment
@kenjis

kenjis Feb 3, 2015

Contributor

How about making a rule which all of you have to send PR to commit?
So it will be easier to remember why.

Contributor

kenjis replied Feb 3, 2015

How about making a rule which all of you have to send PR to commit?
So it will be easier to remember why.

@kenjis

This comment has been minimized.

Show comment
Hide comment
@kenjis

kenjis Jul 4, 2015

Contributor

This was fixed by #1881

Contributor

kenjis replied Jul 4, 2015

This was fixed by #1881

@tominon

This comment has been minimized.

Show comment
Hide comment
@tominon

tominon Jul 4, 2015

Contributor

👍

Contributor

tominon replied Jul 4, 2015

👍

Please sign in to comment.