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

Already on GitHub? Sign in to your account

Router debug refactoring #3960

Closed
wants to merge 1,969 commits into
from

Conversation

Projects
None yet
Contributor

lsmith77 commented Apr 16, 2012

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: Build Status
Fixes the following tickets: -

refactored code to use get() when outputting a single route
this is useful for a CMS, where in most cases there will be too many routes to make it feasible to load all of them. here a router implementation will be used that will return an empty collection for ->all(). with this refactoring the given routes will not be listed via router:debug, but would still be shown when using router:debug [name]

lsmith77 and others added some commits Mar 31, 2012

@lsmith77 lsmith77 updated reference to tests 925b65d
Drak [HttpFoundation] Renamed MetaBag to MetadataBag 4fc04fa
Drak [HttpFoundation] Update changelog. 8a0e6d2
@ruimarinho ruimarinho [Finder] Added sortBy options based on accessed, changed and modified…
… times
dcf82d7
@ruimarinho ruimarinho [WIP] [Locale] Fixes NumberFormatter tests failing when using ICU 4.8…
… or 4.8.1
8689e9c
@eriksencosta eriksencosta fixed CS bec231f
@eriksencosta eriksencosta updated license blocks 013f998
@eriksencosta eriksencosta fixed line break to LF cf67870
@fabpot fabpot merged branch eriksencosta/cs-fixes (PR #3750)
Commits
-------

cf67870 fixed line break to LF
013f998 updated license blocks
bec231f fixed CS

Discussion
----------

fixed CS
452e795
@fabpot fabpot merged branch lsmith77/HttpFoundation_test (PR #3744)
Commits
-------

925b65d updated reference to tests

Discussion
----------

updated reference to tests

this is basically the format used in all other components.

however i am not sure if it really makes sense to list ``phpunit -c src/Symfony/Component/HttpFoundation/``, since this relative path could be confusing for anyone using the standalone components. But even if using ``symfony/symfony`` the path is wrong relative to the location of the README.md.
e92994c
@fabpot fabpot merged branch ruimarinho/sort_by_time (PR #3745)
Commits
-------

dcf82d7 [Finder] Added sortBy options based on accessed, changed and modified times

Discussion
----------

[Finder] Added sortBy options based on accessed, changed and modified times

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
5821abb
@fabpot fabpot merged branch havvg/hotfix/propel-modelchoice-readonly-models (PR #3731)
Commits
-------

64c7183 clean file docs in Propel1 fixtures
97ba218 accept read-only models in ModelChoiceList

Discussion
----------

accept read-only models in ModelChoiceList

Ping @willdurand

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

by willdurand at 2012-03-30T08:57:14Z

👍
d6330e1
@stof stof [ClassLoader] Added a simplified PSR-0 ClassLoader
The new ClassLoader does not differentiate namespaced classes and
PEAR-like classes like the UniversalClassLoader does as the PEAR
format is a subset of PSR-0.
The new loader registers fallbacks by adding a location for an empty
prefix, as done in Composer. This allows using namespaces map generated
by Composer without any special processing on them.
09850bd
@stof stof Changed the test autoloading to use the new autoloader 4d1333f
@stof stof [ClassLoader] Added an ApcClassLoader
Unlike the ApcUniversalClassLoader, ApcClassLoader uses composition,
meaning it can be used to wrap any object providing a findFile($class)
method. Both the UniversalClassLoader and the new ClassLoader follow
this convention. It can also be used to wrap the Composer autoloader.
eae772e
@eriksencosta eriksencosta fixed CS (missing or misplaced license blocks) 2cac50d
@fabpot fabpot merged branch eriksencosta/cs-fixes (PR #3757)
Commits
-------

2cac50d fixed CS (missing or misplaced license blocks)

Discussion
----------

fixed CS (missing or misplaced license blocks)
275267f
@stof stof [TwigBridge] Added a TwigEngine in the bridge
This TwigEngine implements the interface available in the component.
the TwigBridge in TwigBundle now extends this class and provides only
the additional methods for the FrameworkBundle interface.
dbab7e1
@fabpot fabpot moved event dispatcher classes to the EventDispatcher component 93848be
@fabpot fabpot merged branch symfony/event_dispatcher_classes_move (PR #3760)
Commits
-------

93848be moved event dispatcher classes to the EventDispatcher component

Discussion
----------

moved event dispatcher classes to the EventDispatcher component

I have moved the two specialized event dispatcher classes from the FrameworkBundle bundle to the EventDispatcher component.

It makes them reusable outside the Symfony full-stack framework as they can be quite useful (people like @drak has a use for them for instance).

It makes the event dispatcher component *optionally* rely on DependencyInjection and HttpKernel for these classes, like what we have in other components.

What do you think?

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

by stof at 2012-04-02T16:02:01Z

you forgot to update the ``suggests`` section of the composer.json file. Otherwise, it is fine

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

by drak at 2012-04-02T16:06:34Z

w00t, thanks @fabpot!

I'm biased of course, but the optional dependency to `DependencyInjection` is a much more logical dependency that the previous one, to `FrameworkBundle`. It immediately shows of the DIC also.

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

by schmittjoh at 2012-04-02T16:06:39Z

+1, the "no dependencies" should really be changed to "minimal dependencies"

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

by henrikbjorn at 2012-04-02T16:09:48Z

@schmittjoh should properly be "minimal optional dependencies" otherwise people will be scared i think :) And reading "minimal dependencies" reminds me when trying to use a zf1 component standalone.

+1

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

by drak at 2012-04-02T16:12:57Z

@schmittjoh Honestly speaking, some of the Sf2 components do already have a few dependencies with each other, but where there are, they all minimal and rather logical anyway.  It's not like some libraries I could mention where one lib required about 10 deps which each rely on a bunch of other things - in the end, you need everything just for one feature :-P

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

by stof at 2012-04-02T16:14:28Z

@drak there was not optional dependency to FrameworkBundle previously as no code of the component was using FrameworkBundle (the dependency was the other way)

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

by drak at 2012-04-02T16:18:31Z

@stof, yeah but I was talking about other components like Security, DependencyInjection, HttpKernel for example which has a mix of optional and required deps.  It's not true to say all Sf2 components are 100% standalone in all cases.
bff89a4
@stof stof Updated the changelog 0e54a22
@stof stof [ClassLoader] Added a DebugClassLoader using composition f5cb167
@fabpot fabpot merged branch stof/twig_engine_move (PR #3761)
Commits
-------

dbab7e1 [TwigBridge] Added a TwigEngine in the bridge

Discussion
----------

[TwigBridge] Added a TwigEngine in the bridge

This TwigEngine implements the interface available in the component.
the TwigBridge in TwigBundle now extends this class and provides only
the additional methods for the FrameworkBundle interface.

This will allow people to support the PhpEngine and Twig in their code more easily when using the component as they don't need to reimplement the class.
I originally thought about when I helped @dragoonis to integrate the Templating component in the PPI framework 2.0 as he talked about adding the support for Twig later too.
2808acd
@stof stof Added an exception when passing an invalid object to ApcClassLoader f1f1494
@fabpot fabpot merged branch stof/autoloader_refactoring (PR #3756)
Commits
-------

f1f1494 Added an exception when passing an invalid object to ApcClassLoader
f5cb167 [ClassLoader] Added a DebugClassLoader using composition
0e54a22 Updated the changelog
eae772e [ClassLoader] Added an ApcClassLoader
4d1333f Changed the test autoloading to use the new autoloader
09850bd [ClassLoader] Added a simplified PSR-0 ClassLoader

Discussion
----------

Autoloader refactoring

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/stof/symfony.png?branch=autoloader_refactoring)](http://travis-ci.org/stof/symfony)

As discussed in #3623, I added a new ClassLoader instead of modifying the UniversalClassLoader, to be able to use the method names without BC concerns. The new class works the same than the composer autoloader regarding the handling of fallbacks, to be able to reuse namespace maps generated by composer.

```php
<?php

// autoload.php
require_once __DIR__.'/vendor/symfony/class-loader/Symfony/Component/ClassLoader/ClassLoader.php';

$loader = new Symfony\Component\ClassLoader\ClassLoader();

$map = require __DIR__.'/vendor/.composer/autoload_namespaces.php';
$loader->addPrefixes($map);

$loader->register();
```

Differences with the composer class loader:

- Composer's ``add`` method is named ``addPrefix`` in the Symfony ClassLoader
- the methods related to the class map are removed as Symfony has a separate laoder for class maps
- the ``addPrefixes`` method is added, accepting a namespace map.

I also added a new ApcClassLoader which uses composition instead of inheriting from a class loader, which makes it far more easier to reuse (we could wrap a Composer autoloader with it for instance).

```php
<?php

$composerLoader = require __DIR__.'/vendor/.composer/autoload.php';

// no need to require the file manually as Composer already registered its autoloader
$cachedLoader = new Symfony\Component\ClassLoader\ApcClassLoader('autoload.my_app', $composerLoader);

$cachedLoader->register();
// unregister the Composer autoloader as we wrapped it in the ApcClassLoader
$composerLoader->unregister();
```

TODO:

- refactor the Debug class loader to use composition too to be able to support different class loaders

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

by fabpot at 2012-04-02T16:31:28Z

Can you update the CHANGELOG and the UPGRADE file accordingly?

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

by stof at 2012-04-02T16:47:43Z

I added a note in the CHANGELOG. There is nothing to add in the UPGRADE file as the change is fully BC (I did not change the UniversalClassLoader at all so it can still be used).

I'm working on the Debug loader right now so please wait a bit before merging

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

by stof at 2012-04-02T17:12:11Z

Here is a new DebugClassLoader using composition too. this way, it is able to support the UniversalClassLoader, the ApcUniversalClassLoader (without dropping the use of APC as done previously), the new ClassLoader, the new ApcClassLoader and even the composer autoloader.
I'm not sure about the use of ``method_exists`` as it could break if an autoloader implements a protected ``findFile`` method (crappy PHP 😢) but hardcoding the supported classes would be a pain and requiring an interface would make the autoloaders more difficult to use (as the interface would need to be required first) and would drop the support of the composer autoloader.
4923483
@Tobion Tobion performance improvement in PhpMatcherDumper 56c1e31
@fabpot fabpot merged branch Tobion/PhpMatcherDumper-Improvement (PR #3763)
Commits
-------

56c1e31 performance improvement in PhpMatcherDumper

Discussion
----------

performance improvement in PhpMatcherDumper

Tests pass: yes

The code generation uses a string internally instead of an array. The array wasn't used for random access anyway.
I also removed 4 unneeded iterations this way (when imploding, when merging and twice when applying the extra indention). A `preg_replace` could also be saved under certain circumstances by moving it.
And there was a small code errror in line 139.
2620413
@fabpot fabpot merged branch drak/sessionmeta (PR #3718)
Commits
-------

8a0e6d2 [HttpFoundation] Update changelog.
4fc04fa [HttpFoundation] Renamed MetaBag to MetadataBag
2f03b31 [HttpFoundation] Added the ability to change the session cookie lifetime on migrate().
39141e8 [HttpFoundation] Add ability to force the lifetime (allows update of session cookie expiry-time)
ec3f88f [HttpFoundation] Add methods to interface
402254c [HttpFoundation] Changed meta-data responsibility to SessionStorageInterface
d9fd14f [HttpFoundation] Refactored for moved tests location.
29bd787 [HttpFoundation] Added some basic meta-data to Session

Discussion
----------

[2.1][HttpFoundation] Added some basic meta-data to Session

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
References the following tickets: #2171
Todo: -

Session data is stored as an encoded string against a single id.  If we want to store meta-data about the session, that data has to be stored as part of the session data to ensure the meta-data can persist using any session save handler.

This patch makes it much easier to determine the logic of session expiration.  In general a session expiry can be dealt with by the gc handlers, however, in some applications more specific expiry rules might be required.

Session expiry may also be more complex than a simple, session was idle for x seconds.  For example, in Zikula there are three security settings, Low, Medium and High.  The rules for session expiry are more complex as under the Medium setting, a session will expire after x minutes idle time, unless the rememberme option was ticked on login.  If so, the session will not idle.  This gives the user some control over their experience.  Under the high security setting, then there is no option, sessions will expire after the idle time is reached and login the UI has the rememberme checkbox removed.

The other advantage is that under this methodology, there can be a UI experience on expiry, like "Sorry, your session expired due to being idle for 10 minutes".

Keeping in the spirit of Symfony2 Components, I am seeking to make session handling flexible enough to accommodate these general requirements without specifically covering expiration rules. It would mean that it would be up to the implementing application to specifcally check and expire session after starting it.

Expiration might look something like this:

    $session->start();
    if (time() - $session->getMetadataBag()->getLastUpdate() > $maxIdleTime) {
        $session->invalidate();
        throw new SessionExpired();
    }

This commit also brings the ability to change the `cookie_lifetime` when migrating a session. This means one could move from a default of browser only session cookie to long-lived cookie when changing from a anonymous to a logged in user for example.

    $session->migrate($destroy, $lifetime);

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

by drak at 2012-03-30T18:18:43Z

@fabpot I have removed [WIP] status.

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

by drak at 2012-03-31T13:34:57Z

NB: This PR has been rebased and the tests relocated as per recent master changes.

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

by drak at 2012-04-03T02:16:43Z

@fabpot - ping
b9de0be
@fabpot fabpot merged branch jfsimon/master (PR #3613)
Commits
-------

2a90871 [Console] Removed previously introduced BC break.
90a2a6e [Console] Undecorated formatter must update style stack too.
bd7e01a [Console] Fixed output formatter test broken by new implementation.
a1add4b [Console] Updated output formatter to use style stack.
4f298dd [Console] Added formatter style stack.
93ffe54 [Console] Added getters to output formatter style (and its interface).
48e6b49 [Console] Updated formatter test to match styles bug fix.
ad334b6 [Console] Fixed empty style appliance.
31d5fe5 [Console] Fixed output formatter docblock.

Discussion
----------

[Console] Fixes formatter nested style appliance.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes

When outputing styled text in the console, you sometimes face to a confusing behavior: style tags cannot be nested. If tou try something like `<fg=blue>Hello <fg=red>world</fg=red>!</fg=blue>`, the trailing `!` will not be styled.

This PR introduce a new FormatterOutputStyleStack to keep open/closed styles informations up-to-date. It slightly changes OutputFormatter implementation which no longer uses `OutputFormatterStyle::apply()` method, but the new `OutputFormatterStyle::getTerminalSequence()`.

**Question:** I don't une `OutputFormatterStyleInterface` but `OutputFormatterStyle` to type `OutputFormatterStyleStack` methods arguments (to avoid BC break on the interface). Do you think it's right?

**Notice:** I also needed to fix some tests broken by new implementation.

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

by stof at 2012-03-16T10:27:56Z

Adding new methods in an interface is a BC break for people implementing it

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

by jfsimon at 2012-03-16T10:33:21Z

@stof indeed... this is a problem, should I remove them? If I do so, I should use `OutputFormatterStyle` instead of the interface to type arguments in `OutputFormatterStyleStack` right?
959158f
Peter Kokot slovenian translations updated to latest list 2519544
@fabpot fabpot merged branch maastermedia/translations (PR #3769)
Commits
-------

2519544 slovenian translations updated to latest list

Discussion
----------

Slovenian translations updated to latest list

Thank you!
e5121e9
@stloyd stloyd Polish translations sync 1456e1e
@pulzarraider pulzarraider Slovak translation update b5de295
@stof stof [Routing] Added the possibility to define options for imported resources
Closes #2772
3c32569
@fabpot fabpot merged branch pulzarraider/translation_update_sk (PR #3772)
Commits
-------

b5de295 Slovak translation update

Discussion
----------

[FrameworkBundle] Slovak translation update
21b45f2
@fabpot fabpot merged branch stloyd/patch-3 (PR #3771)
Commits
-------

1456e1e Polish translations sync

Discussion
----------

[Validators] Polish translations sync
97fd965
@fabpot fabpot Revert "merged branch jfsimon/master (PR #3613)"
This reverts commit 959158f, reversing
changes made to b9de0be.
479d808
@fabpot fabpot merged branch stof/routing_options_import (PR #3775)
Commits
-------

3c32569 [Routing] Added the possibility to define options for imported resources

Discussion
----------

[Routing] Added the possibility to define options for imported resources

Closes #2772
71c9dc3
Thomas Chmielowiec (chmielot) [#3446] [Form] Fix getChoicesForValues of EntityChoiceList on empty v…
…alues
a430f3d
@jjbohn jjbohn [FORM] Give PropertyPath ability to read hassers b6ac1aa
@stof stof [FrameworkBundle] Added the translation file for the en locale
This fixes the translation when the fallback is set to another language.
This new file can be used as reference by translators to find missing keys
in their translations as every contributor will be able to update it when
adding new keys.
a2f65f7
@fabpot fabpot merged branch stof/en_translations (PR #3782)
Commits
-------

a2f65f7 [FrameworkBundle] Added the translation file for the en locale

Discussion
----------

[FrameworkBundle] Added the translation file for the en locale

This fixes the translation when the fallback is set to another language.
This new file can be used as reference by translators to find missing keys
in their translations as every contributor will be able to update it when
adding new keys.

See the discussion on #3773
b4a44fe
Drak [EventDispatcher] More logical positions for classes. d04638a
@excelwebzone excelwebzone Hebrew translation update 1b48abb
@excelwebzone excelwebzone Fix hebrew grammar bec2f0f
marc.weistroff Updates CHANGELOG for commit f718859. f5bc208
@fabpot fabpot merged branch excelwebzone/hebrew_translations (PR #3787)
Commits
-------

bec2f0f Fix hebrew grammar
1b48abb Hebrew translation update

Discussion
----------

Hebrew translation update
404e58f
@fabpot fabpot merged branch marcw/feat-patch-changelog (PR #3788)
Commits
-------

f5bc208 Updates CHANGELOG for commit f718859.

Discussion
----------

Updates CHANGELOG for commit f718859.
fec3d29
@fabpot fabpot merged branch drak/caed (PR #3783)
Commits
-------

d04638a [EventDispatcher] More logical positions for classes.

Discussion
----------

[EventDispatcher] More logical positions for classes.

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

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

by stof at 2012-04-04T18:59:02Z

ah sorry, I looked at the patch too fast. Only the interface is moved
6bcd0a2
@lanthaler @fabpot lanthaler [Yaml] fixed phpdoc (closes symfony/yaml#3) 0ccb6fa
@Seldaek Seldaek [Validator] Allow empty keys in the validation config 8702ea5
@Seldaek Seldaek Fix typo 8ceb569
@kmohrf kmohrf [Validator] added less-strict email host verification
New checkHost attribute in email constraint will
make the validator check for only one of MX, A or AAAA
DNS resource records to verify it as a valid
email address.
f617e02
Drak Add isMethod() to Request object 3dc72cd
Drak update changelog bf33bd4
Drak [HttpFoundation] Coding standards. aec1339
Drak [HttpFoundation] Add more tests for casing 33881dd
@fabpot fabpot merged branch drak/ismethod (PR #3800)
Commits
-------

33881dd [HttpFoundation] Add more tests for casing
aec1339 [HttpFoundation] Coding standards.
bf33bd4 update changelog
3dc72cd Add isMethod() to Request object

Discussion
----------

[HttpFoundation] Add isMethod() to Request object.

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

Justification is for convenience.

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

by hhamon at 2012-04-06T07:14:44Z

+1
c392f74
@fabpot fabpot merged branch Seldaek/validator_yaml (PR #3794)
Commits
-------

8ceb569 Fix typo
8702ea5 [Validator] Allow empty keys in the validation config

Discussion
----------

[Validator] Allow empty keys in the validation config

This allows you to just list fields that don't have validation rules (yet), for future reference it's kinda helpful. Right now if they're not commented out a fatal is thrown because null isn't an array.

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

by bschussek at 2012-04-05T15:34:09Z

Could you add a test please?

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

by Seldaek at 2012-04-05T15:34:48Z

The dummy key I added in the test makes it fail without the fix.

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

by bschussek at 2012-04-05T15:46:54Z

Ah, that's perfect, I overlooked that. Thanks!
37c9fe9
@fabpot fabpot merged 2.0 b9daae2
@fabpot fabpot merged branch ruimarinho/icu-48-fix (PR #3748)
Commits
-------

8689e9c [WIP] [Locale] Fixes NumberFormatter tests failing when using ICU 4.8 or 4.8.1

Discussion
----------

[WIP] [Locale] Fixes NumberFormatter tests failing when using ICU 4.8 or 4.8.1.1

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: no
Todo: fix DateFormatter

The ICU CLDR 2.0 data has been updated on ICU 4.8 and the same data set is used on version 4.8.1.1. The problem is related to [this commit](http://bugs.icu-project.org/trac/changeset?reponame=&new=31307%40icu%2Ftrunk%2Fsource%2Fdata%2Fcurr%2Fen.txt&old=31074%40icu%2Ftrunk%2Fsource%2Fdata%2Fcurr%2Fen.txt) which has since been updated with new data and subsequently shipped with version 49.

The `DateFormatter` tests are still failing - see this [gist](https://gist.github.com/2004d40e5167286028ea). Suggestions are welcomed on how to handle this part.

Test results with PHP 5.4.0 with ICU 4.8.1.1 on OSX:

````
FAILURES!
Tests: 5917, Assertions: 12749, Failures: 26, Incomplete: 11, Skipped: 47.
```

with this WIP patch:

```
FAILURES!
Tests: 5917, Assertions: 12749, Failures: 13, Incomplete: 11, Skipped: 47.
```
245a7b7
@fabpot fabpot moved a fixture file 85535de
@fabpot fabpot merged branch jjbohn/feature/property-path-hasser (PR #3549)
Commits
-------

b6ac1aa [FORM] Give PropertyPath ability to read hassers

Discussion
----------

[Form] Give PropertyPath ability to read hassers

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

Using a `hasser` instead of `isser` for Boolean values is pretty common. I've found myself using `issers` a handful of times just to make an interface play nice with the form component, but the code reads funny now. I don't think we should be accounting for every possible `getter` variation, but I think this one is common enough that it warrants a discussion.

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

by fabpot at 2012-03-11T08:25:31Z

I tend to agree with with. What do you think @bschussek?

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

by kriswallsmith at 2012-03-16T22:42:28Z

I'm not so sure. There are lots of reasons to write a *hasser* that accepts an argument (i.e. `User::hasRole($role)`). Doesn't seem as clean as *issers* and *getters*.

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

by vicb at 2012-03-16T22:49:14Z

> There are lots of reasons to write a hasser that accepts an argument

May be can check for 0 args as we are already using reflexion ?

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

by kriswallsmith at 2012-03-16T22:55:43Z

In that case we should check that there are either 0 arguments or only optional arguments and also consider adding the same logic to the other varieties.

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

by jjbohn at 2012-03-16T23:37:47Z

Passing arguments seems like a pretty big departure for PropertyPath. How would you annotate that? I'm not sure I see a common use case for needing arguments when mapping data to and from forms.

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

by stof at 2012-03-16T23:50:22Z

@jjbohn it is not about passing arguments but about using the hasser only if it does not have required arguments

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

by jjbohn at 2012-03-17T01:54:18Z

Ah. I see. I have a tendency to read @kriswallsmith comments wrong :D. I could see that but iirc, there's not any current check like this on the other accessors. Happy to add it though if there's a consensus.

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

by fabpot at 2012-03-17T11:24:34Z

What's the point is checking the hasser/getter/isser arguments. It's up to the developer to check if he can use them or not. Let's not complexify the code for this.

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

by kriswallsmith at 2012-03-17T15:37:39Z

My concern is that someone writes a hasser method on their model that is not intended for use with the form component but it's called anyway, leading to WTFs.

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

by stof at 2012-04-03T22:28:21Z

@fabpot what's your decision about this ?

@jjbohn you need to rebase your PR. It conflicts with master as tests have been moved

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

by bschussek at 2012-04-05T14:53:55Z

@kriswallsmith is right. The check for 1 === $method->getNumberOfRequiredParameters() can (and should) easily be added to all of the if-clauses here.

Apart from that, I'm okay with adding this.
20a9961
@jakzal jakzal [Filesystem] Added missing docblock comment. 1c833e7
@jakzal jakzal [Filesystem] Added unit tests for copy method. 6ac5486
@webmozart webmozart Merge remote branch 'umpirsky/collection-name' into issue3738 fc342d1
@webmozart webmozart [Form] Fixed label of prototype in CollectionType 6e0b03a
@webmozart webmozart [Form] Improved labels generated by default from form names 6584721
@jakzal jakzal [Filesystem] Added unit tests for mkdir method. 7e297db
@jakzal jakzal [Filesystem] Added unit tests for touch method. a91e200
@jmikola jmikola [DependencyInjection] Fix Yaml file loader test
This broke when 2.0 was recently merged into master with b9daae2, as the Yaml fixture change from 24a0d0a was not included.
1c3e4ac
@kimhemsoe kimhemsoe Added XCache class loader 7e66908
@jakzal jakzal [Filesystem] Introduced workspace directory to limit complexity of te…
…sts.
8e861b7
@jakzal jakzal [Filesystem] Added unit tests for remove method. bba0080
@jakzal jakzal [Filesystem] Added unit tests for chmod method. 8071859
@jakzal jakzal [Filesystem] Added unit tests for rename method. a041feb
@kimhemsoe kimhemsoe Updated to new cache loader pattern. b74a5d4
@jakzal jakzal [Filesystem] Added unit tests for symlink method. 21860cb
@jakzal jakzal [Filesystem] Added unit tests for makePathRelative method. 2ee4b88
@jakzal jakzal [Filesystem] Added unit tests for isAbsolutePath method. 8c94069
@jakzal jakzal [Filesystem] Added unit tests for mirror method. 11a676d
@jakzal jakzal [Filesystem] Fixed a bug in remove being unable to remove symlinks to…
… unexisting file or directory.
d4243a2
@jakzal jakzal [Filesystem] Fixed typos in the docblocks. f5f5c21
@eriksencosta eriksencosta added composer.lock to .gitignore 46c7ae1
@eriksencosta eriksencosta [Propel1Bridge][TwigBridge] fixed export instructions in the README f…
…iles
f3efea3
@eriksencosta eriksencosta [TwigBundle] fixed CS f5f0506
@fabpot fabpot merged branch jmikola/di-yaml-loader-test (PR #3808)
Commits
-------

1c3e4ac [DependencyInjection] Fix Yaml file loader test

Discussion
----------

[DependencyInjection] Fix Yaml file loader test

This broke when 2.0 was recently merged into master with b9daae2, as the Yaml fixture change from 24a0d0a was not included.

https://twitter.com/webmozart/status/188325579894947840
987e69e
@fabpot fabpot merged branch eriksencosta/readme-fixes (PR #3814)
Commits
-------

f3efea3 [Propel1Bridge][TwigBridge] fixed export instructions in the README files

Discussion
----------

[Propel1Bridge][TwigBridge] fixed export instructions in the README files
1c10b0b
@fabpot fabpot merged branch eriksencosta/patch-1 (PR #3813)
Commits
-------

46c7ae1 added composer.lock to .gitignore

Discussion
----------

added composer.lock to .gitignore
048f8da
@fabpot fabpot merged branch bschussek/issue3738 (PR #3807)
Commits
-------

6584721 [Form] Improved labels generated by default from form names
6e0b03a [Form] Fixed label of prototype in CollectionType
fc342d1 Merge remote branch 'umpirsky/collection-name' into issue3738
f91660d Added test for prototype label.

Discussion
----------

[Form] Fixed default label generated for the CollectionType prototype

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3738, #3739
Todo: -

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

(the fact that the build fails seems to origin from the broken master)
526fb7b
@fabpot fabpot merged branch chmielot/3446-empty-multiple-entity-choice (PR #3734)
Commits
-------

a430f3d [#3446] [Form] Fix getChoicesForValues of EntityChoiceList on empty values

Discussion
----------

[Form] Fix reverseTransform on multiple entity form type

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3446, #3727
Todo: -

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

by stof at 2012-04-03T23:05:55Z

@bschussek ping

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

by stof at 2012-04-03T23:06:45Z

This is an alternate implementation for #3727

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

by chmielot at 2012-04-04T13:47:27Z

OK, this is another possibility to fix this issue with working tests. What do you think about this?

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

by chmielot at 2012-04-04T13:51:27Z

OK, just done.

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

by stof at 2012-04-04T13:51:39Z

@beberlei @bschussek ping

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

by bschussek at 2012-04-06T18:50:37Z

@fabpot 👍
d967d39
@eriksencosta eriksencosta Merge branch 'master' into cs-fixes abd59c3
@fabpot fabpot merged branch eriksencosta/cs-fixes (PR #3816)
Commits
-------

abd59c3 Merge branch 'master' into cs-fixes
f5f0506 [TwigBundle] fixed CS

Discussion
----------

CS fixes
0a585b7
@fabpot fabpot fixed CS 72e854e
@jakzal jakzal [Filesystem] Fixed warnings in makePathRelative(). 100e97e
@eriksencosta eriksencosta [FrameworkBundle] synced pt_BR translations dc94e27
@eriksencosta eriksencosta [FrameworkBundle] fixed pt_BR translations (added commas to take a br…
…eath :)
bf51a58
@fabpot fabpot merged branch eriksencosta/pt_BR-translations (PR #3817)
Commits
-------

bf51a58 [FrameworkBundle] fixed pt_BR translations (added commas to take a breath :)
dc94e27 [FrameworkBundle] synced pt_BR translations

Discussion
----------

synced and fixed pt_BR translations
e7dbc38
@fabpot fabpot merged branch jakzal/FilesystemTests (PR #3811)
Commits
-------

100e97e [Filesystem] Fixed warnings in makePathRelative().
f5f5c21 [Filesystem] Fixed typos in the docblocks.
d4243a2 [Filesystem] Fixed a bug in remove being unable to remove symlinks to unexisting file or directory.
11a676d [Filesystem] Added unit tests for mirror method.
8c94069 [Filesystem] Added unit tests for isAbsolutePath method.
2ee4b88 [Filesystem] Added unit tests for makePathRelative method.
21860cb [Filesystem] Added unit tests for symlink method.
a041feb [Filesystem] Added unit tests for rename method.
8071859 [Filesystem] Added unit tests for chmod method.
bba0080 [Filesystem] Added unit tests for remove method.
8e861b7 [Filesystem] Introduced workspace directory to limit complexity of tests.
a91e200 [Filesystem] Added unit tests for touch method.
7e297db [Filesystem] Added unit tests for mkdir method.
6ac5486 [Filesystem] Added unit tests for copy method.
1c833e7 [Filesystem] Added missing docblock comment.

Discussion
----------

[Filesystem] Fixed a bug in remove() being unable to unlink broken symlinks

While working on test coverage for Filesystem class I discovered a bug in remove() method.

Before removing a file a check is made if it exists:

    if (!file_exists($file)) {
        continue;
    }

Problem is [file_exists()](http://php.net/file_exists) returns false if link's target file doesn't exist. Therefore remove() will fail to delete a directory containing a broken link. Solution is to handle links a bit different:

    if (!file_exists($file) && !is_link($file)) {
        continue;
    }

Additionally, this PR improves test coverage of Filesystem component.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes

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

by cordoval at 2012-04-07T00:55:59Z

✌.|•͡˘‿•͡˘|.✌

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

by fabpot at 2012-04-07T06:12:34Z

Tests do not pass for me:

    PHPUnit 3.6.10 by Sebastian Bergmann.

    Configuration read from /Users/fabien/work/symfony/git/symfony/phpunit.xml.dist

    .........................EE.......

    Time: 0 seconds, Memory: 5.25Mb

    There were 2 errors:

    1) Symfony\Component\Filesystem\Tests\FilesystemTest::testMakePathRelative with data set #0 ('/var/lib/symfony/src/Symfony/', '/var/lib/symfony/src/Symfony/Component', '../')
    Uninitialized string offset: 29

    .../rc/Symfony/Component/Filesystem/Filesystem.php:183
    .../rc/Symfony/Component/Filesystem/Tests/FilesystemTest.php:434

    2) Symfony\Component\Filesystem\Tests\FilesystemTest::testMakePathRelative with data set #1 ('var/lib/symfony/', 'var/lib/symfony/src/Symfony/Component', '../../../')
    Uninitialized string offset: 16

    .../rc/Symfony/Component/Filesystem/Filesystem.php:183
    .../rc/Symfony/Component/Filesystem/Tests/FilesystemTest.php:434

    FAILURES!
    Tests: 34, Assertions: 67, Errors: 2.

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

by jakzal at 2012-04-07T07:26:15Z

Sorry for this. For some reason my PHP error reporting level was to low to catch this...

Should be fixed now but I needed to modify the makePathRelative() (this bug existed before).
13aa515
marc.weistroff Adds a linter command for templates 1d7e9d9

fabpot and others added some commits Apr 11, 2012

@fabpot fabpot [Console] fixed typo 9d5743b
@fabpot fabpot merged 2.0 c7ba1b9
@fabpot fabpot merged branch hhamon/command_description_fixes (PR #3871)
Commits
-------

b4f0a04 [TwigBundle] fixed twig:lint command description.
809933f [FrameworkBundle] fixed translation:update command description.

Discussion
----------

Command description fixes

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
e9539d8
@fabpot fabpot merged branch Tobion/patch-1 (PR #3870)
Commits
-------

810b2e1 fix formatting of changelog-2.1.md

Discussion
----------

fix formatting of changelog-2.1.md

Fix indention for correct display. Also a few language fixes.
5e81389
@fabpot fabpot merged branch bschussek/issue3635 (PR #3820)
Commits
-------

65aa387 [Form] Fixed index generation in EntityChoiceList if ID is not an integer

Discussion
----------

[Form] Fixed index generation in EntityChoiceList if ID is not an integer

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3635
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3635)
191aaa3
@fabpot fabpot merged branch kimhemsoe/xcache_classloader (PR #3809)
Commits
-------

c36651b Fixed spelling error
f123684 Removed leftover from c/p
b74a5d4 Updated to new cache loader pattern.
7e66908 Added XCache class loader

Discussion
----------

[ClassLoader] Added XCache class loader

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

There is no tests, as it seems there is no way to use xcache storage functions from CLI.

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

by stof at 2012-04-06T20:12:09Z

Please implement a XcacheClassLoader following the same pattern than the new ApcClassLoader instead

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

by cordoval at 2012-04-07T14:20:47Z

- should include tests
- should include documentation (will you also update the component documentation for this new class?)

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

by stof at 2012-04-07T14:25:00Z

@cordoval the PR explains why there is no tests: xcache canot be used in the CLI.

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

by cordoval at 2012-04-07T14:26:43Z

ok @stof sorry it said it seemed not to be possible, i thought it was possible but I am wrong.

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

by kimhemsoe at 2012-04-07T15:01:24Z

@cordoval My english is really horrible. I would not mind if someone else could do that task for me. We also need to add doc for the new ApcClassLoader.

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

by cordoval at 2012-04-07T15:03:57Z

I wish you can explain me more then about this class and how to use it in code so then I can write easily the documentation :D deal?

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

by kimhemsoe at 2012-04-07T15:21:25Z

Deal :P

The XcacheClassLoader and ApcClassLoader replaces the old ApcUniversalClassLoader.
They giving us support for using another loader then UniversalClassLoader, without duplicating the cache layer.
Aslong it have a public function findFile($class) method.

 $loader = new ClassLoader();

// register classes with namespaces
$loader->add('Symfony\Component', __DIR__.'/component');
$loader->add('Symfony', __DIR__.'/framework');

$cachedLoader = new XcacheClassLoader('my_prefix', $loader);

// activate the cached autoloader
$cachedLoader->register();

Think that is more or less the essence of this.

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

by cordoval at 2012-04-09T08:28:53Z

it is not add but registerNamespace right?

so the main idea is to get rid of the restriction to use Apc with Universal loader

what is the comparative advantage between APC and Xcache?

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

by kimhemsoe at 2012-04-09T08:55:23Z

Yes if the $loader (class finder) were to be a instance UniversalClassLoader.

Yes the main idea is to be able to reuse the cache layer with any class loader there obey to the one restriction.

Difference between apc and xcache and why to use what is coming down to taste and your setup. We use xcache as APC have some issues in fastcgi setups. when we upgrade to php54 at somepoint we get to chance to move to php-fpm wich solves these issues. Short story: Slightly out of scope for any documentation in here :-P
22d8a43
@fabpot fabpot updated CHANGELOG ff9b731
@fabpot fabpot merged branch tvlooy/GetSetMethodNormalizer (PR #3582)
Commits
-------

039ff6f allow more control on GetSetMethodNormalizer by using callback functions and an ignoreAttributes list

Discussion
----------

allow more control on GetSetMethodNormalizer

Here is an other attempt. You would use this as follows:

    $serializer = new \Symfony\Component\Serializer\Serializer(
        array(new \Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer()),
        array('json' => new \Symfony\Component\Serializer\Encoder\JsonEncoder())
    );

    $callbacks = array('books' => function ($books) { return NULL; });

    return new Response(
        $serializer->serialize($paginator->getRows(), 'json', $callbacks),
        200,
        array('Content-Type' => 'application/json')
    );

Besides of returning NULL, you could also do things like:

    $callbacks = array(
        'books' => function ($books) {
            $ids = array();
            foreach ($books as $book) {
                $ids[] = $book->getId();
            }
            return $ids;
        },
        'author' => function ($author) {
            return $author->getId();
        },
        'creationDate' => function ($creationDate) {
            return $creationDate->format('d/m/Y');
        },
    );

The commit is not complete yet. But at this point I am interested in your opinions.

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

by lsmith77 at 2012-03-12T22:53:18Z

in general i agree that using a callback is a good solution to provide more power without complicating the API or implementation in this case.

please add a test case, this should also help illustrate how this can be used in practice.

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

by schmittjoh at 2012-03-13T04:54:33Z

Note that your change breaks the API defined by the interface, i.e. someone using this method needs to type-hint the serializer implementation, not the interface.

It also adds a parameter to the public API of the serializer which will only work with one specific normalizer. What if another normalizer needs additional information, should another parameter be added to the serialize method? What about deserialization?

Bottom line is, the serializer component was simply not designed for this kind of thing. I've tried to make it more flexible before creating the bundle, but some things simply cannot be fixed in a sane way.

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

by tvlooy at 2012-03-13T06:07:45Z

Would just adding a setCallbacks() to the GetSetMethodNormalizer be a better solution? That doesn't touch the API. I will try to write some tests this evening.

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

by schmittjoh at 2012-03-13T16:22:50Z

That would definitely be better.

You would then need to retrieve the normalizer instance before calling ``serialize`` on the serializer which also leaves a stale taste, but I have no other solution for now.

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

by tvlooy at 2012-03-13T21:32:26Z

So, this should be it then. Yet an other usage example:

    $normalizer = new \Symfony\Component\Serializer\Normalizer\GetSetMethodNormalizer();
    $normalizer->setCallbacks(
        array(
            'books' => function ($books) {
                $ids = array();
                foreach ($books as $book) {
                    $ids[] = $book->getId();
                }
                return $ids;
            },
        )
    );
    $serializer = new \Symfony\Component\Serializer\Serializer(
        array($normalizer),
        array('json' => new \Symfony\Component\Serializer\Encoder\JsonEncoder())
    );

    return new Response(
        $serializer->serialize($paginator->getRows(), 'json'),
        200,
        array('Content-Type' => 'application/json')
    );

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

by tvlooy at 2012-03-18T21:16:48Z

Anything else needed for this to get pulled in?

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

by tvlooy at 2012-03-19T18:33:58Z

Hm, I like to keep it that way because I like the fact that not passing a callable will result in a warning instead of silently skipping it. You don't get that behaviour by treating it as null.

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

by vicb at 2012-03-19T23:15:37Z

I was unclear: the code should throw an exception when an element is not callable, this is why `null` will not be supported any more (it is not a callback as the `setCallbacks` indicate).

They are several way to support the former behavior:

* the cb can return a defined interface,
* the cb can throw a defines exc,
* by adding a `setIgnoredAttributes` method

Please also squash your commits.

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

by tvlooy at 2012-03-20T21:02:06Z

Yes, I like the setIgnoredAttributes solution. I changed it and squashed the commits.

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

by tvlooy at 2012-03-26T20:07:36Z

some improvements and squashed the commits

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

by stof at 2012-04-03T22:36:15Z

@tvlooy Please rebase your branch. It conflicts with master because of the move of tests.

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

by tvlooy at 2012-04-04T07:43:47Z

@stof I will do it on saturday, if that is ok with you.

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

by fabpot at 2012-04-10T18:29:30Z

Is it mergeable now? ping @Seldaek, @schmittjoh.

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

by tvlooy at 2012-04-10T18:55:04Z

yes, it should be
3c3ec5c
@richardmiller richardmiller Added info on changes to setting validation subpath to UPGRADE-2.1.md 60d9aff
@richardmiller richardmiller Removed duplicated content 6433ca0
@fabpot fabpot merged branch richardmiller/validation_path_upgrade (PR #3874)
Commits
-------

6433ca0 Removed duplicated content
60d9aff Added info on changes to setting validation subpath to UPGRADE-2.1.md

Discussion
----------

Added info on changes to setting validation subpath to UPGRADE-2.1.md

As per GH-3424

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

by stof at 2012-04-11T11:05:00Z

you duplicated it

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

by richardmiller at 2012-04-11T11:07:17Z

I've closed the other one - there was a mess with the commits and an unnecessary merge.

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

by stof at 2012-04-11T11:30:00Z

@richardmiller I'm talking about the content

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

by richardmiller at 2012-04-11T11:35:40Z

@stof, ah sorry misunderstood, one of those days it seems :)

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

by stof at 2012-04-11T11:42:21Z

I should have commented inline to make it more clear
3d6575e
@fabpot fabpot merged branch kmohrf/ticket_2827_validator_mail_A_RR_DNS_hostcheck (PR
…#3799)

Commits
-------

f617e02 [Validator] added less-strict email host verification

Discussion
----------

[Validator] added less-strict email host verification

uhhhhh, my first pull request :>. uhm... tell me if i did something wrong :)

#### Request info
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes... well,not really (i guess master branch is not passing - at least i didnt broke the email test)
Fixes the following tickets: #2827

#### Description
New checkHost attribute in email constraint will make the validator check for only one of MX, A or AAAA DNS resource records to verify it as a valid email address.
3469713
@Tobion @vicb Tobion [Routing] Implement bug fixes and enhancements 9307f5b
@fabpot fabpot merged branch vicb/routing_improvements (PR #3876)
Commits
-------

9307f5b [Routing] Implement bug fixes and enhancements

Discussion
----------

[Routing] Implement bug fixes and enhancements (from @Tobion)

This is mainly #3754 with some minor formatting changes.

Original PR message from @Tobion:

Here a list of what is fixed. Tests pass.

1. The `RouteCollection` states

    > it overrides existing routes with the same name defined in the instance or its children and parents.

    But this is not true for `->addCollection()` but only for `->add()`. addCollection does not remove routes with the same name in its parents (only in its children). I don't think this is on purpose.
    So I fixed it by making sure there can only be one route per name in all connected collections. This way we can also simplify `->get()` and `->remove()` and improve performance since we can stop iterating recursively when we found the first and only route with a given name.
    See `testUniqueRouteWithGivenName` that fails in the old code but works now.

2. There was an bug with `$collection->addPrefix('0');` that didn't apply the starting slash. Fixed and test case added.

3. There is an issue with `->get()` that I also think is not intended. Currently it allows to access a sub-RouteCollection by specifing $name as array integer index. But according to the PHPdoc you should only be allowed to receive a Route instance and not a RouteCollection.
    See `testGet` that has a failing test case. I fixed this behavior.

4. Then I recognized that `->addCollection` depended on the order of applying them. So

        $collection1->addCollection($collection2, '/b');
        $collection2->addCollection($collection3, '/c');
        $rootCollection->addCollection($collection1, '/a');

    had a different pattern result from

        $collection2->addCollection($collection3, '/c');
        $collection1->addCollection($collection2, '/b');
        $rootCollection->addCollection($collection1, '/a');

    Fixed and test case added. See `testPatternDoesNotChangeWhenDefinitionOrderChanges`.

5. PHP could have ended in an infinite loop when one tried to add an existing RouteCollection to the tree. Fixed by throwing an exception when this situation is detected. See tests `testCannotSelfJoinCollection` and `testCannotAddExistingCollectionToTree`.

6. I made `setParent()` private because its not useful outside the class itself. And `remove()` also removes the route from its parents. Added public `getRoot()` method.

7. The `Route` class throwed a PHP warning when trying to set an empty requirement.

8. Fixed issue #3777. See discussion there for more info. I fixed it by removing the over-optimization that was introduced in 91f4097 but didn't work properly. One cannot reorder the route definitions, as is was done, because then the wrong route might me matched before the correct one. If one really wanted to do that, it would require to calculate the intersection of two regular expressions to determine if they can be grouped together (a tool that would also be useful to check whether a route is unreachable, see discussion in #3678) We can only safely optimize routes with a static prefix within a RouteCollection, not across multiple RouteCollections with different parents.  This is especially true when using variables and regular expressions requirements.
I could however apply an optimization that was missing yet: Collections with a single route were missing the static prefix optimization with `0 === strpos()`.

9. Fixed an issue where the `PhpMatcherDumper` would not apply the optimization if the root collection to be dumped has a prefix itself. For this I had to rewrite `compileRoutes`. It is also much easier to understand now. Addionally I added many comments and PHPdoc because complex recursive methods like this are still hard to grasp.
I added a test case for this (`url_matcher3.php`).

10. Fix that `Route::compile` needs to recompile a route once it is modified. Otherwise we have a wrong result. Test case added.
0c57db1
@richardmiller richardmiller Added some bc breaks from the changelog into UPGRADE-2.1.md f23b4fa
@fabpot fabpot merged branch jakzal/FilesystemMirrorCleanup (PR #3844)
Commits
-------

efad5d5 [Filesystem] Prevented infiite loop on windows while calling mirror on symlink. Added test for mirroring symlinks.

Discussion
----------

[Filesystem] Prevented infinite loop on windows while mirrorring symlinks

First check for filetype in *mirror()* method is:

    if (is_link($file)) {
        $this->symlink($file, $target);

later we see:

    } elseif (is_file($file) || ($copyOnWindows && is_link($file))) {
        $this->copy($file, $target, isset($options['override']) ? $options['override'] : false);

The later check for links on windows (*$copyOnWindows && is_link($file)*) won't ever get called. Calling *symlink()* in *mirror()* on windows would lead to calling *mirror()* again.

Note that I didn't actually try running it on windows platform. I added a test for mirroring symlinks (non-windows test). I think it'd be good if someone added some windows specific tests to this class.

I also modified the target path:

    $target = $targetDir.'/'.str_replace($originDir.DIRECTORY_SEPARATOR, '', $file->getPathname());

It didn't use DIRECTORY_SEPARATOR and is equivalent to:

    $target = str_replace($originDir, $targetDir, $file->getPathname());

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
70d49c3
@fabpot fabpot merged branch hason/validator (PR #552)
Commits
-------

f9a486e [Validator] Added support for pluralization of the SizeLengthValidator
c0715f1 [FrameworkBundle], [TwigBundle] added support for form error message pluralization
7a6376e [Form] added support for error message pluralization
345981f [Validator] added support for plural messages

Discussion
----------

[Validator] Added support for plural error messages

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Todo: create translations for en and update others (FrameworkBundle)

[![Build Status](https://secure.travis-ci.org/hason/symfony.png?branch=validator)](http://travis-ci.org/hason/symfony)

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

by fabpot at 2011-05-14T20:41:01Z

@bschussek: What's your opinion?

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

by stof at 2011-09-04T13:14:29Z

@hason could you rebase your branch on top of master and update the PR ?

You also need to change the messages in the constraint that uses the pluralization to a pluralized format.

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

by stof at 2011-10-16T18:06:22Z

@hason ping

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

by stof at 2011-11-11T14:58:19Z

@hason ping again

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

by stof at 2011-12-12T20:39:10Z

@hason ping again. Can you update your PR ?

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

by hason at 2011-12-12T21:29:14Z

@stof I hope that I will update PR this week.

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

by bschussek at 2012-01-15T19:07:32Z

Looks good to me.

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

by canni at 2012-02-02T17:28:54Z

@hason can you update this PR and squash commits, it conflicts with current master

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

by hason at 2012-02-09T07:21:41Z

@stof, @canni Rebased.

What is the best solution for the translation of messages?

1. Change messages in the classes and all xliff files?
2. Keep messages in the classes and change all xliff files?

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

by stof at 2012-02-09T08:19:41Z

The constraints contain the en message so you will need to modify them to update the message

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

by hason at 2012-02-09T08:55:55Z

I prefer second option. The Validator component should be decoupled from the Translation component. The constraints contain the en message which is also the key for Translation component. We should create validators.en.xlf in the FrameworkBundle for en message. I think that this is better solution. What do you think?

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

by stof at 2012-04-04T02:22:02Z

@hason Please rebase your branch. It conflicts with master because of the move of the tests

@fabpot ping
cc833b1
@fabpot fabpot merged branch richardmiller/bc_break_changes_in_upgrade (PR #3877)
Commits
-------

f23b4fa Added some bc breaks from the changelog into UPGRADE-2.1.md

Discussion
----------

Added some bc breaks from the changelog into UPGRADE-2.1.md
d167fd4
@jmikola @webmozart jmikola [Form] Failing test for empty_data option BC break
This demonstrates the issue described in symfony/symfony#3354. FieldType no longer has access to the child type's data_class option, which makes it unable to create the default closure for empty_data.
cb87ccb
@webmozart webmozart [Form] Fixed option support in Form component b733045
@webmozart webmozart [Form] Fixed typos 5adec19
Contributor

jalliot commented on UPGRADE-2.1.md in d167fd4 Apr 11, 2012

typo: "you will need" instead of "you will you need"

Contributor

jalliot commented on UPGRADE-2.1.md in d167fd4 Apr 11, 2012

typo: created

webmozart and others added some commits Apr 11, 2012

@webmozart webmozart [Form] Moved calculation of ChoiceType options to closures 8329087
@fabpot fabpot merged branch bschussek/issue3354 (PR #3789)
Commits
-------

8329087 [Form] Moved calculation of ChoiceType options to closures
5adec19 [Form] Fixed typos
cb87ccb [Form] Failing test for empty_data option BC break
b733045 [Form] Fixed option support in Form component

Discussion
----------

[Form] Fixed option support in Form component

Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #3354, #3512, #3685, #3694
Todo: -

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

This PR also introduces a new helper `DefaultOptions` for solving option graphs. It accepts default options to be defined on various layers of your class hierarchy. These options can then be merged with the options passed by the user. This is called *resolving*.

The important feature of this utility is that it lets you define *lazy options*. Lazy options are specified using closures that are evaluated when resolving and thus have access to the resolved values of other (potentially lazy) options. The class detects cyclic option dependencies and fails with an exception in this case.

For more information, check the inline documentation of the `DefaultOptions` class and the UPGRADE file.

@fabpot: Might this be worth a separate component? (in total the utility consists of five classes with two associated tests)

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

by beberlei at 2012-04-05T08:54:10Z

"The important feature of this utility is that it lets you define lazy options. Lazy options are specified using closures"

What about options that are closures? are those differentiated?

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

by bschussek at 2012-04-05T08:57:35Z

@beberlei Yes. Closures for lazy options receive a Symfony\Component\Form\Options instance as first argument. All other closures are interpreted as normal values.

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

by stof at 2012-04-05T11:09:49Z

I'm wondering if these classes should go in the Config component. My issue with it is that it would add a required dependency to the Config component and that the Config component mixes many different things in it already (the loader part, the resource part, the definition part...)

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

by sstok at 2012-04-06T13:36:36Z

Sharing the Options class would be great, and its more then one class so why not give it its own Component folder?
Filesystem is just one class, and that has its own folder.

Great job on the class bschussek 👏

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

by bschussek at 2012-04-10T12:32:34Z

@fabpot Any input?

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

by bschussek at 2012-04-10T13:54:13Z

@fabpot Apart from the decision about the final location of DefaultOptions et al., could you merge this soon? This would make my work a bit easier since this one is a blocker.

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

by fabpot at 2012-04-10T18:08:18Z

@bschussek: Can you rebase on master? I will merge afterwards. Thanks.
05842c5
@willdurand willdurand [Form] Fixed documentation, and the DateType (default options) 779d3bb
@richardmiller richardmiller Added some more bc breaks to UPGRADE-2.1.md from the changelog 7276a04
@Tobion Tobion [Routing] simplified regex with named variables f666836
@richardmiller richardmiller A few changes from proofreading CHANGELOG-2.1md 3686799
@fabpot fabpot merged branch Tobion/named-variables (PR #3884)
Commits
-------

f666836 [Routing] simplified regex with named variables

Discussion
----------

[Routing] simplified regex with named variables

Test pass: yes
BC break: no

Since PHP 5.2.2 subpatterns in regex can be simplified from `?P<name>` to `?<name>` (see http://www.php.net/manual/en/regexp.reference.subpatterns.php).
This enhances readability.
bc0c7ba
@fabpot fabpot merged branch richardmiller/bc_break_changes_in_upgrade (PR #3883)
Commits
-------

7276a04 Added some more bc breaks to UPGRADE-2.1.md from the changelog

Discussion
----------

Added some more bc breaks to UPGRADE-2.1.md from the changelog
7657a44
@fabpot fabpot merged branch richardmiller/fixing_typos_in_changelog (PR #3885)
Commits
-------

3686799 A few changes from proofreading CHANGELOG-2.1md

Discussion
----------

A few changes from proofreading CHANGELOG-2.1md
1db7ba4
@fabpot fabpot [HttpFoundation] added missing variable declaration 61bec64
@willdurand willdurand [Form] Fixed DateType default options 6f56dfc
@willdurand willdurand [Form] Added all() method to the FormBuilder class
In order to perform some introspection on a FormBuilder instance,
we need to be able to get its children. Almost everything is accessible
in this class, but the children are not.

This PR adds a all() method to respect the current API (get(), remove(),
...).
4120f13
@fabpot fabpot merged branch willdurand/fix-form-type-doc (PR #3880)
Commits
-------

6f56dfc [Form] Fixed DateType default options
779d3bb [Form] Fixed documentation, and the DateType (default options)

Discussion
----------

[Form] Fixed documentation, and the DateType (default options)

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

by fabpot at 2012-04-11T16:48:04Z

That breaks the tests.

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

by willdurand at 2012-04-11T16:50:35Z

I got an error with the Form test suite before to write this patch..

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

by willdurand at 2012-04-11T16:53:30Z

Nevermind, I can see broken tests.. I'm on, sorry

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

by willdurand at 2012-04-11T16:57:52Z

@fabpot fixed.

```
OK, but incomplete or skipped tests!
Tests: 945, Assertions: 1439, Incomplete: 11.
```
570bb6a
@aubx aubx Updated croatian validator translation and deleted validator xliff file 771ab45
@willdurand willdurand [Form] [Tests] Used assertCount() be2456b
@fabpot fabpot merged branch aubx/croatian_validator_translation_update (PR #3887)
Commits
-------

771ab45 Updated croatian validator translation and deleted validator xliff file

Discussion
----------

[FrameworkBundle][translations]Updated croatian validator translation and deleted validator xliff file

Updated croatian validator messages.

Also, "validators.hr.xliff" file was in translations folder so I deleted it (how did it get there? It should be in the 2.0 branch only).
b47cb35
@kimhemsoe kimhemsoe [process] Added destructor to process to make sure handles are always…
… closed in the right order.
89a5c1a
@jalliot jalliot [Security][ACL] Fixed ObjectIdentity::fromDomainObject and UserSecuri…
…tyIdentity::from(Account|Token) when working with proxies

Backported ClassUtils class from Doctrine Common 2.2
Fixes #2611, #2056, #2048, #2035
6483d88
@Tobion Tobion [Routing] remove duplicated cache of compiled routes 03d30fd
@Tobion Tobion [Routing] small optimization of PhpGeneratorDumper 27a05f4
@fabpot fabpot merged branch jalliot/acl_proxies-2 (PR #3826)
Commits
-------

6483d88 [Security][ACL] Fixed ObjectIdentity::fromDomainObject and UserSecurityIdentity::from(Account|Token) when working with proxies

Discussion
----------

[WIP] Fixed ACL handling of Doctrine proxies

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/jalliot/symfony.png?branch=acl_proxies-2)](http://travis-ci.org/jalliot/symfony)
Fixes the following tickets: #3818, #2611, #2056, #2048, #2035 and probably others
Todo: Fix tests, update changelog

Hi,

As per @fabpot's request, here is #3818 ported to `master`.

> Here is a new attempt to fix all the issues related to Symfony ACL identities and Doctrine proxies.
> It only fixes the issue for Doctrine >=2.2 (older versions of Doctrine will still not work properly with ACL because `Doctrine\Common\Util\ClassUtils` didn't exist before and proxy naming strategy was not consistent between all Doctrine implementations (ORM/ODM/etc.)).

/cc @schmittjoh @beberlei

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

by schmittjoh at 2012-04-09T00:07:52Z

I'm -1 on adding a dependency on the Doctrine class.

The naming scheme was designed in a generic way, we should just copy the method.

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

by jalliot at 2012-04-09T00:13:07Z

@schmittjoh The ACL component requires Doctrine DBAL already (and as such Common) so I don't think this is really an issue at the moment. If (when?) the component is refactored to be decoupled from Doctrine, then maybe we will have to change that.
But I can also copy the class (where?) if you think it is better :)

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

by schmittjoh at 2012-04-09T01:01:27Z

I'd suggest ``Symfony\Component\Security\Core\Util\ClassUtils``.

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

by jalliot at 2012-04-09T11:16:19Z

@fabpot @schmittjoh It's done: I've backported the ClassUtils class and its tests from Doctrine Common into the Security component. Maybe this PR can be merged into 2.0 as well now, what do you think?

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

by oscherler at 2012-04-11T06:27:20Z

There seems to be a consensus that ACLs don’t (fully?) work with Doctrine < 2.2, i.e. in Symfony 2.0. Can it therefore be documented somewhere, typically on the main documentation page on the subject? http://symfony.com/doc/current/cookbook/security/acl.html

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

by jalliot at 2012-04-11T07:36:25Z

@oscherler You can make ACL work with Doctrine < 2.2 and without this patch if you use something like this code (note this is for ORM only but it can be adapted for ODM; it also assumes that your identifier can be retrieved with `getId()`):

``` php
<?php

use Doctrine\ORM\Proxy\Proxy;
use Symfony\Component\Acl\Domain\ObjectIdentity;

$domainObject = ... // some Doctrine entity (maybe a proxy...)

if ($domainObject instanceof Proxy) {
    $objectIdentity = new ObjectIdentity($domainObject->getId(), get_parent_class($domainObject));
} else {
    $objectIdentity = new ObjectIdentity($domainObject->getId(), get_class($domainObject));
}
```
It is ugly but it is the only way to get the correct identity for < 2.2. Never use `ObjectIdentity::fromDomainObject` with a proxy and without this patch!
The same applies to `UserSecurityIdentity`.

This should indeed be documented in the doc for 2.0. /cc @weaverryan

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

by jalliot at 2012-04-11T22:34:37Z

I just fixed the tests and squashed the commits.
I didn't write a test for `UserSecurityIdentity::fromAccount` and `fromToken` because I didn't really have time to look for a clean solution (without creating a full UserInterface implementation for instance...). Anyway the test for `ObjectIdentity::fromDomainObject` should be enough I guess...
Don't hesitate to add one if you think it is necessary. /cc @schmittjoh

@fabpot Apart from @beberlei approval to use MIT license, I think it is ready for merge.
dd5d7f2
@fabpot fabpot merged branch kimhemsoe/process_deadlock (PR #3888)
Commits
-------

89a5c1a [process] Added destructor to process to make sure handles are always closed in the right order.

Discussion
----------

[process] Added destructor to process to make sure handles are always cl...

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
3923ade
@fabpot fabpot merged branch Tobion/generator-cache (PR #3892)
Commits
-------

03d30fd [Routing] remove duplicated cache of compiled routes

Discussion
----------

[Routing] remove duplicated cache of compiled routes

The UrlGenerator caches compiled routes for generating URLs. But the Route class caches it's compiled route itself as long as it does not get modified. So compiled routes are cached twice which makes no sense in my opinion.

Test pass: yes
BC break: no
bf1131c
@Tobion Tobion [Routing] improved generated class by PhpGeneratorDumper 382b083
@jalliot jalliot Update changelog for latest fix in ACL component 36d96e6
@webmozart webmozart [Form] Clarified the CHANGELOG entry about the activation of addXxx()…
…/removeXxx() methods
2e5182f
@fabpot fabpot merged branch bschussek/issue3539 (PR #3898)
Commits
-------

2e5182f [Form] Clarified the CHANGELOG entry about the activation of addXxx()/removeXxx() methods

Discussion
----------

[Form] Clarified the CHANGELOG entry about the activation of addXxx()/removeXxx() methods
2bad835
@fabpot fabpot merged branch jalliot/patch-1 (PR #3895)
Commits
-------

36d96e6 Update changelog for latest fix in ACL component

Discussion
----------

Update changelog for latest fix in ACL component
c1aa618
@fabpot fabpot merged branch Tobion/generator-dumper (PR #3894)
Commits
-------

382b083 [Routing] improved generated class by PhpGeneratorDumper
27a05f4 [Routing] small optimization of PhpGeneratorDumper

Discussion
----------

[Routing] improved PhpGeneratorDumper

Test pass: yes
BC break: no

The first commit only replaces arrays with strings and makes some cosmetic changes, so that it's more readable. This makes the `PhpGeneratorDumper` consistent in style with `PhpMatcherDumper` that I fixed recently and should slightly improve performance of the generation of the class.

The second commit changes the output of the `PhpGeneratorDumper->dump` and tries to optimize the resulting class that is used to generate URLs. It's best explained with an example.

Before my changes:

```php
class ProjectUrlGenerator extends Symfony\Component\Routing\Generator\UrlGenerator
{
    static private $declaredRouteNames = array(
       'Test' => true,
       'Test2' => true,
    );

    /**
     * Constructor.
     */
    public function __construct(RequestContext $context)
    {
        $this->context = $context;
    }

    public function generate($name, $parameters = array(), $absolute = false)
    {
        if (!isset(self::$declaredRouteNames[$name])) {
            throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
        }

        $escapedName = str_replace('.', '__', $name);

        list($variables, $defaults, $requirements, $tokens) = $this->{'get'.$escapedName.'RouteInfo'}();

        return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $absolute);
    }

    private function getTestRouteInfo()
    {
        return array(array (  0 => 'foo',), array (), array (), array (  0 =>   array (    0 => 'variable',    1 => '/',    2 => '[^/]+?',    3 => 'foo',  ),  1 =>   array (    0 => 'text',    1 => '/testing',  ),));
    }

    private function getTest2RouteInfo()
    {
        return array(array (), array (), array (), array (  0 =>   array (    0 => 'text',    1 => '/testing2',  ),));
    }
}
```

After my changes in second commit:

```php
class ProjectUrlGenerator extends Symfony\Component\Routing\Generator\UrlGenerator
{
    static private $declaredRoutes = array(
        'Test' => array (  0 =>   array (    0 => 'foo',  ),  1 =>   array (  ),  2 =>   array (  ),  3 =>   array (    0 =>     array (      0 => 'variable',      1 => '/',      2 => '[^/]+?',      3 => 'foo',    ),    1 =>     array (      0 => 'text',      1 => '/testing',    ),  ),),
        'Test2' => array (  0 =>   array (  ),  1 =>   array (  ),  2 =>   array (  ),  3 =>   array (    0 =>     array (      0 => 'text',      1 => '/testing2',    ),  ),),
    );

    /**
     * Constructor.
     */
    public function __construct(RequestContext $context)
    {
        $this->context = $context;
    }

    public function generate($name, $parameters = array(), $absolute = false)
    {
        if (!isset(self::$declaredRoutes[$name])) {
            throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
        }

        list($variables, $defaults, $requirements, $tokens) = self::$declaredRoutes[$name];

        return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $absolute);
    }
}
```

As you can see, there is no need to escape the route name and invoke a special method anymore. Instead the route properties are included in the static route array directly, that existed anyway. Is also easier to read as defined routes and their properties are in the same place.
5ab006a
@fabpot fabpot merged 2.0 5a320ca
Drak [EventDispatcher] Fixed edge case not covered by tests that generated…
… E_NOTICES
e199049
@fabpot fabpot merged branch drak/eventsubscribersnotice (PR #3902)
Commits
-------

e199049 [EventDispatcher] Fixed edge case not covered by tests that generated E_NOTICES

Discussion
----------

[EventDispatcher] Fixed edge case not covered by tests.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

Fixes case not covered by tests.
b2af6b4
@Tobion Tobion [Routing] small refactoring + language fixes cb47b03
@Tobion Tobion HttpKernel test fix on windows c331f4a
@fabpot fabpot merged branch willdurand/add-all-method-to-form-builder (PR #3886)
Commits
-------

be2456b [Form] [Tests] Used assertCount()
4120f13 [Form] Added all() method to the FormBuilder class

Discussion
----------

[Form] Added all() method to the FormBuilder class

In order to perform some introspection on a FormBuilder instance,
we need to be able to get its children. Almost everything is accessible
in this class, but the children are not.

This PR adds a all() method to respect the current API (get(), remove(),
...).

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

by bschussek at 2012-04-12T09:54:04Z

👍
cec8fed
@fabpot fabpot merged branch Tobion/route-compiler (PR #3891)
Commits
-------

cb47b03 [Routing] small refactoring + language fixes

Discussion
----------

[Routing] small refactoring + language fixes

tests pass, no bc break

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

by vicb at 2012-04-12T06:42:19Z

@Tobion could you squash your commits ?

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

by Tobion at 2012-04-12T19:25:03Z

done
0956be9
@fabpot fabpot [FrameworkBundle] made the Esi and Store instances configurable in Ht…
…tpCache base class
70df8d3
Drak [HttpKernel] Allow override of ContainerBuilder instance used to buil…
…d container
82bbf3b
@fabpot fabpot merged branch drak/kernel_dicb (PR #3909)
Commits
-------

82bbf3b [HttpKernel] Allow override of ContainerBuilder instance used to build container

Discussion
----------

[HttpKernel] Allow override of ContainerBuilder class in Kernel

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

The ContainerBuilder class is hard coded into the `buildContainer()` method and is therefor not extensible.

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

by fabpot at 2012-04-13T04:54:28Z

I would definitely prefer a `getContainerBuilder()` that returns a container builder. But why would you want to do that?

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

by drak at 2012-04-13T05:12:13Z

@fabpot - The use case is to override the behaviour of the compilation that is performed by the kernel on the container.  There is no way to override the compile() method called on the builder because the class is created hard in `Kernel::buildContainer()`. This was the simplest way to change it without other cascading changes.

If we had a method which returned a container builder instance, would it just be something that creates a container builder, or acts as a getter on a property? The reason I ask is there is no real need to have a container builder persisting in a property, so a method that just returns a `new ContainerBuilder()` would also work but then we have to deal with the constructor as there is no way to set a ParameterBag on a container and `Kernel::buildContainer()` needs that.

Basically, my reasoning is that since this particular container is just a throwaway, it seems ok to control the class name that was used to create the throwaway builder.

So are you suggesting doing this instead?

    protected function getContainerBuilder()
    {
        return new ContainerBuilder(new ParameterBag($this->getKernelParameters()));
    }

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

by fabpot at 2012-04-13T05:22:20Z

Yes this was my suggestion. But are you sure you cannot solve your problems with compiler passes?

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

by drak at 2012-04-13T05:38:29Z

@fabpot, yes, this particular issue can't be solved by compiler passes.  I'll adapt the PR.

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

by drak at 2012-04-13T05:46:03Z

@fabpot, amended as per your suggestion.
d87a197
Owner

fabpot commented on 70df8d3 Apr 13, 2012

This is an abstract class. The concrete class is generated under the app/ directory. So, overriding the ESI class is just a matter of overriding the getEsi() method.

Owner

fabpot replied Apr 13, 2012

fixed now

fabpot and others added some commits Apr 13, 2012

@fabpot fabpot [FrameworkBundle] fixed name collision 114bc14
@asm89 asm89 [FrameworkBundle] Make session save path configurable c0e7ee9
@fabpot fabpot merged branch Tobion/httpkernel-test (PR #3906)
Commits
-------

c331f4a HttpKernel test fix on windows

Discussion
----------

HttpKernel test fix on windows

The changes in `StopwatchEventTest` are only for consistency with the other tests in this file.

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

by drak at 2012-04-13T04:16:19Z

@fabpot - This seems to be an eternal problem with the these particular tests. I wonder if there is a better way to do this.  How about a simple greater than condition to show time has elapsed?

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

by Tobion at 2012-04-13T04:33:04Z

The tests are fine. I didn't change them. Just made it more consistent.

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

by drak at 2012-04-13T04:49:20Z

Yes, but if you look at the history of these tests files, they are constantly being tweaked for whatever reason (and they often fail on windows builds randomly). This is a clear indication the tests are not robust and a different approach is probably warranted if it can be found.

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

by drak at 2012-04-13T04:52:53Z

@Tobion - regarding the commit message, what does "fix" refer to if the tests are fine?

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

by Tobion at 2012-04-13T04:56:39Z

The test in `KernelTest` did not pass for me. That's fixed.
1547a82
@fabpot fabpot merged branch asm89/configurable-session-save-path (PR #3912)
Commits
-------

c0e7ee9 [FrameworkBundle] Make session save path configurable

Discussion
----------

[FrameworkBundle] Make session save path configurable

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/asm89/symfony.png?branch=configurable-session-save-path)](http://travis-ci.org/asm89/symfony)

Makes it possible to configure the session save path. It still defaults to saving sessions in the kernel cache dir. This might not be appropriate if you want to keep your user sessions after deploying your application.

As promised to @fabpot I will also do a PR on the docs explaining why this might be useful.

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

by drak at 2012-04-13T10:16:17Z

It might be good to default this value to `%kernel.cache_dir%/sessions`.

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

by stloyd at 2012-04-13T10:30:58Z

@drak https://github.com/symfony/symfony/pull/3912/files#L0R184

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

by drak at 2012-04-13T10:31:57Z

@stloyd need my eyes checked :-P
4631141
@webmozart webmozart [Form] Deprecated FormValidatorInterface and moved implementations to…
… event listeners
6df7a72
@fabpot fabpot merged branch bschussek/remove-form-validator (PR #3797)
Commits
-------

6df7a72 [Form] Deprecated FormValidatorInterface and moved implementations to event listeners

Discussion
----------

[Form] Removed FormValidatorInterface and notion of form validators

Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

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

This cleanup has been lying on a disk for a while, so here it goes.

The FormValidatorInterface has been removed in order to reduce the complexity of the Form API. The same expressivity can be achieved by using POST_BIND event listeners. The POST_VALIDATE event introduced in e6577de can be replaced by a POST_BIND listener with low priority.

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

by bschussek at 2012-04-11T15:19:24Z

Rebased on master.

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

by bschussek at 2012-04-12T12:46:14Z

@fabpot Can this be merged?
baabe26
@vicb vicb [Profiler] Minimize the number of Profile writes
squash

squash
2551270
@vicb vicb [Profiler] Sub requests are not Main requests b611db8
@fabpot fabpot merged branch vicb/profiler/listener (PR #3920)
Commits
-------

b611db8 [Profiler] Sub requests are not Main requests
2551270 [Profiler] Minimize the number of Profile writes

Discussion
----------

[HttpKernel] Profiler Listener tweaks

* `setParent()` is called in [`Profile::addChild()`](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/Profiler/Profile.php#L180) in 2.1
* The profiles are now only saved once only in the listener (either at the end of the main request or on an exception)
* The profiles are now only saved once only in the TraceableEventDispatcher (twice for the root profile when there is a kernel.terminate' event

[![Build Status](https://secure.travis-ci.org/vicb/symfony.png?branch=profiler/listener)](http://travis-ci.org/vicb/symfony)

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

by vicb at 2012-04-13T11:15:25Z

Not so sure for the save part... I'll double check
22177fa
@stof stof [Form] Cleaned the FormValidatorInterface deprecation 9e956a8
@stof stof [Form] Fixed the tests 538819a
@jmikola jmikola [DoctrineBridge] Initialize proxies in UniqueEntityValidator
Fixes #3163
41bdf26
@fabpot fabpot merged branch jmikola/doctrine-unique-proxies (PR #3933)
Commits
-------

41bdf26 [DoctrineBridge] Initialize proxies in UniqueEntityValidator

Discussion
----------

[DoctrineBridge] Initialize proxies in UniqueEntityValidator

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/jmikola/symfony.png?branch=master)](http://travis-ci.org/jmikola/symfony)
Fixes the following tickets: #3163

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

by stof at 2012-04-13T18:17:57Z

I think you should use ``$em->initializeObject($value)`` instead (which is a no-op for other objects)

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

by jmikola at 2012-04-13T18:24:22Z

@stof: Thanks for the suggestion. I wasn't aware of that method.
8e59a4e
@fabpot fabpot merged branch stof/form_cleanup (PR #3931)
Commits
-------

538819a [Form] Fixed the tests
9e956a8 [Form] Cleaned the FormValidatorInterface deprecation

Discussion
----------

[Form] Cleaned the FormValidatorInterface deprecation

This fixes my feedback on #3797

[![Build Status](https://secure.travis-ci.org/stof/symfony.png?branch=form_cleanup)](http://travis-ci.org/stof/symfony)
d9fa1b9
@fabpot fabpot merged 2.0 d2fd9ce
Drak [DependencyInjection] Add ability to clear tags by name. 80f96b7
@stloyd stloyd [Tests] Use proper assertions bfc1aaf
@fabpot fabpot merged branch stloyd/tests (PR #3941)
Commits
-------

bfc1aaf [Tests] Use proper assertions

Discussion
----------

[Tests] Use proper assertions

* `assertEquals(false, *)` -> `assertFalse(*)`
* `assertEquals(true, *)` -> `assertTrue(*)`
* `assertEquals(null, *)` -> `assertNull(*)`
* `assertEquals(x, count(*))` -> `assertCount(x, *)`
* `assertSame(false, *)` -> `assertFalse(*)`
* `assertSame(true, *)` -> `assertTrue(*)`
* `assertSame(null, *)` -> `assertNull(*)`

![Travis CI](https://secure.travis-ci.org/stloyd/symfony.png?branch=tests)

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

by drak at 2012-04-14T08:44:16Z

This is nice, but the commit message leads one to believe bugs are beings fixed, when actually it's just about using PHPUnit shortcut/covenience methods.
8cdf49b
@fabpot fabpot merged branch drak/clear_tag (PR #3940)
Commits
-------

80f96b7 [DependencyInjection] Add ability to clear tags by name.

Discussion
----------

[DependencyInjection] Add ability to clear tags by name.

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

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

by jalliot at 2012-04-14T07:50:51Z

@drak Just for information, what is the use case?

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

by drak at 2012-04-14T08:40:13Z

I'm using this to filter (and prevent) some listeners from being loaded
into the dispatcher.

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

by lsmith77 at 2012-04-14T09:37:39Z

tags are used by compiler passes. bundles that set these tags expect some defined bundle to take some specific actions on the given services. as such this is already a fairly "fragile" relationship. once other compiler passes then start messing with these tags it can get even more tricky. then again, as long as you know what you are doing ..

that being said .. this method just adds convenience, since one could always just get all the tags, unset and reset them in bulk.

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

by drak at 2012-04-14T09:42:04Z

@lsmith77 - exactly.
45d43d3
@willdurand willdurand [Propel1] Fixed phpunit.xml.dist bbf0122
@willdurand willdurand [Propel1] Added composer.json fba5d68
@willdurand willdurand Added propel1-bridge to the main composer.json 51e19d5
@fabpot fabpot merged branch willdurand/propel-bridge (PR #3949)
Commits
-------

51e19d5 Added propel1-bridge to the main composer.json
fba5d68 [Propel1] Added composer.json
bbf0122 [Propel1] Fixed phpunit.xml.dist

Discussion
----------

Added composer.json to the Propel bridge

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

by stof at 2012-04-15T00:14:12Z

you need to add it in the ``replace`` section for the main package too

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

by willdurand at 2012-04-15T00:17:13Z

Ah done, I forgot that file, thanks.
e7470ff

@stof stof and 1 other commented on an outdated diff Apr 16, 2012

...Bundle/FrameworkBundle/Command/RouterDebugCommand.php
$output->writeln($this->getHelper('formatter')->formatSection('router', 'Current routes'));
$maxName = 4;
$maxMethod = 6;
foreach ($routes as $name => $route) {
$requirements = $route->getRequirements();
- $method = isset($requirements['_method']) ? strtoupper(is_array($requirements['_method']) ? implode(', ', $requirements['_method']) : $requirements['_method']) : 'ANY';
+ $method = isset($requirements['_method'])
+ ? strtoupper(is_array($requirements['_method'])
+ ? implode(', ', $requirements['_method']) : $requirements['_method'])
@stof

stof Apr 16, 2012

Member

the formatting is confusing here as the argument of strtoupper is on 2 lines. I first thought that strtoupper was only applied on the condition of the inner ternary operator when looking at it

@lsmith77

lsmith77 Apr 16, 2012

Contributor

what do you prefer .. that i move the closing parenthesis on a line of its own?
i just think the current one line for everything is unreadable.

@stof stof commented on an outdated diff Apr 16, 2012

...Bundle/FrameworkBundle/Command/RouterDebugCommand.php
@@ -87,7 +90,10 @@ protected function outputRoutes(OutputInterface $output, $routes)
$output->writeln(sprintf($format1, '<comment>Name</comment>', '<comment>Method</comment>', '<comment>Pattern</comment>'));
foreach ($routes as $name => $route) {
$requirements = $route->getRequirements();
- $method = isset($requirements['_method']) ? strtoupper(is_array($requirements['_method']) ? implode(', ', $requirements['_method']) : $requirements['_method']) : 'ANY';
+ $method = isset($requirements['_method'])
+ ? strtoupper(is_array($requirements['_method'])
+ ? implode(', ', $requirements['_method']) : $requirements['_method'])
@stof

stof Apr 16, 2012

Member

same here

@stof stof commented on an outdated diff Apr 16, 2012

...Bundle/FrameworkBundle/Command/RouterDebugCommand.php
{
- $output->writeln($this->getHelper('formatter')->formatSection('router', sprintf('Route "%s"', $name)));
-
- if (!isset($routes[$name])) {
+ $route = $this->getContainer()->get('router')->getRouteCollection()->get($name);
+ if (!isset($route)) {
@stof

stof Apr 16, 2012

Member

I would use if (null !== $route) {

Contributor

lsmith77 commented Apr 17, 2012

ok should have addressed all of @stof's concerns now.

Owner

fabpot commented Apr 17, 2012

This should be done on master as this is not a bug fix.

Contributor

lsmith77 commented Apr 17, 2012

i am fine doing it on 2.1, but figured 2.0 users could also benefit, but i guess it could break things if someone extends the command ..

lsmith77 added some commits Apr 17, 2012

@lsmith77 lsmith77 refactored code to use get() when outputting a single route
this is useful for a CMS, where in most cases there will be too many routes to make it feasible to load all of them. here a router implementation will be used that will return an empty collection for ->all(). with this refactoring the given routes will not be listed via router:debug, but would still be shown when using router:debug [name]
b06537e
@lsmith77 lsmith77 improved readability 98a0052

@lsmith77 lsmith77 closed this Apr 17, 2012

Hmm is this realy the right check? After this change I need to rewrite all my ajax stuff because I send "true" or "false" for a field and now all "false" are true! In PHP false == emtpy string.. so maybe we can support this case too?

Contributor

webmozart replied Apr 17, 2012

Yes, this is the right check. When a checkbox/radio is submitted as unchecked, no value is transmitted by the browser and $value here is null. Otherwise, the value configured in the "value" attribute is transmitted. It does not matter for us which value is submitted, only if a value was submitted.

Contributor

webmozart replied Apr 17, 2012

Can you describe your use case in more detail?

Contributor

tecbot replied Apr 17, 2012

IMO the BooleanToStringTransformer has nothing to do with the checkbox. Its a transformer to convert a string to boolean and '' == false and (string)false => ''

Contributor

tecbot replied Apr 23, 2012

It will now stay that way? I hope not. I would need to write a extra transformer to achieve the basic functionality (Boolean to string).

Contributor

webmozart replied Apr 23, 2012

@tecbot The core transformers are tailored to the needs of the core types. If you don't find a fitting transformer, you have to write your own, which is not that difficult I think.

Hardcode literals, is it good?
Are usefull using constants, mb?

return LintCommand::FILE_DOES_NOT_EXISTS

Is there a specific reason why you didn't keep the backward compatibility with a signature like protected function outputRoutes(OutputInterface $output, $routes = null), and a test on $routes? That would have been particularly useful for FOSJsRoutingBundle's fos:js-routing:debug command.

Contributor

lsmith77 replied Jun 28, 2012

i didn't consider it .. in theory we could do protected function outputRoutes(OutputInterface $output, $routes = array()) but i am not sure exactly FOSJSRoutingBundle gains from that?

Contributor

xavierlacot replied Jun 28, 2012

See https://github.com/FriendsOfSymfony/FOSJsRoutingBundle/blob/master/Command/RouterDebugExposedCommand.php#L64

FOSJSRoutingBundle allows to expose a list of routes to generates urls using javascript. One of the features allows to filter the list of exposed routes, and therefore the fos:js-routing:debug should only display this subset, not all of the routes.

I believe the correct signature would rather use $routes = null, as an empty array is still a valid array of routes (we could want to output an empty array of routes...). The null value does not imply such an ambiguity.

Contributor

lsmith77 replied Jun 28, 2012

imho in that case we should rather think about adding a $filter parameter to ->all()

Contributor

xavierlacot replied Jun 28, 2012

Hum, this would end up in creating a RouteCollectionFilter class, making the process of displaying a subset of the routes rather complex and antinomic to the KISS principle... If you don't mind, I will propose a PR with the upper mentioned changes :-)

@dbu dbu referenced this pull request in symfony-cmf/routing Sep 21, 2012

Closed

[DynamicRouter] implement getRouteCollection #1

Not quite sure, but I have the feeling that this changes 2.0 behaviour in regard of RouteCollections that consist only of child RouteCollections.

All these child collections share a common prefix, and previously (2.0) an if (...shared prefix ...) statement surrounded the evaluation code, preventing us from evaluating all child route collections individually.

With this implementation, all child RouteCollections will be checked, also if they share a common prefix that does not match the request...

Is that understandable at all :-)? Shall I file a bug for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment