8000 [Form] RFC: Replace dynamic by proper type inheritance · Issue #3879 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Form] RFC: Replace dynamic by proper type inheritance #3879

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
webmozart opened this issue Apr 11, 2012 · 48 comments
Closed

[Form] RFC: Replace dynamic by proper type inheritance #3879

webmozart opened this issue Apr 11, 2012 · 48 comments

Comments

@webmozart
Copy link
Contributor

Since dynamic inheritance is inherently flawed (#3793), it should be replaced by proper PHP inheritance. This should make it much more intuitive for newcomers to extend a type. This change has been made possible by the recently added class DefaultOptions (#3789).

A BC layer should allow to use the old and new API side by side. Core types should be moved to the new API.

API before:

class MyType extends AbstractType
{
    public function getName()
    {
        return 'my_type';
    }

    public function getParent()
    {
        return 'form';
    }

    public function buildForm(FormBuilder $builder, array $options)
    {
        // ...
    }

    public function buildView(FormView $view, FormInterface $form)
    {
        // ...
    }

    public function buildViewBottomUp(FormView $view, FormInterface $form)
    {
        // ...
    }

    public function getDefaultOptions()
    {
        return ...
    }
}

API after:

class MyType extends FormType
{
    public function getName()
    {
        return 'my_type';
    }

    public function buildForm(FormBuilder $builder, array $options)
    {
        // no parent::buildForm() call

        // ...
    }

    public function buildView(FormView $view, FormInterface $form)
    {
        // no parent::buildView() call

        // ...
    }

    public function finishView(FormView $view, FormInterface $form)
    {
        // no parent::finishView() call

        // ...
    }

    public function setDefaultOptions(OptionsResolver $resolver)
    {
        // no parent::setDefaultOptions() call

        $resolver->setDefaults(array(...));
    }
}

As you can see, the type methods must not call their parent methods. This allows the factory to call each single method at the appropriate time and to interline the type extension methods in between.

Positive effects:

  • Understandable inheritance
  • Use of instanceof, e.g. $form->getType() instanceof MyType
  • Reuse of protected methods from the parent type

Negative effects:

  • If you call parent::.. in any of the interface methods, this will lead to strange side effects. I'm not sure if we can catch these situations and throw an exception.
@marijn
Copy link
marijn commented Apr 11, 2012

👍 I never really understood why it wasn't like this in the first place. Great to see this will be changed. Any chance this will be finished for a 2.1 release?

@webmozart
Copy link
Contributor Author

It was like this in the first place because the differentiation between FieldType and FormType seemed necessary. As I know now, this is not the case anymore (#3878).

@stof
Copy link
Member
stof commented Apr 11, 2012

@bschussek I see 2 potential issues here:

  • people are likely to forget to overwrite getName, which would break things as it would replace the parent type in the factory (which is the reason why we removed the guessing implementation in the AbstractType during the beta cycle to force the user to specify a name)
  • forgetting to call the parent method is likely to break things in the rendering (or in other methods)

@webmozart
Copy link
Contributor Author

@stof As for the first issue, I just updated the description. As for the second, yes. We can't change this, but people will (should) notice immediately.

@rande
Copy link
Contributor
rande commented Apr 11, 2012

I found this less intuitive ;) The effort should be on documentation and clean API.

@marijn
Copy link
marijn commented Apr 11, 2012

[...]forgetting to call the parent method[...]

You can never prevent that, but one might expect from a decent developer to be smart enough to call a parent method right?

@stof
Copy link
Member
stof commented Apr 11, 2012

@marijn From my experience on providing support for Symfony during one year, I see 2 possible answers: either decent developers are not (always) smart enough for it or symfony users are not (always) decent developers. They are able to forget the parent call even when they copy-paste code from the doc and the doc example contains it.

@marijn
Copy link
marijn commented Apr 11, 2012

@stof well you definitely are the authority on that matter 😄

@alvarezmario
Copy link
Contributor

@stof it is always possible to add a big tip on the Form documentation to clarify to such developers that they should call the parent method.
I don't think this point to be a valid reason to be against this change, which seems pretty good.

@docteurklein
Copy link
Contributor

I think using php's inheritance instead of artificial one is better, because it seems equally complicated to understand for mid-level users.
I would even say that understanding how getParent() is used is more hard to explain than simple class inheritance.
So i'm +1 for this change.

@tystr
Copy link
Contributor
tystr commented Apr 11, 2012

+1

@willdurand
Copy link
Contributor

Will it be the last change on this component?

A BC layer should allow to use the old and new API side by side. Core types should be moved to the new API.

Does it mean Symfony2 will support the old API forever?

@stof
Copy link
Member
stof commented Apr 11, 2012

@willdurand no. A BC layer would be marked a deprecated, meaning it would be removed later. See for instance the Session methods related to flash messages. They are tagged as deprecated and documented as being dropped for 2.3

@jmikola
Copy link
Contributor
jmikola commented Apr 12, 2012

@bschussek: I recently upgraded our Symfony submodule to yesterday's pointer, as we needed all of the recent Form PR's. This one obviously is not in our fork, since it has yet to be merged.

We're still having an issue with a string-based date field. We have a custom form type that uses DateType as its parent, and sets the widget option's default value to single_text. The problem is that the field ends up being created with a data mapper. I believe this is due to:

[BC BREAK] FormType::getParent() does not see default options anymore

The rest of the default options in the base DateType include my child type's default as I'd expect, but the form type itself isn't correct (well, it's technically "correct" given the noted BC break, but it's broken for me :)

Will this PR address the issue of option defaults being used to decide parent types? For now, we can get by with redundantly specifying the widget option when building the form that uses our custom field.

@webmozart
Copy link
Contributor Author

@jmikola Your problem will be addressed by #3878.

@jmikola
Copy link
Contributor
jmikola commented Apr 13, 2012

Thanks, @bschussek

fabpot added a commit that referenced this issue Apr 18, 2012
Commits
-------

6e4ed9e [Form] Fixed regression: bind(null) was not converted to an empty string anymore
fcb2227 [Form] Deprecated FieldType, which has been merged into FormType
bfa7ef2 [Form] Removed obsolete exceptions
2a49449 [Form] Simplified CSRF mechanism and removed "csrf" type

Discussion
----------

[Form] Merged FieldType into FormType

Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #3878
Todo: update the documentation on theming

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3878)

