- Added compiler class which picks up any services tagged "twig.loader"
- If there is one loader registered we set the alias to this loader
- If there is more than one we set the alias to a chain loader and all
the loaders to it
- If there is no loaders we throw an Exception
This PR was merged into the 2.1 branch.
Commits
-------
6e7e08f [Form] Fixed the default value of "format" in DateType to DateType::DEFAULT_FORMAT if "widget" is not "single_text"
Discussion
----------
[Form] Fixed the "format" option in DateType
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #6141
Todo: -
License of the code: MIT
Documentation PR: -
This PR fixes a regression introduced in #4839. To quote that PR:
> This PR changes DateType and DateTimeType to support HTML5 by default when setting the option "widget" to "single_text".
In reality, the "format" option now defaults to the HTML5 format always, not just when "widget" is "single_text". This is fixed here.
The second commit in this PR removes special characters between select/text fields. What, with German locale, was
```
<day input>.<month input>.<year input>
```
before is now
```
<day input><month input><year input>
```
This is the way date fields are represented on the majority of websites. If you *need* separators, you can have them by setting the "format" option to a custom value:
```php
$builder->add('myDate', 'date', array(
'format' => 'dd.MM.yyyy',
));
```
---------------------------------------------------------------------------
by fabpot at 2012-12-07T08:52:21Z
The second commit should probably be done on master and it changes the behavior.
---------------------------------------------------------------------------
by bschussek at 2012-12-07T12:23:22Z
Ok, I removed the second commit now and removed the entries from the CHANGELOG.
This PR was merged into the master branch.
Commits
-------
d7a1154 make it possible for bundles extensions to prepend settings into the application configuration of any Bundle
Discussion
----------
[2.2] add possibility for bundles extensions to prepend the app configs
Bug fix: #4652
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
As can be seen in the patch the extensions that should prepend the configuration are enabled automatically if they implement ``PrependExtensionInterface``.
Just as an example, here an extension, which checks if SonataAdminBundle is available and if not disables integration with it in several Bundles. It also sets some default settings for ``document_class`` and ``default_document_manager_name``:
```
diff --git a/DependencyInjection/SymfonyCmfCoreExtension.php b/DependencyInjection/SymfonyCmfCoreExtension.php
index 9f92410..c0a8dbb 100644
--- a/DependencyInjection/SymfonyCmfCoreExtension.php
+++ b/DependencyInjection/SymfonyCmfCoreExtension.php
@@ -3,11 +3,12 @@
namespace Symfony\Cmf\Bundle\CoreBundle\DependencyInjection;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
+use Symfony\Component\\DependencyInjection\PrependExtensionInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
use Symfony\Component\Config\FileLocator;
-class SymfonyCmfCoreExtension extends Extension
+class SymfonyCmfCoreExtension extends Extension implements PrependExtensionInterface
{
public function load(array $configs, ContainerBuilder $container)
{
@@ -15,4 +16,45 @@ class SymfonyCmfCoreExtension extends Extension
$loader->load('config.xml');
$loader->load('services.xml');
}
+
+ public function prepend(ContainerBuilder $container)
+ {
+ $bundles = $container->getParameter('kernel.bundles');
+ if (!isset($bundles['SonataDoctrinePHPCRAdminBundle'])) {
+ // disable SonataDoctrinePHPCRAdminBundle admin support in Bundles
+ $config = array('use_sonata_admin' => false);
+ foreach ($container->getExtensions() as $name => $extension) {
+ switch ($name) {
+ case 'symfony_cmf_menu':
+ case 'symfony_cmf_routing_extra':
+ case 'symfony_cmf_simple_cms':
+ $container->prependExtensionConfig($name, $config);
+ break;
+ }
+ }
+ }
+
+ // process the configuration of SymfonyCmfCoreExtension
+ $configs = $container->getExtensionConfig($this->getAlias());
+ $config = $this->processConfiguration(new Configuration(), $configs);
+ // add the default configs to various Bundles
+ foreach ($container->getExtensions() as $name => $extension) {
+ switch ($name) {
+ case 'symfony_cmf_content':
+ case 'symfony_cmf_simple_cms':
+ $container->prependExtensionConfig($name, $config);
+ break;
+ }
+ }
+ }
}
```
---------------------------------------------------------------------------
by stof at 2012-09-21T21:10:00Z
I think you are giving too much power to bundles here: a bundle becomes able to modify all the config defined explicitly by the user if it wants to do it.
I think it would be safer to give them the possibility to load an additional config file which would be prepended (so that user-defined config would still win). Giving the ability to load files means passing the loader used by the kernel, and it should then be called before calling the load method on the kernel itself (to respect the order of loaded files)
---------------------------------------------------------------------------
by lsmith77 at 2012-09-22T05:50:08Z
Not sure how a config file helps solve anything. I mean they can load as many config files as they want already. The key is being able to automatically apply configuration to multiple Bundles as well as enabling/disabling features based on if certain Bundles are registered.
BTW the end result in my examples is also prepended, so that user config wins. However yes this would be up to the person implementing the Bundle. We could however provide a dedicated method for prepending in addition to or instead of ``setExtensionConfig``.
---------------------------------------------------------------------------
by stof at 2012-09-22T11:40:29Z
@lsmith77 If you can load a file with the main loader, this file can provide some app-level configuration (be it for your own bundle or others).
And your code example is indeed prepending. But imagine what would occur when someone uses this feature without knowing well how the component works: he will likely call ``setExtensionConfig`` in a first implementation, thus dropping all userland config for the bundle. Your setup does not only allow to make a file win over the userland config but makes it even easier to remove the userland config.
---------------------------------------------------------------------------
by lsmith77 at 2012-09-22T18:11:29Z
but i dont get how that would help. the point is to be able for one bundle to configure other bundles before the load as this is obviously alot cleaner than trying to do the same via a compiler pass. so imho this is what is needed to encourage decoupled bundles. otherwise for example CMS or other reuseable and extensible apps will be forced to always put everything in one bundle.
---------------------------------------------------------------------------
by stof at 2012-09-22T19:23:45Z
@lsmith77 I agree about the feature, not about the way to implement it. If you allow bundles to load a file as it it were some app-level config, they would become able to provide some config for other bundles (and you could load several files depending of which bundles are enabled), but without allowing bundles to remove the userland config.
---------------------------------------------------------------------------
by lsmith77 at 2012-09-22T19:50:19Z
sorry but i dont understand what you suggest. more over i dont see the problem. its already possible to seriously break stuff with compiler passes which cannot be easily enabled/disabled. this is just convenience. if it doesnt work because of some obscure combo then simply dont use it for the app since it needs to be explicitly enabled in the kernel.
---------------------------------------------------------------------------
by lsmith77 at 2012-09-24T09:25:10Z
@stof thought about your comments, are you suggesting for a Bundle to be able to generate a config file that is prepended? in that case the current behavior would already be that if we change ``setExtensionConfig`` to just be a ``prependExtensionConfig`` .. however i am not sure if we really need this limitation since as i point out this would still by far be less dangerous than compiler passes and also i expect this to be used mainly by open source applications on top of Symfony2 rather than standard bundles.
---------------------------------------------------------------------------
by lsmith77 at 2012-10-13T13:28:29Z
@lolautruche i also think this is relevant for you guys. this way you could start preconfiguring 3rd party bundles as part of your main ezPublish bundle.
---------------------------------------------------------------------------
by lolautruche at 2012-10-13T13:57:09Z
While I suspect a nice feature, the implementation looks obscure to me...
---------------------------------------------------------------------------
by lsmith77 at 2012-10-13T17:43:02Z
The implementation of the example extension or the implementation of the actual changes proposed in this PR?
---------------------------------------------------------------------------
by lolautruche at 2012-10-13T17:46:57Z
The example, sorry 😃
---------------------------------------------------------------------------
by lsmith77 at 2012-10-13T17:50:38Z
The example was fairly quickly hacked together. The basic thing you need to do is fetch the config for the bundle you want to change, manipulate the config (usually by appending an array to the array of configs so that you dont affect explicit configuration) and then set it again.
As I explained to @stof it would alternative/additionally be possible to support a method that pushes a config array to the top of the array of config stack. Such a method might make the necessary code simpler.
---------------------------------------------------------------------------
by stof at 2012-10-13T21:39:07Z
@fabpot what do you think about it ?
---------------------------------------------------------------------------
by jrobeson at 2012-10-20T15:45:18Z
I've been porting much of an existing framework over to use more symfony components and bundles. I think that this might help some of the problems i've been having. I would really appreciate some better examples as how to one would use it (same for the cmf router, but that's another story).
---------------------------------------------------------------------------
by lsmith77 at 2012-10-21T07:28:52Z
not really sure what other examples i could give. the process is quite simple:
1) determine what configuration options to add to other Bundles
for example with the following code I determine that SonataAdmin for PHPCR is not installed (which means i should disable using it in other Bundles):
```
$bundles = $container->getParameter('kernel.bundles');
if (!isset($bundles['SonataDoctrinePHPCRAdminBundle'])) {
```
alternatively I could simply already process the configuration and then pick all or some of these configuration options:
```
$configs = $container->getExtensionConfig($this->getAlias());
$config = $this->processConfiguration(new Configuration(), $configs);
```
2) then add these configuration to what other Bundles I feel should get these options
usually I will add these to the top of the config array stack. this means that if the user would manually set the same setting in most cases the user setting will override what the pre-processor set.
```
$container->unshiftExtensionConfig($name, array('use_sonata_admin' => false));
```
---------------------------------------------------------------------------
by lsmith77 at 2012-10-24T12:52:38Z
added ``ContainerBuilder::unshiftExtensionConfig`` since this is the usual use case. with this method added it could be discussed if ``ContainerBuilder::setExtensionConfig`` is still needed or not.
---------------------------------------------------------------------------
by lsmith77 at 2012-11-24T14:48:44Z
I spoke to @fabpot today and he said that since this patch just allows you to set defaults and not really "process" the actual configuration I shouldn't call it "preProcess" so I renamed it to "prepend".
Furthermore as its just prepending @fabpot said there isnt really a need to require manually enabling it, so here is a patch to auto-enable the prepending logic:
```
diff --git a/src/Symfony/Component/HttpKernel/Kernel.php b/src/Symfony/Component/HttpKernel/Kernel.php
index b890fbf..7374b87 100644
--- a/src/Symfony/Component/HttpKernel/Kernel.php
+++ b/src/Symfony/Component/HttpKernel/Kernel.php
@@ -701,8 +701,8 @@ abstract class Kernel implements KernelInterface, TerminableInterface
*/
protected function prependExtensionConfigs(ContainerBuilder $container)
{
- foreach ($this->getPrependingExtensions() as $name) {
- $extension = $container->getExtension($name);
+ foreach ($this->bundles as $bundle) {
+ $extension = $bundle->getContainerExtension();
if ($extension instanceof PrependExtensionInterface) {
$extension->prepend($container);
}
@@ -710,16 +710,6 @@ abstract class Kernel implements KernelInterface, TerminableInterface
}
/**
- * Returns the ordered list of extensions that may prepend extension configurations.
- *
- * @return array
- */
- protected function getPrependingExtensions()
- {
- return array();
- }
-
- /**
* Gets a new ContainerBuilder instance used to build the service container.
*
* @return ContainerBuilder
```
---------------------------------------------------------------------------
by lsmith77 at 2012-11-25T19:31:01Z
ok .. i pondered the code some more and now i have enabled registering of the prepending extensions by default, since its now quite easy to just override the ``prependExtensionConfigs()`` method since there is almost no logic in there anymore.
@fabpot i am not 100% sure with the naming yet ..
---------------------------------------------------------------------------
by lsmith77 at 2012-12-05T14:03:43Z
@fabpot if you are ok with the PR as it is now, i can do the rebase so you can merge this?
---------------------------------------------------------------------------
by lsmith77 at 2012-12-05T18:30:29Z
@fabpot all good now? then i will squash the commits ..
---------------------------------------------------------------------------
by lsmith77 at 2012-12-05T18:34:50Z
actually looking at the full change set again i am no longer sure if it makes sense to have ``PrependExtensionInterface`` in the DI rather than the HttpKernel.
---------------------------------------------------------------------------
by lsmith77 at 2012-12-07T09:21:14Z
@fabpot all good now?
---------------------------------------------------------------------------
by fabpot at 2012-12-07T09:37:52Z
The code looks good to me now. There are two remaining task before merging:
* Is it something we need to add somewhere in the documentation?
* Can you add a note in the DI component CHANGELOG?
Thanks.
---------------------------------------------------------------------------
by lsmith77 at 2012-12-07T09:49:17Z
i have added a changelog entry and squashed the commits.
i will also work on a documentation entry, i guess i will make it a cookbook entry. not sure if it should be included in http://symfony.com/doc/2.0/cookbook/bundles/extension.html .. but imho it would better be a separate entry.
* 2.1:
[HttpFoundation] changed UploadedFile::move() to use move_uploaded_file() when possible (closes#5878, closes#6185)
[HttpFoundation] added a check for the host header value
[DoctrineBridge] Improved performance of the EntityType when used with the "query_builder" option
[DoctrineBridge] Improved exception message
[DoctrineBridge] Fixed: Exception is thrown if the entity class is not known to Doctrine
Removed useless branch alias for dev-master in composer.json
Conflicts:
composer.json
src/Symfony/Bridge/Doctrine/composer.json
src/Symfony/Bridge/Monolog/composer.json
src/Symfony/Bridge/Propel1/composer.json
src/Symfony/Bridge/Swiftmailer/composer.json
src/Symfony/Bridge/Twig/composer.json
src/Symfony/Bundle/FrameworkBundle/composer.json
src/Symfony/Bundle/SecurityBundle/composer.json
src/Symfony/Bundle/TwigBundle/composer.json
src/Symfony/Bundle/WebProfilerBundle/composer.json
src/Symfony/Component/BrowserKit/composer.json
src/Symfony/Component/ClassLoader/composer.json
src/Symfony/Component/Config/composer.json
src/Symfony/Component/Console/composer.json
src/Symfony/Component/CssSelector/composer.json
src/Symfony/Component/DependencyInjection/composer.json
src/Symfony/Component/DomCrawler/composer.json
src/Symfony/Component/EventDispatcher/composer.json
src/Symfony/Component/Filesystem/composer.json
src/Symfony/Component/Finder/composer.json
src/Symfony/Component/Form/composer.json
src/Symfony/Component/HttpFoundation/composer.json
src/Symfony/Component/HttpKernel/composer.json
src/Symfony/Component/Locale/composer.json
src/Symfony/Component/OptionsResolver/composer.json
src/Symfony/Component/Process/composer.json
src/Symfony/Component/Routing/composer.json
src/Symfony/Component/Security/composer.json
src/Symfony/Component/Serializer/composer.json
src/Symfony/Component/Templating/composer.json
src/Symfony/Component/Translation/composer.json
src/Symfony/Component/Validator/composer.json
src/Symfony/Component/Yaml/composer.json
* 2.0:
[HttpFoundation] changed UploadedFile::move() to use move_uploaded_file() when possible (closes#5878, closes#6185)
[HttpFoundation] added a check for the host header value
Conflicts:
src/Symfony/Component/HttpFoundation/File/File.php
src/Symfony/Component/HttpFoundation/Request.php
src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
This PR was merged into the 2.0 branch.
Commits
-------
0489799 [HttpFoundation] added a check for the host header value
Discussion
----------
[HttpFoundation] added a check for the host header value
alternative for #3865
This PR was squashed before being merged into the master branch (closes#6005).
Commits
-------
577ee80 [HttpFoundation] Move IP check methods to a HttpUtils class for reuse
Discussion
----------
[HttpFoundation] Move IP check methods to a HttpUtils class for reuse
---------------------------------------------------------------------------
by vicb at 2012-11-13T18:05:18Z
Thanks @stof ! (didn't get my copy paste error as PHP allow calling non static method w/o a warning).
---------------------------------------------------------------------------
by GromNaN at 2012-11-17T23:19:29Z
Having an `Utils` class with mixed functions doesn't seem to be a good practice. I think the class should be called something like `Symfony\Component\HttpFoundation\IpAddress`.
---------------------------------------------------------------------------
by vicb at 2012-11-27T09:37:20Z
@fabpot could this be merged if `HttpUtils` is renamed to `IpUtils` ?
---------------------------------------------------------------------------
by fabpot at 2012-12-06T13:35:28Z
Renaming the class to `IpUtils` is indeed a good idea.
---------------------------------------------------------------------------
by vicb at 2012-12-06T14:07:59Z
ready !
---------------------------------------------------------------------------
by fabpot at 2012-12-06T14:39:19Z
Can you add an entry in the CHANGELOG?
---------------------------------------------------------------------------
by vicb at 2012-12-06T14:53:09Z
done, thanks for the reminder !
This PR was merged into the 2.1 branch.
Commits
-------
b604eb7 [DoctrineBridge] Improved performance of the EntityType when used with the "query_builder" option
db2ee54 [DoctrineBridge] Improved exception message
99321cb [DoctrineBridge] Fixed: Exception is thrown if the entity class is not known to Doctrine
Discussion
----------
[DoctrineBridge] fixed caching when EntityType is used with the "query_builder" option
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
This PR was squashed before being merged into the master branch (closes#6207).
Commits
-------
57e9d28 [DI] Add a base class for extension
Discussion
----------
[DI] Add a base class for extension
depends on #6148
@fabpot should we change `addClassesToCompile` & the likes (thinking of traits).
---------------------------------------------------------------------------
by fabpot at 2012-12-06T13:05:05Z
Can you rebase?
---------------------------------------------------------------------------
by fabpot at 2012-12-06T13:06:43Z
hmmm, now that I see the result, I'm not sure it is worth it as the Extension class in the DI component depends on the Config one.
---------------------------------------------------------------------------
by vicb at 2012-12-06T13:23:29Z
No pb, I can remove it, should I remove the `ContainerBuilder` altogether ?
---------------------------------------------------------------------------
by fabpot at 2012-12-06T13:37:18Z
I would keep everything that is strictly in the DI namespace in the DI extension class and everything else in the HttpKernel class as it is now.
---------------------------------------------------------------------------
by vicb at 2012-12-06T13:38:59Z
But this change is **great** if you need the DI without the full stack.
What about my other comment ?
---------------------------------------------------------------------------
by fabpot at 2012-12-06T13:55:30Z
Which other comment? This one? "should I remove the ContainerBuilder altogether?" In which case, I don't understand what it means.
What about adding 2 classes in the DI component: the base one and another one with the dependency on the config component? Is it overkill?
---------------------------------------------------------------------------
by vicb at 2012-12-06T14:06:43Z
> "should I remove the ContainerBuilder altogether?"
I mean that the **widely used** (ie loaders) `ContainerBuilder` also depends on Config - that was kind of a joke !
I was refering to my first comment here
> should we change addClassesToCompile & the likes (thinking of traits).
Overkill I don't know but useless for sure: the `ExtensionInterface` depends on `ContainerBuilder` which depends on `Config`.
This PR was squashed before being merged into the master branch (closes#5853).
Commits
-------
63b0059 [Process] Add ability to reset arguments on ProcessBuilder
Discussion
----------
[Process] Add ability to reset arguments on ProcessBuilder
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
This PR adds the ability to "reset" the arguments set on a `ProcessBuilder`. This allows the builder to be re-used without having to set things like custom environment variables, current working directory etc again.
This PR was squashed before being merged into the master branch (closes#5860).
Commits
-------
d0057d0 Added failure_path_parameter to mirror target_path_parameter
Discussion
----------
Added failure_path_parameter to mirror target_path_parameter
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
Enable login failure redirect path can be assigned in a form field just like target path.
---------------------------------------------------------------------------
by stof at 2012-10-29T09:40:17Z
Please also open a PR to the doc repo to document this new feature
---------------------------------------------------------------------------
by leevigraham at 2012-10-29T09:56:29Z
@stof @fabpot Done.
This PR was merged into the master branch.
Commits
-------
7f16c1f [HttpKernel] Add DI extension configs as ressources when possible
Discussion
----------
[HttpKernel] Add DI extension configs as ressources when possible
/cc @rdohms @richardmiller
---------------------------------------------------------------------------
by vicb at 2012-11-30T11:57:48Z
btw @fabpot what about having a base class for `Extension` in the DI ? Would make it easier to re-use it when using standalone components, Di and (the suggested) Config as the greatest part of the class is not HttpKernel specific.
---------------------------------------------------------------------------
by fabpot at 2012-12-06T08:47:28Z
@vicb your suggestion makes sense.
Can you also explain the goal of this PR?
---------------------------------------------------------------------------
by vicb at 2012-12-06T09:01:58Z
The goal of this PR is to avoid having to sfcc when you modify a DI extension configuration. I think @rdohms got trapped.
---------------------------------------------------------------------------
by vicb at 2012-12-06T09:08:08Z
see https://twitter.com/rdohms/status/274059267428978688
---------------------------------------------------------------------------
by stof at 2012-12-06T09:20:54Z
I thought about it several times but never took time to implement it. It is annoying to have to clear the cache when you modify a default value in the Configuration class. So +1
This PR was squashed before being merged into the master branch (closes#6083).
Commits
-------
6236c18 [FrameworkBundle] Added caching to TemplateController
Discussion
----------
[FrameworkBundle] Added caching to TemplateController
Because the main purpose for the `TemplateController` seems to be to render static pages like "disclaimer" and such, it seems useful to allow caching.
imprint:
pattern: /imprint
defaults:
_controller: SymfonyFrameworkBundle:Template:template
template: "::pages/imprint.html.twig"
maxAge: 86400
---------------------------------------------------------------------------
by pierredup at 2012-11-21T20:24:53Z
IMHO I think the caching should be allowed to be set optionally
---------------------------------------------------------------------------
by KingCrunch at 2012-11-21T20:38:54Z
I wrote it this way, because I assume, that it will cover more use-cases, than the other way round, but you are right, that this will change the current behaviour. Would like to hear other opinions, because I don't think one uses this action for anything else than fully-static content (means: The current behaviour doesn't feel very useful to me).
---------------------------------------------------------------------------
by pierredup at 2012-11-21T20:48:19Z
I totally agree, but I would then suggest keep the caching on by default, but have the option to turn it off if necessary
---------------------------------------------------------------------------
by pierredup at 2012-11-21T20:52:01Z
Actually I think to have caching permanently enabled for static content would probably be the best scenario, but I like to think in terms of flexibility and specific user requirements. It would be great to get some opinions from others on this
---------------------------------------------------------------------------
by KingCrunch at 2012-11-23T21:12:45Z
I thought about it and I come to the conclusion, that it is probably a not so good idea to enable caching by default, because ... well, it's not possible to disable it again. I guess something like this
{{ render '@AcmeBundle:ArticleController:latest' with {count: 1} }}
may be not so uncommon as I suggested in the first commit.
---------------------------------------------------------------------------
by fabpot at 2012-12-03T22:18:51Z
Can you make a PR for the docs? (symfony/symfony-docs). Thanks.
This PR was squashed before being merged into the master branch (closes#6173).
Commits
-------
4878ec0 [HttpKernel] [WebProfilerBundle] Better handling of deprecated methods
Discussion
----------
[HttpKernel] [WebProfilerBundle] Better handling of deprecated methods
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes, if you were expecting E_USER_DEPRECATED or E_DEPRECATED to throw an exception
Symfony2 tests pass: yes
Fixes the following tickets: #6139 partly, I'd go through and add the actual trigger_error() calls in another (or possibly one per component) PR
Todo: call trigger_error()
License of the code: MIT
Documentation PR: -
I added the deprecation count with the Exception icon in the Profiler Toolbar, and changed the color of it to be yellow for deprecations and red for exceptions (was yellow for exceptions).
---------------------------------------------------------------------------
by fabpot at 2012-12-03T09:43:09Z
Adding trigger_error calls should be done in one PR to ease the merging. thanks.
This PR was merged into the master branch.
Commits
-------
a3a832c Fix CS in the whole Propel1 bridge
86ab4b3 Add typehint to isInteger(), fix tests
a26a690 remove useless ColumnMap
ffd8759 fix some formatting issue
6fb9536 fix indentation problem
e5e3341 oups. It seems that here, we need \PDO
36d6c40 fix indentation problem
6f8cd9d Removed the PropelColumnTypes.php copy
8125163 removed the TODO mention. Will keep the Propel code here so it can work with older version of Propel
0e4419b I found the error in my latest commit. This pass the test suite.
cf8a6c0 Fix my code and also fix the test suite.
737b596 Merge remote-tracking branch 'upstream/master'
972e503 Fix problem when 1 column identifier in propel is a string.
Discussion
----------
Fix ModelChoiceList problem with string key
Replaces #6150
---------------------------------------------------------------------------
by willdurand at 2012-12-05T20:51:44Z
Note that 5f54ed1 is a "CS fix" commit. I don't want to open a PR just for that. Let me know if I should to remove it.
Also, I'm 👍 on this PR. Review has been made already, so it seems mergeable.
This PR was merged into the master branch.
Commits
-------
51223c0 added upgrade instructions
50e6259 adjusted tests
98f3ca8 [Routing] removed tree structure from RouteCollection
Discussion
----------
[Routing] removed tree structure from RouteCollection
BC break: yes (see below)
Deprecations: RouteCollection::getParent(); RouteCollection::getRoot()
tests pass: yes
The reason for this is so quite simple. The RouteCollection has been designed as a tree structure, but it cannot at all be used as one. There is no getter for a sub-collection at all. So you cannot access a sub-collection after you added it to the tree with `addCollection(new RouteCollection())`. In contrast to the form component, e.g. `$form->get('child')->get('grandchild')`.
So you can see the RouteCollection cannot be used as a tree and it should not, as the same can be achieved with a flat array!
Using a flat array removes all the need for recursive traversal and makes the code much faster, much lighter, less memory (big problem in CMS with many routes) and less error-prone.
BC break: there is only a BC break if somebody used the PHP API for defining RouteCollection and also added a Route to a collection after it has been added to another collection.
So
```
$rootCollection = new RouteCollection();
$subCollection = new RouteCollection();
$rootCollection->addCollection($subCollection);
$subCollection->add('foo', new Route('/foo'));
```
must be updated to the following (otherwise the 'foo' Route is not imported to the rootCollection)
```
$rootCollection = new RouteCollection();
$subCollection = new RouteCollection();
$subCollection->add('foo', new Route('/foo'));
$rootCollection->addCollection($subCollection);
```
Also one must call addCollection from the bottom to the top. So the correct sequence is the following (and not the reverse)
```
$childCollection->->addCollection($grandchildCollection);
$rootCollection->addCollection($childCollection);
```
Remeber, this is only needed when using PHP for defining routes and calling methods in a special order. There is no change required when using XML or YAML for definitions. Also, I'm pretty sure that neither the CMF, nor Drupal routing, nor Silex is relying on the tree stuff. So they should also still work.
cc @fabpot @crell @dbu
One more thing: RouteCollection wasn't an appropriate name for a tree anyway as a collection of routes (that it now is) is definitely not a tree.
Yet another point: The XML declaration of routes uses the `<import>` element, which is excatly what the new implementation of addCollection without the need of a tree does. So this is now also more analogous.
---------------------------------------------------------------------------
by Koc at 2012-11-26T17:34:15Z
What benefit of this?
---------------------------------------------------------------------------
by Tobion at 2012-11-26T17:56:53Z
@Koc Why did you not simply wait for the description? ^^
---------------------------------------------------------------------------
by dbu at 2012-11-26T18:33:09Z
i love PR that remove more code than they add whithout removing functionality.
---------------------------------------------------------------------------
by Crell at 2012-11-26T18:49:52Z
There's an issue somewhere in Drupal where we're trying to use addCollection() as a shorthand for iterating over one collection and calling add() on the other for each item. We can't do that, however, because the subcollections are not flattened properly when reading back and our current dumper can't cope with that. So this change would not harm Drupal at all, and would mean I don't have fix a bug in our dumper. :-) I cannot speak for any other projects, of course.
---------------------------------------------------------------------------
by Tobion at 2012-11-27T19:06:34Z
Ok, this is ready.
This PR was merged into the master branch.
Commits
-------
acfc750#2042 initial implementation of fatal error handler
Discussion
----------
Display traces for fatal errors
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: looks like yes
Fixes the following tickets: #2042 (partly)
License of the code: MIT
Output looks like on screen http://easycaptures.com/fs/uploaded/737/1191436899.png . I've added one line to css to prevent displaying standard xdebug trace http://easycaptures.com/fs/uploaded/737/5939488074.png
---------------------------------------------------------------------------
by Koc at 2012-11-08T21:55:41Z
So, community please advice me, how can I trigger `KernelEvents::EXCEPTION` event in `ErrorHandler` or `ExceptionHandler`? Or should I provide other event for this?
---------------------------------------------------------------------------
by stof at 2012-11-08T22:03:23Z
@Koc Don't. the exception handler is there to be the safe guard when developing, and does not depend on the kernel (which would be required to trigger the event). If you were triggering the listener again, it would mean that any exception thrown in a listener would lead to a loop.
And if it is for the fatal error handling, you simply cannot be sure the kernel is still available (and even less in a wokring state) at this point.
---------------------------------------------------------------------------
by Koc at 2012-11-08T22:06:31Z
But how can I notify logger (which will send me mail or just log this situation)?
---------------------------------------------------------------------------
by fabpot at 2012-11-09T07:33:41Z
The error handler is only registered when in debug mode in the Kernel and can be triggered very early in the handling of a request (even before we have access to the dispatcher or anything else). So, the current PR looks fine to me (apart from the typo and the lack of unit tests).
---------------------------------------------------------------------------
by Koc at 2012-11-09T09:13:03Z
> The error handler is only registered when in debug mode
Ooh! I haven't see that before. But the goal - be notified about errors by email or log-file. Like now exceptions with traces from site emails to me.
---------------------------------------------------------------------------
by fabpot at 2012-11-09T09:20:54Z
I think there are two goals. The first one being to have nice pages in the development environment when a fatal error occurs. And this PR addresses that feature quite nicely. The second can be addressed in another PR.
---------------------------------------------------------------------------
by henrikbjorn at 2012-11-14T11:50:22Z
I have some questions about the ErrorHandler. Is there a reason for it only to be registered in an debug environment (which prod is not). Would assume that if i enable the ErrorHandler in productions aswell Monolog would log thoose instead of them just vanishing?
---------------------------------------------------------------------------
by Koc at 2012-11-14T12:01:50Z
I am thinking about it too. But as Fabien says it will another PR
---------------------------------------------------------------------------
by GromNaN at 2012-11-18T10:38:09Z
You should add a memory reserve to be able to handle "Out of memory" errors.
An example is here :
513d628966/lib/Raven/ErrorHandler.php (L91)513d628966/lib/Raven/ErrorHandler.php (L62)
---------------------------------------------------------------------------
by fabpot at 2012-11-28T11:35:21Z
@Koc can you finish this PR (probably by integrating the memory reserve as explained by @GromNaN)?
---------------------------------------------------------------------------
by Koc at 2012-11-28T11:46:12Z
of course, on this weekend
---------------------------------------------------------------------------
by Koc at 2012-12-02T17:44:44Z
@fabpot done
This PR was squashed before being merged into the master branch (closes#6100).
Commits
-------
0e3671b [WiP] Split urlmatcher for easier overriding
Discussion
----------
[WiP] Split urlmatcher for easier overriding
Based on discussion in https://github.com/symfony-cmf/Routing/pull/30, this PR splits the matchCollection() method of UrlMatcher into two methods. The reason is to allow Symfony CMF and Drupal to override just one of them, while leaving the actual meat of the class intact.
Additionally, it switches $routes from private to protected for the same reason: It makes it possible for us to extend the class cleanly.
Marking as WIP in case further discussion in CMF suggests other/different changes, but review and a conceptual go/no-go would be appreciated now.
---------------------------------------------------------------------------
by dbu at 2012-11-25T12:57:46Z
i think this variant really just extracts part of the logic into a separate method whithout changing any behaviour or concept. it would help a lot for the cmf to have it this way so we can extend and tweak the logic. is this now good or anybody has more input?
---------------------------------------------------------------------------
by dbu at 2012-11-27T19:42:04Z
sorry for being pushy about this one, but we need to know if this change is ok or not, if we need to improve something. if there is some problem we did not think of, we have to find different solutions for the cmf/drupal matchers.
---------------------------------------------------------------------------
by fabpot at 2012-11-28T14:29:10Z
`PhpMatcherDumper` should probably also be updated to call the new `getAttributes()` method; but that won't be possible as the Route is not accessible when using this dumper. Adding a feature that can only be used by the standard `UrlMatcher` and not by the other matchers does not sound good to me.
---------------------------------------------------------------------------
by dbu at 2012-11-28T17:18:09Z
in the context of the cmf, our problem is that we have too many routes to hold in memory. i think the dumper is not of interest for that use case - @Crell correct me please if i am wrong. if we need any caching we would need to write our own dumper probably. but currently we just extend the UrlMatcher
---------------------------------------------------------------------------
by Crell at 2012-11-30T05:47:39Z
Correct. In both the CMF case and Drupal case we have our own dumpers, because the current ones don't work for us anyway. (1000 routes and all that. :-) ) This isn't a new public method. It's just a small refactor of UrlMatcher itself to make it easier to extend. If you're using a dumped PhpMatcher, I don't know why you'd be using something like NestedMatcher in the first place.
This PR was merged into the master branch.
Commits
-------
918bad6 fix phpdoc in ExecutionContextInterface
Discussion
----------
fix phpdoc in ExecutionContextInterface
This PR was merged into the master branch.
Commits
-------
3d0c70e made ExecutionContext more consistent
Discussion
----------
[Validator] made ExecutionContext more consistent
BC break: no
The default should be an empty string instead of null because
1. a string is expected according to phpdoc
2. its more consistent with `validate($value, $groups = null, $subPath = '', $traverse = false, $deep = false)` and `validateValue($value, $constraints, $groups = null, $subPath = '')` that both have `''` as default for subPath
---------------------------------------------------------------------------
by bschussek at 2012-12-03T21:14:49Z
👍
* 2.1:
[TwigBundle] Moved the registration of the app global to the environment
needs to use simpleContent in xsd to allow empty elements
bumped Symfony version to 2.1.5-DEV
bumped Symfony version to 2.0.19-DEV
removed wrong routing xsd statement `mixed="true"`
removed unused attribute from routing.xsd
[HttpFoundation] added a small comment about the meaning of Request::hasSession() as this is a recurrent question (refs #4541)
updated VERSION for 2.1.4
updated CHANGELOG for 2.1.4
updated VERSION for 2.0.19
update CONTRIBUTORS for 2.0.19
updated CHANGELOG for 2.0.19
Conflicts:
src/Symfony/Component/HttpKernel/Kernel.php
src/Symfony/Component/Routing/Loader/schema/routing/routing-1.0.xsd
* 2.0:
[TwigBundle] Moved the registration of the app global to the environment
needs to use simpleContent in xsd to allow empty elements
bumped Symfony version to 2.0.19-DEV
removed wrong routing xsd statement `mixed="true"`
removed unused attribute from routing.xsd
updated VERSION for 2.0.19
update CONTRIBUTORS for 2.0.19
updated CHANGELOG for 2.0.19
Conflicts:
CONTRIBUTORS.md
src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
src/Symfony/Bundle/TwigBundle/TwigEngine.php
src/Symfony/Component/HttpKernel/Kernel.php
This makes the app global variable available also when accessing the Twig
environment directly instead of using the TwigEngine.
Conflicts:
src/Symfony/Bridge/Twig/CHANGELOG.md
src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml
src/Symfony/Bundle/TwigBundle/TwigEngine.php
Swapped arguments $groups and $subPath in ExecutionContext::validate and ExecutionContext::validateValue, so they match their interface signatures again.
This PR was merged into the 2.0 branch.
Commits
-------
57edf56 removed wrong routing xsd statement `mixed="true"`
Discussion
----------
removed wrong routing xsd statement `mixed="true"`
mixed="true" means that the element could contain both text and other elements, e.g.
`<requirement key="_locale">text <subelement /></requirement>`
But this wrong and such a definition would not even validate against the scheme as the xsd does not define which elements would be expected inside.
This PR was merged into the 2.0 branch.
Commits
-------
d5623b4 removed unused attribute from routing.xsd
Discussion
----------
removed unused attribute from routing.xsd
This PR was merged into the master branch.
Commits
-------
20dbe47 added annotation
c73cb8a add default for pattern for clarity
ddd8918 make id attribute required
62536e5 refactor to an xsd:group
451dcdc it should be possible to define the defaults, req. and options in any order, just like in YAML
Discussion
----------
improve routing xml scheme
bc break: no
Main points:
- the xml scheme only allowed defaults, requirements and options in this specific order. but the XmlFileLoader does not have the restriction and the YAML definions does not have such an restriction either. this is now fixed. so you can use
```
<requirement key="_locale">en</requirement>
<default key="_controller">Foo</default>
```
Before it had the be first all defaults, then all requirements, then all options.
- make id attribute required
For more changes see commits.
This PR was merged into the master branch.
Commits
-------
373be62 Bugfix for creating cookie on loginSuccess in AbstractRememberMeServices
Discussion
----------
Bugfix for creating cookie on loginSuccess in AbstractRememberMeServices
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #6055
Todo: -
License of the code: MIT
By a mistake setting of new cookies did not work for other RememberMe services than PersistentTokenBasedRememberMeServices
---------------------------------------------------------------------------
by TerjeBr at 2012-12-01T17:28:08Z
Ping. Any feedback on this?
mixed="true" means that the element could contain both text and other elements, e.g.
`<requirement key="_locale">text <subelement /></requirement>`
But this wrong and such a definition would not even validate against the scheme as the xsd does not define which elements would be expected inside.