Permalink
Browse files

merged branch davedevelopment/form-extension-points (PR #553)

This PR was squashed before being merged into the master branch (closes #553).

Commits
-------

7c38c9a Add services allowing for form type extensions and guessers

Discussion
----------

Add services allowing for form type extensions and guessers

I don't like the empty `FormTypeGuesserChain`, but `PreloadedExtension` requires one to be passed until symfony/symfony#5479 gets merged.

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

by fabpot at 2012-11-24T11:54:08Z

symfony/symfony#5479 has been merged now.

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

by davedevelopment at 2012-11-25T19:59:41Z

So, do we want this documenting at this point? I'm glad to have removed the dummy TypeGuesser, but without it, people who don't have a 'dev' version of symfony/form, will need the workaround in their userland code, which may cause headaches.

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

by fabpot at 2013-01-10T07:34:15Z

I think we need to switch to Symfony 2.2 in Silex now. What others think? @igorw

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

by davedevelopment at 2013-01-10T07:54:20Z

๐Ÿ‘ from me, though while we're on the topic, are there any plans to tag a 1.0?

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

by igorw at 2013-01-10T13:52:38Z

@fabpot Is there any reason why you would want to enforce users to use an unstable version?

I am against it for the following reasons:

* Users should be able to get stable versions if the stable versions work correctly.
* Silex already allows you to get 2.2, simply by lowering the `minimum-stability` to `beta`.
* Our current recommended way to install silex is `1.0.*@dev` (except on the download page, because it was not updated correctly when we changed it). This means that once they update it will blow up (due to `minimum-stability`), which will result in a lot of frustration, and a lot of support requests.

IMO we should encourage our users to try 2.2 by lowering their `minimum-stability`, but not enforce it.

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

by stof at 2013-01-10T13:55:34Z

another solution would be to bump the requirement to 2.2 only after releasing Silex.

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

by fabpot at 2013-01-11T07:51:36Z

Silex has not reached 1.0 yet. So, switching to 2.2 seems logical to me. That being said, we can wait for 2.2 stable to be out if this is what you suggest.

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

by igorw at 2013-01-11T16:23:06Z

Bumping to 2.2 after it is stable sounds acceptable to me, but unless there is a specific change in 2.2 that silex *needs*, I still don't really see the point. Supporting 2.1+ is easy enough to do.

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

by davedevelopment at 2013-03-06T13:28:19Z

So the current status with this PR, is that adding a TypeExtension with anything less than 2.1.4 of the symfony components will break, unless the user also adds an FormTypeGuesserChain to the guessers array. That is, the test `testFormServiceProviderWillLoadTypeExtensions` will fail with symfony/form < 2.1.4.

If we want to merge this in I guess we can:

* Document the whole feature, with a note/gotcha for Symfony <2.1.4
* Bump the minimum Symfony component versions to 2.1.4
* Just merge it anyway

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

by stof at 2013-03-06T14:13:38Z

bumping to 2.1.4 as minimum is fine IMO. I don't see why someone would want to use 2.1.0 explicitly instead of using the bufix releases for 2.1

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

by fabpot at 2013-03-06T21:39:29Z

๐Ÿ‘ for bumping to 2.1.4+

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

by igorw at 2013-03-06T22:08:37Z

In this particular case I'd say it's acceptable to bump to `2.1.4`.

In the future these kind of changes will need more consideration though, as they can cause BC issues when multiple branches of symfony are supported by silex.

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

by fabpot at 2013-03-08T12:05:04Z

When we said bumping to 2.1.4, I thought we were talking about bumping for just the component that has problems with earlier Symfony version, not all of them.

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

by davedevelopment at 2013-03-08T12:14:06Z

I did that because I wasn't sure how to approach it, I'll have a think about it, but I'll probably need some guidance.

I guess we may have to fall back on documenting that we have a dependency on 2.1.4 version of the Form Component, because it's not actually required? Maybe that should be in `suggest`? I don't think suggest enforces version requirements, so it either needs to be in the suggest message, or in the docs?

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

by igorw at 2013-03-08T12:22:59Z

It should be in both. Specifically, the form service provider docs.

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

by davedevelopment at 2013-03-08T13:01:26Z

Changed the requirements bump and documented, do we need to have something more specific/wordy in the docs?

Squashed a little as well, can squash further if it's desirable.
  • Loading branch information...
2 parents fd3ba62 + 7c38c9a commit 8e6d30ac82286767be18bb657d371ff525597321 @fabpot committed Mar 8, 2013
Showing with 107 additions and 4 deletions.
  1. +1 โˆ’1 .travis.yml
  2. +3 โˆ’2 composer.json
  3. +18 โˆ’1 doc/providers/form.rst
  4. +11 โˆ’0 src/Silex/Provider/FormServiceProvider.php
  5. +74 โˆ’0 tests/Silex/Tests/Provider/FormServiceProviderTest.php
View
@@ -7,7 +7,7 @@ env:
before_script:
# symfony/*
- sh -c "if [ '$SYMFONY_DEPS_VERSION' = '2.2' ]; then sed -i 's/>=2.1,<2.3-dev/2.2.*@dev/g' composer.json; composer require --no-update 'symfony/property-access:2.2.*@dev'; composer update --dev --prefer-source; fi"
- - sh -c "if [ '$SYMFONY_DEPS_VERSION' = '2.1' ]; then sed -i 's/>=2.1,<2.3-dev/2.1.*@dev/g' composer.json; composer update --dev --prefer-source; fi"
+ - sh -c "if [ '$SYMFONY_DEPS_VERSION' = '2.1' ]; then sed -i 's/>=2.1\(.[0-9]\+\)\?,<2.3-dev/2.1.*@dev/g' composer.json; composer update --dev --prefer-source; fi"
- composer install --dev --prefer-source
php:
View
@@ -26,7 +26,7 @@
"symfony/security": ">=2.1,<2.3-dev",
"symfony/config": ">=2.1,<2.3-dev",
"symfony/locale": ">=2.1,<2.3-dev",
- "symfony/form": ">=2.1,<2.3-dev",
+ "symfony/form": ">=2.1.4,<2.3-dev",
"symfony/browser-kit": ">=2.1,<2.3-dev",
"symfony/css-selector": ">=2.1,<2.3-dev",
"symfony/dom-crawler": ">=2.1,<2.3-dev",
@@ -45,7 +45,8 @@
"suggest": {
"symfony/browser-kit": ">=2.1,<2.3-dev",
"symfony/css-selector": ">=2.1,<2.3-dev",
- "symfony/dom-crawler": ">=2.1,<2.3-dev"
+ "symfony/dom-crawler": ">=2.1,<2.3-dev",
+ "symfony/form": "To make use of the FormServiceProvider, >= 2.1.4 is required"
},
"autoload": {
"psr-0": { "Silex": "src/" }
@@ -55,7 +55,7 @@ Registering
.. code-block:: json
"require": {
- "symfony/form": "~2.1"
+ "symfony/form": "~2.1.4"
}
If you are going to use the validation extension with forms, you must also
@@ -170,6 +170,23 @@ You can register form extensions by extending ``form.extensions``::
return $extensions;
}));
+
+You can register form type extensions by extending ``form.type.extensions``::
+
+ $app['form.type.extensions'] = $app->share($app->extend('form.type.extensions', function ($extensions) use ($app) {
+ $extensions[] = new YourFormTypeExtension();
+
+ return $extensions;
+ }));
+
+You can register form type guessers by extending ``form.type.guessers``::
+
+ $app['form.type.guessers'] = $app->share($app->extend('form.type.guessers', function ($guessers) use ($app) {
+ $guessers[] = new YourFormTypeGuesser();
+
+ return $guessers;
+ }));
+
Traits
------
@@ -18,6 +18,7 @@
use Symfony\Component\Form\Extension\Csrf\CsrfProvider\SessionCsrfProvider;
use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationExtension;
use Symfony\Component\Form\Extension\Validator\ValidatorExtension as FormValidatorExtension;
+use Symfony\Component\Form\FormTypeGuesserChain;
use Symfony\Component\Form\Forms;
/**
@@ -46,6 +47,14 @@ public function register(Application $app)
$app['form.secret'] = md5(__DIR__);
+ $app['form.type.extensions'] = $app->share(function ($app) {
+ return array();
+ });
+
+ $app['form.type.guessers'] = $app->share(function ($app) {
+ return array();
+ });
+
$app['form.extensions'] = $app->share(function ($app) {
$extensions = array(
new CsrfExtension($app['form.csrf_provider']),
@@ -67,6 +76,8 @@ public function register(Application $app)
$app['form.factory'] = $app->share(function ($app) {
return Forms::createFormFactoryBuilder()
->addExtensions($app['form.extensions'])
+ ->addTypeExtensions($app['form.type.extensions'])
+ ->addTypeGuessers($app['form.type.guessers'])
->getFormFactory()
;
});
@@ -0,0 +1,74 @@
+<?php
+
+/*
+ * This file is part of the Silex framework.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Silex\Tests\Provider;
+
+use Silex\Application;
+use Silex\Provider\FormServiceProvider;
+
+use Symfony\Component\Form\AbstractTypeExtension;
+use Symfony\Component\Form\FormTypeGuesserChain;
+use Symfony\Component\OptionsResolver\OptionsResolverInterface;
+
+class FormServiceProviderTest extends \PHPUnit_Framework_TestCase
+{
+ public function testFormFactoryServiceIsFormFactory()
+ {
+ $app = new Application();
+ $app->register(new FormServiceProvider());
+ $this->assertInstanceOf('Symfony\Component\Form\FormFactory', $app['form.factory']);
+ }
+
+ public function testFormServiceProviderWillLoadTypeExtensions()
+ {
+ $app = new Application();
+
+ $app->register(new FormServiceProvider());
+
+ $app['form.type.extensions'] = $app->share($app->extend('form.type.extensions', function($extensions) {
+ $extensions[] = new DummyFormTypeExtension();
+ return $extensions;
+ }));
+
+ $form = $app['form.factory']->createBuilder('form', array())
+ ->add('file', 'file', array('image_path' => 'webPath'))
+ ->getForm();
+
+ $this->assertInstanceOf('Symfony\Component\Form\Form', $form);
+ }
+
+ public function testFormServiceProviderWillLoadTypeGuessers()
+ {
+ $app = new Application();
+
+ $app->register(new FormServiceProvider());
+
+ $app['form.type.guessers'] = $app->share($app->extend('form.type.guessers', function($guessers) {
+ $guessers[] = new FormTypeGuesserChain(array());
+ return $guessers;
+ }));
+
+ $this->assertInstanceOf('Symfony\Component\Form\FormFactory', $app['form.factory']);
+ }
+}
+
+class DummyFormTypeExtension extends AbstractTypeExtension
+{
+ public function getExtendedType()
+ {
+ return 'file';
+ }
+
+ public function setDefaultOptions(OptionsResolverInterface $resolver)
+ {
+ $resolver->setOptional(array('image_path'));
+ }
+}

0 comments on commit 8e6d30a

Please sign in to comment.