This PR is a preparatory PR for #3879. See also #3878.

---------------------------------------------------------------------------

by juliendidier at 2012-04-13T14:25:19Z

What's the benefit ?

---------------------------------------------------------------------------

by henrikbjorn at 2012-04-13T14:26:40Z

why `input_widget` ? and not just `widget`

---------------------------------------------------------------------------

by Burgov at 2012-04-13T14:27:49Z

@juliendidier dynamic inheritance is now obsolete which fixes some other issues

---------------------------------------------------------------------------

by stloyd at 2012-04-13T14:37:26Z

What about __not__ breaking API so *badly* and leaving `FieldType` which will be simple like (with marking as deprecated):

```php
<?php

class FieldType extends AbstractType
{
    public function getParent(array $options)
    {
        return 'form';
    }

    public function getName()
    {
        return 'field';
    }
}

---------------------------------------------------------------------------

by bschussek at 2012-04-13T14:43:41Z

@stloyd That's a very good idea.

---------------------------------------------------------------------------

by mvrhov at 2012-04-13T17:41:21Z

IMHO what @stloyd proposed sounds like a good idea, but removing FieldType class, if #3903 will come into life might ensure that more forms will broke and people will check them thoroughly.

---------------------------------------------------------------------------

by r1pp3rj4ck at 2012-04-13T18:46:08Z

@bschussek looks great, but I'm concerned about how quickly will the third-party bundles adapt to this BC break. I hope really quick, because if they don't the whole stuff will be useless :S of course it's not your problem to solve.

---------------------------------------------------------------------------

by stof at 2012-04-13T18:50:32Z

@r1pp3rj4ck there is already another BC break requiring to update custom types for Symfony master. So third party bundles already have to do some work.

---------------------------------------------------------------------------

by r1pp3rj4ck at 2012-04-13T18:59:37Z

@stof which one? I've looked into @bschussek 's RFC about these [foo].bar stuff, but it's not yet implemented. Are you refering to this or another one I've missed?

---------------------------------------------------------------------------

by stof at 2012-04-13T19:04:06Z

@r1pp3rj4ck the change regarding default options

---------------------------------------------------------------------------

by r1pp3rj4ck at 2012-04-13T19:06:10Z

@stof oh, I forgot that one. Weird thing is that I've already changed my default options today and still forgetting these stuff :D

---------------------------------------------------------------------------

by bschussek at 2012-04-14T08:58:29Z

I restored and deprecated FieldType now. I'd appreciate further reviews.

---------------------------------------------------------------------------

by stloyd at 2012-04-14T09:02:32Z

Maybe we should try to avoid this BC in templates ? What do you think about similar move like with `FieldType` ? (hold old, but inside just render new)

---------------------------------------------------------------------------

by bschussek at 2012-04-14T09:07:22Z

@stloyd You mean for those cases where people explicitely render the block "field_*"? We can do that.

---------------------------------------------------------------------------

by stloyd at 2012-04-14T09:09:45Z

@bschussek Yes I mean this case =) Sorry for not being explicit, I need some coffee I think =)

---------------------------------------------------------------------------

by bschussek at 2012-04-17T14:45:35Z

I added the field_* blocks again for BC. Could someone please review again? Otherwise this can be merged.

---------------------------------------------------------------------------

by Burgov at 2012-04-17T15:11:16Z

@bschussek I'm not sure what has changed to cause this, but if I try out your branch on our forms, if I leave the value of an input empty, eventually the reverseTransform method receives a null value, rather than a '' (empty string) value, as on the current symfony master.

DateTimeToLocalizedStringTransformer, for example, will throw an Exception if the value is not a string

```php
if (!is_string($value)) {
   throw new UnexpectedTypeException($value, 'string');
}
```

Other than that, all forms render just the same as they do on symfony master

---------------------------------------------------------------------------

by bschussek at 2012-04-17T15:30:29Z

@Burgov Fixed.
@webmozart
Copy link
Contributor Author

I discovered a major issue: type extensions are very difficult to implement with native inheritance. They need to be triggered after each method of the type they are extending, i.e.:

$type = EntityType();
$type->buildForm(...)

with native inheritance results in (due to the parent::buildForm() calls at the very beginning)

1.  FormType::buildForm()
2.  ChoiceType::buildForm()
3.  EntityType::buildForm()

but should result in the execution chain

1.  FormType::buildForm()
1a. all FormType extensions ::buildForm()
2.  ChoiceType::buildForm()
2a. all ChoiceType extensions ::buildForm()
3.  EntityType::buildForm()
3a. all EntityType extensions ::buildForm()

Obviously, this is non-trivial to implement. It would require to generate Proxy classes for the types in order to simulate AOP-like behavior, which sucks somewhat and is just as intransparent as the current architecture.

Ideas?

@marijn
Copy link
marijn commented May 15, 2012

I'm not entirely sure I understand your question. The order of execution is incorrect or you don't want an execution chain down to the parent class? If you could explain it with an example it would be easier to help.

@stof
Copy link
Member
stof commented May 15, 2012

I don't want to remove the type extensions as they are a great extension point of the component. And if the new version is not cleaner than the current one, I don't really see the added value about breaking BC (and I see the drawback of breaking BC)

@marijn
Copy link
marijn commented May 15, 2012

I agree with @stof on that: the only reason reason for a BC break in the type system is if we can replace it with native inheritance.

@Tobion
Copy link
Contributor
Tobion commented May 15, 2012

@bschussek I guess the extensions must be called inbetween because they might modify the form and these changes must be available to sub-types, correct? Looking at your example, my first idea would be to include this logic in AbstractType, so all extensions get called as soon as a the method of the type is executed. I guess that is too simple. But why does it not work?

AbstractType::buildForm(FormBuilder $builder, array $options)
{
    forearch ($this->getExtensions() as $extension) {
        $extension->buildForm($builder, $options);
    }
}

@schmittjoh
Copy link
Contributor

Maybe just do not pass in the options to the getParent() method and leave inheritance as is otherwise. At least, that should address the original ticket which triggered this.

As @stof said, the extensions are a great feature, and I'd even say that they are not only difficult but impossible to implement. You would not only have to generate proxy classes, but also rewrite userland code to extend a different class.

@webmozart
Copy link
Contributor Author

@Tobion: Your example does not work, because buildForm() in your example is only called on top of the inheritance chain (AbstractType) and not between each layer.

@stof, @schmittjoh: Removing extensions is out of question. The only feasible way I see (which is basically leaving in the current approach but using PHP inheritance) is to avoid calling the parent methods, i.e.

class MyType extends FormType
{
    public function buildForm()
    {
        // no call to parent::buildForm here
    }
}

The framework could then take a look at the inheritance chain and invoke each method in the chain separately "FormType::buildForm", then the extensions, then "MyType::buildForm" etc.). But I don't think this makes the code any clearer.

@Tobion
Copy link
Contributor
Tobion commented May 15, 2012

@bschussek you're right. Why not include this logic explicisely in each type:

class FormType extends AbstractType
{
    public function buildForm(...)
    {
        parent::buildForm(...)
        forearch ($this->getExtensions() as $extension) {
  
6D40
          $extension->buildForm($builder, $options);
        }
        // or offer a shortcut method for this logic
    }
}

It wouldn't be a problem to always use this for all core types. And for custom user types, it's the developers responsibility to implement this logic if he needs it. And he only needs it when he has added extensions to one of his own types, so he would already know that he must call it.

class MyType extends FormType
{
    public function buildForm(...)
    {
        parent::buildForm(...)
        // call it or not (depending on needs)
        forearch ($this->getExtensions() as $extension) {
            $extension->buildForm($builder, $options);
        }
    }
}

Maybe I'm expecting too much of the devs. But on the other hand, they would already need to be aware to call the parent method, so calling another method should not be that unexpected if he needs this extensibility for his type.

@alvarezmario
Copy link
Contributor

@Tobion forcing the developer to do that is pretty ugly, I think there should be another solution.

@Tobion
Copy link
Contributor
Tobion commented May 15, 2012

As I said, he is not forced to do that as it's his decision to make it extensible or not. I agree it's not perfect but doing it explicitely also has some advantages.

@webmozart
Copy link
Contributor Author

I agree with @Nomack84. If we provide an extension mechanism, we have to guarantee it works. Otherwise it's flawed.

@Tobion
Copy link
Contributor
Tobion commented May 15, 2012

Then how about using events for this that are registered when adding an extension? So we don't need proxy classes, it's explicit and works consistently without any burden on the developer.

@schmittjoh
Copy link
Contributor

Where would you put the event-dispatching code?

On Tue, May 15, 2012 at 1:29 PM, Tobias Schultze <
reply@reply.github.com

wrote:

Then how about using events for this that are registered when adding an
extension? So we don't need proxy classes, it's explicit and works
consistently without any burden on the developer.


Reply to this email directly or view it on GitHub:
#3879 (comment)

@Tobion
Copy link
Contributor
Tobion commented May 15, 2012

Good point, I close my case.

@jonathaningram
Copy link
Contributor

FWIW, in my current app, I've got some types that use getParent and some that use PHP's extends as I did not know which was the preferred method and my original call on the user group did not have any response. So I'd at least vote for a big fat message in the docs (if it isn't there - maybe I missed it?) explaining what is the best practice. But at the end of the day, I think that both solutions are elegant enough as long as they are not fundamentally flawed and do not cause un-fixable bugs.

@stof
Copy link
Member
stof commented May 23, 2012

using F438 the PHP inheritance currently does not produce the expected result: it will not inherit the rendering of the parent type (as it will not be considered as a child of the type)

@jonathaningram
Copy link
Contributor

@stof fair enough, but (luckily) in my case using extends has not resulted in any problems thus far (that I know of), however unfortunately I got in that position in the first place because native inheritance is more intuitive I guess.

@webmozart
Copy link
Contributor Author

@stof As I said, we can use native inheritance if we force the user not to do the parent:: calls by himself. I updated the description above in this regard.

@AurelC2G
Copy link
Contributor

I only see one issue here: in this setup, wouldn't it become impossible to easily overwrite the behaviour of the parent type? (= to avoid calling the parent's method and to replace it completely)

@stof
Copy link
Member
stof commented May 23, 2012

@AurelC2G if you don't want to call the logic of the parent type, simply don't make your type a child of the other type.

@AurelC2G
Copy link
Contributor

@stof Well some methods could still be common (inherited without any changes), and some other completely rewritten (buildView for example). I do not have any specific use case in mind, I am just trying to figure out whether this solution will hinder us later on.

@stof
Copy link
Member
stof commented May 23, 2012

@AurelC2G if you change the logic, you will likely break the parent type as you would change the way the form is built (buildView depends of what has been done in buildForm and the form theme depends of what has been done in buildView)

@AurelC2G
Copy link
Contributor

Well if you are positive about this, @bschussek's solution seems both clear and efficient to me. We will just have to emphasize in the docs that the parent methods must not be called.

@stof
Copy link
Member
stof commented May 23, 2012

Using native inheritance but forbidding to call the parent method (and calling it in an hidden way) does not seem better than the current solution IMO. Especially as child types could then mess things in the parent type if they are modifying some protected properties somewhere

@stof
Copy link
Member
stof commented May 23, 2012

and it would also break things if a child type does not overwrite all methods: calling the method on it would call the parent logic, which would then be applied twice (as it will be applied again when calling the parent type). This is due to the fact that omitting the method will call the parent one, which is exactly what breaks things in this proposal.

@webmozart
Copy link
Contributor Author

@stof Not the latter. We need to do introspection for this anyway, so we can just as well avoid to call a method that is not defined on a specific class.

@stof
Copy link
Member
stof commented May 23, 2012

Well, doing such introspection would not make the inheritance easier to understand than currently IMO (and maybe even more complicated as people will expect the PHP inheritance to work for calling the parent method). Btw, I see a big issue with the getName method here too (currently, if you don't define it, PHP will complain directly about a missing method of the interface, and most IDEs will warn you even before executing the code)

@webmozart
Copy link
Contributor Author

Yes, introspection definitely is not straight-forward. Btw, we could also catch the case where getName() is not defined and throw an exception.

The question is whether the benefits mentioned above outweigh the downsides or not.

@stof
Copy link
Member
stof commented May 23, 2012

I don't think we need instanceof so much. It cannot be checked in Twig templates anyway.

And for the understandable inheritance, I think that the magic applied using introspection and the restriction about the calls to parent methods would make it confusing and not so much understandable. So IMO, it is not worth breaking the BC here

@webmozart
Copy link
Contributor Author

Ok. Then the remaining changes are:

  • getParent: remove argument $options
  • getDefaultOptions: rename to setDefaultOptions and pass options resolver
  • buildViewBottomUp: rename to finishView (I hate the old name, better suggestions welcome)

What do you think?

@webmozart
Copy link
Contributor Author

Btw, for the second and the third case we can keep BC. For the first we cannot.

@stof
Copy link
Member
stof commented May 23, 2012

is there any need for the options in getParent now ? I think the merge of the field and form types removed this needs for all core types, right ?

for the other points, I think it is a good idea to clean things. The naming for buildViewBottomUp is weird and could be confusing for people.

fabpot added a commit that referenced this issue May 25, 2012
Commits
-------

bc15e2d [Form] Some code cleanup
94f6f77 Restructured Form section of UPGRADE
3d800af [Form] Remove usages of deprecated features
ee803cd [Form] Renamed setVars() to addVars() in FormViewInterface
1c4f632 [Form] Fixed API docs and usage of FormBuilderInterface instead of FormBuilder
2e6cdd1 [Form] Inverted the logic of "single_control" and renamed it to "compound". The opposite is now "simple".
98a7c0c [Form] Consolidated FormInterface, FormBuilderInterface and FormViewInterface
8e128fc [Form][OptionsResolver] Fixed typos
877d8f7 [Form] Reversed the order of $type and $name in FormFactory::createNamed[Builder]()
33fecca [Form] Merged various form events and added class FormEvent
bec8015 [Form] Renamed client and application format to view and model format
8cae328 [Form] setDefaultOptions() is now coded against OptionsResolverInterface
1ecddbc [OptionsResolver] Renamed recommended method to setDefaultOptions()
dc2fa9d [OptionsResolver] Added OptionsResolverInterface
2cd99e8 [Form] Added FormBuilderInterface and FormViewInterface and cleaned up FormTypeInterface and FormTypeExtensionInterface
0ef4066 [Form] Options are now passed to buildView() and buildViewBottomUp()
027259e [Form] Changed getDefaultOptions() to setDefaultOptions(OptionsResolver $resolver) in FormTypeInterface
b4e8bcf [OptionsResolver] Relaxed tests to check that allowed values can also be passed as scalars
97de004 [OptionsResolver] Added option type validation capabilities
0af5f06 [OptionsResolver] Added method setFilters() for augmenting the final option values

Discussion
----------

[Form] Cleaned up the Form API

Bug fix: no
Feature addition: no
Backwards compatibility break: **YES**
Symfony2 tests pass: yes
Fixes the following tickets: #3855, #3879, #4342, #4371, #4375
Todo: -

This PR cleans up the Form API as described in the referenced tickets in order to stabilize and freeze this API in the future. BC is kept wherever possible. After this PR, form types are expected to follow the following interface:

```php
<?php

class MyType extends AbstractType
{
	public function buildForm(FormBuilderInterface $builder, array $options)
	{
	}

	public function buildView(FormViewInterface $view, FormInterface $form, array $options)
	{
	}

	public function finishView(FormViewInterface $view, FormInterface $form, array $options)
	{
	}

	public function setDefaultOptions(OptionsResolverInterface $resolver)
	{
	}

	public function getParent()
	{
	    return 'form';
	}

	public function getName()
	{
	    return 'my_type';
	}
}
```

Note that the options are now passed to `buildView` and `finishView` (formerly `buildViewBottomUp`) as well, reducing the need for creating form attributes in most cases.

---------------------------------------------------------------------------

by travisbot at 2012-05-23T19:07:44Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414486) (merged 277f5f78 into e023807).

---------------------------------------------------------------------------

by bschussek at 2012-05-23T19:51:40Z

The PR now also contains the fix for #4342.

---------------------------------------------------------------------------

by travisbot at 2012-05-23T19:51:55Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1414932) (merged 13d284ba into e023807).

---------------------------------------------------------------------------

by travisbot at 2012-05-24T06:55:35Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1419776) (merged 5aba0778 into e023807).

---------------------------------------------------------------------------

by travisbot at 2012-05-24T06:56:28Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1419783) (merged 00c4f7e into b07fb3c).

---------------------------------------------------------------------------

by travisbot at 2012-05-24T12:26:25Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1421748) (merged 73cd9f8 into b07fb3c).

---------------------------------------------------------------------------

by bschussek at 2012-05-24T12:27:32Z

The FormView changes described in #4371 are now contained as well. Invitation for final review.

---------------------------------------------------------------------------

by travisbot at 2012-05-24T14:03:10Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1422116) (merged e1f502b into b07fb3c).

---------------------------------------------------------------------------

by cordoval at 2012-05-25T03:26:05Z

any ETA @bschussek ? I want to use it for tackling the problem of dynamic selects city-state
here http://www.craftitonline.com/2011/08/symfony2-ajax-form-republish/

I am told:
"getDefaultOptions is changed to setDefaultOptions which will allow you to set depenedent values based on other forms"

so I am most interested +300!

---------------------------------------------------------------------------

by bschussek at 2012-05-25T06:08:53Z

@cordoval I think you misunderstood this. The OptionsResolver allows you to set options dependent on other options, but of the same field. Also, this is already possible before this merge by setting an option to a closure as described in the README of the OptionsResolver component.

---------------------------------------------------------------------------

by travisbot at 2012-05-25T06:35:53Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1430534) (merged b61cc55 into b07fb3c).

---------------------------------------------------------------------------

by vicb at 2012-05-25T06:42:24Z

@bschussek great changes ! I have just sent you a PR with some modifs related to deprecated features. I'll rebase and submit the other one we have already discussed.

---------------------------------------------------------------------------

by travisbot at 2012-05-25T07:16:18Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1430699) (merged e18830d into b07fb3c).

---------------------------------------------------------------------------

by cordoval at 2012-05-25T07:19:07Z

@bschussek what is already possilble @ "Also, this is already possible before"

I am confused could you link to what you are referring to please?

---------------------------------------------------------------------------

by travisbot at 2012-05-25T07:22:07Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1430727) (merged 20c02a7 into b07fb3c).

---------------------------------------------------------------------------

by bschussek at 2012-05-25T07:22:29Z

```
public function getDefaultOptions()
{
    return array(
        'a' => 'foo',
        'b' => function (Options $options) {
            return 'bar' === $options['a'] ? 'x' : 'y';
        }
    );
}

---------------------------------------------------------------------------

by travisbot at 2012-05-25T10:38:04Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1431903) (merged bc15e2d into 45849ce).
@fabpot fabpot closed this as completed May 25, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

0