Commits
-------
bad4a1f [OptionsResolver] CS fix in LazyOption
a54ea1b [OptionsResolver] small optimization in Options class
104dcf2 [OptionsResolver] fixed bugs concerning required options
1bfcff4 [OptionsResolver] added failing test cases to demonstrate two bugs
37a3a29 [OptionsResolver] optimized validation
Discussion
----------
[OptionsResolver] fixed two bugs and applied optimization
The first commit optimizes the validation in OptionsResolver by removing several unneeded method calls (without changing anything semantically).
Then I recognized two bugs in the current code that I wrote failing test cases for in the second commit.
1. setAllowedValues wrongly validated missing options
2. required options with defaults were considered missing by `resolve` (contrary to the `isRequired` method)
The third commit fixes these bugs.
The forth commit applies a small optimization in Options and uses a static function call for a static function.
---------------------------------------------------------------------------
by travisbot at 2012-05-24T03:39:34Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1418785) (merged a54ea1b6 into b07fb3c4).
---------------------------------------------------------------------------
by travisbot at 2012-05-24T05:22:33Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1419232) (merged bad4a1f7 into b07fb3c4).
---------------------------------------------------------------------------
by bschussek at 2012-05-24T06:20:02Z
I just tested this on my machine, and static calls are a tiny bit faster here, although this is really irrelevant for practical use. Even though I dislike useless micro-optimizations like this, I'm ok with this PR in general.
---------------------------------------------------------------------------
by Tobion at 2012-05-24T13:23:11Z
I didn't say that's an optimization in the first place. (The optimization was the removal of a variable assignment)
I just changed it because in other PRs I've been told, static functions should be called in a static way.
---------------------------------------------------------------------------
by Tobion at 2012-05-24T23:36:13Z
Please merge before 4387
When installing the bundle and the bridge from the standalone repositories
the relative path between them is different. This simply backports the
change done in symfony 2.1 to allow using subtree repositories with 2.0.x
too.
Commits
-------
40fd99e [FrameworkBundle] Added another missing dependency to Config
Discussion
----------
Yet another composer missing dep
Config is only suggested by DI, not required. So it not installed currently.
Commits
-------
03183b5 [Templating] added missing @return PHPDoc for LoaderInterface::isFresh method.
Discussion
----------
Template loader phpdoc
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: -
License of the code: MIT
Documentation PR: ~
---------------------------------------------------------------------------
by travisbot at 2012-05-22T17:39:15Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1402200) (merged 03183b5b into 55faa546).
Commits
-------
82c221a [Form] Fixed strict "data_class" check to work with instances of \ArrayAccess
Discussion
----------
[Form] Fixed collection type to work with recent Form changes
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
---------------------------------------------------------------------------
by tristanbes at 2012-05-22T16:42:36Z
Ping @fabpot Could you please merge it ASAP, because this bugs breaks all forms containing collection type.
Thanks
---------------------------------------------------------------------------
by travisbot at 2012-05-22T16:54:24Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1401580) (merged 82c221a1 into 1a1403f5).
Commits
-------
b7fc009 [Config] Numerical keys for prot. arrays if useAttributeAsKey is set
Discussion
----------
[Config] Numerical keys for prot. arrays if useAttributeAsKey is set
Bug fix: not sure
Feature addition: not sure
Backwards compatibility break: not sure
Symfony2 tests pass: [](http://travis-ci.org/asm89/symfony)
License of the code: MIT
When using an array node with children of prototype array and `useAttributeAsKey`, using numerical values for the keys throws an exception. For example (`useAttributeAsKey('id')`):
``` php
<?php
// works
array (
'thing' => array(
array('foo', 'bar', 'id' => 'a')
)
);
// works and is the same as above
array (
'thing' => array(
'a' => array('foo', 'bar')
)
);
// works
array(
'thing' => array(
array('foo', 'bar', 'id' => 42), array('baz', 'qux', 'id' => 1337),
),
);
// works with this patch and is the same as above
array(
'thing' => array(
42 => array('foo', 'bar'), 1337 => array('baz', 'qux'),
),
);
```
---------------------------------------------------------------------------
by travisbot at 2012-05-14T14:26:32Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1327383) (merged 42d252da into 46ffbd52).
---------------------------------------------------------------------------
by travisbot at 2012-05-14T14:32:59Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1327430) (merged b7fc0093 into 46ffbd52).
---------------------------------------------------------------------------
by vicb at 2012-05-21T15:16:24Z
Could this be fixed by changing [PrototypedArrayNode](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Config/Definition/PrototypedArrayNode.php#L255)
`$k = $v[$this->keyAttribute];` -> `$k = (string) $v[$this->keyAttribute];`
---------------------------------------------------------------------------
by asm89 at 2012-05-22T07:01:53Z
I tried it and the test I added still fails. The code change you propose doesn't execute because `if (!isset($v[$this->keyAttribute]) && is_int($k))` will still evaluate to true.
Commits
-------
0935964 Modified example for mod_rewrite to not add Authorization header if it is not set in the request
Discussion
----------
Correct example mod_rewrite to not add Authorization header that does not exists
The in-line PHP code comment suggest to do some changes in .htaccess with mod_rewrite to pass HTTP-Authorization header to PHP. This leads to the Authorization header being introduced even when it's not originally in the request (albit empty, the result of ParameterBag->has('Authorization') will return true when you expect it not to.
Some external libraries might check for this header and perform logic based on wether it was set or not (The php-oauth2 library in my case).
I suggest this fix which I think is a more proper way of handling the case anyway, since when the header is not set you don't expect it to exist in the ServerBag either.
(I tried to search the documentation for this but did not find it, but I guess this probably should go into the documentation somewhere?)
---------------------------------------------------------------------------
by travisbot at 2012-05-22T12:51:25Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399753) (merged 0935964b into 517ae43f).
Commits
-------
e92212a [Form] Added valid attribute to a FormView
Discussion
----------
[Form] Added "valid" to view parameters
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo:
License of the code: MIT
Documentation PR:
This PR adds a parameter of contains_errors to a FormView when buildView is called.
The use case for this addition is when you want to show that a form or sub forms has errors (e.g. when rendering a long form a header message of "This form contains errors" or adding a class to a whole sub form to show an erroneous state) is currently very difficult/near impossible and may need the original form object being accessible in the view layer.
Whats a bit grey here is the best phrasing to use for this. Options I weighed up were is_valid (seemed a bit semantically incorrect in a template, since it would return true pre bind) and has_errors_deep (which i wasn't sure if it fitted naming conventions).
---------------------------------------------------------------------------
by travisbot at 2012-05-07T20:25:55Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1269345) (merged fe1f5aee into 919604ab).
---------------------------------------------------------------------------
by henrikbjorn at 2012-05-19T18:36:33Z
couldnt you just use `{% if errors %}` in your views?
---------------------------------------------------------------------------
by stof at 2012-05-19T19:17:20Z
@henrikbjorn ``errors`` contains only the errors attached to the form itself. It will not tell you when there is some errors in a child form.
---------------------------------------------------------------------------
by kevindew at 2012-05-22T10:43:45Z
Sure, done
---------------------------------------------------------------------------
by travisbot at 2012-05-22T10:45:12Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399006) (merged 9f6658d4 into 517ae43f).
---------------------------------------------------------------------------
by kevindew at 2012-05-22T10:57:54Z
Shoot, sorry about that.
---------------------------------------------------------------------------
by travisbot at 2012-05-22T11:00:00Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399096) (merged 88920591 into 517ae43f).
---------------------------------------------------------------------------
by bschussek at 2012-05-22T11:17:38Z
Can you squash the commits and prefix the message with "[Form]" please?
---------------------------------------------------------------------------
by travisbot at 2012-05-22T11:49:52Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1399390) (merged ca771822 into 517ae43f).
---------------------------------------------------------------------------
by kevindew at 2012-05-22T11:53:18Z
Sure, done (I've sorted the travis fail btw)
---------------------------------------------------------------------------
by travisbot at 2012-05-22T11:55:10Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1399404) (merged e92212ac into 517ae43f).
---------------------------------------------------------------------------
by bschussek at 2012-05-22T12:25:45Z
@fabpot 👍
Commits
-------
2c19b3c Empty shortcut check in the constructor
cf9039e Added a unit test for the shortcut name of the InputOption class
Discussion
----------
[Console] Single dash for option shortcuts
See https://github.com/symfony/symfony/pull/4062
---------------------------------------------------------------------------
by travisbot at 2012-05-20T13:09:18Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1380389) (merged 02290da4 into f433f6b0).
---------------------------------------------------------------------------
by stof at 2012-05-20T13:16:34Z
please rebase your branch to get rid of these merge commits
btw, you should use feature branches to send your next pull requests instead of using your master branch each time, which limits you to a single PR.
---------------------------------------------------------------------------
by Nanocom at 2012-05-20T13:39:32Z
Sorry for the mess, cleaning it
---------------------------------------------------------------------------
by travisbot at 2012-05-20T13:41:46Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1380549) (merged 63129657 into f433f6b0).
---------------------------------------------------------------------------
by travisbot at 2012-05-20T13:43:07Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1380553) (merged 2c19b3c7 into f433f6b0).
Commits
-------
23bad29 Added translation to placeholder and title attributes
Discussion
----------
Added translation to placeholder and title attributes
---------------------------------------------------------------------------
by schmittjoh at 2012-05-03T13:52:38Z
Better translate it where it is defined.
Dynamic translations are usually not desirable as they cannot be automatically extracted, and thus require more work.
---------------------------------------------------------------------------
by ruimarinho at 2012-05-03T13:57:30Z
@schmittjoh but isn't that the same case as with labels for instance? I don't think injecting the translator service into the form type would require less work than what this PR suggests.
---------------------------------------------------------------------------
by schmittjoh at 2012-05-03T14:02:02Z
Yeah, same thing.
There might be some cases where it's fine, but in general, we should try to not translate dynamic vars.
---------------------------------------------------------------------------
by ruimarinho at 2012-05-03T14:17:44Z
@schmittjoh I think that's one of those cases, since these attributes in particular (title and placeholder) are intended to aid the user with a brief description. I understand (and agree) with your concern regarding dynamic vars, but in my opinion this is a use case where it is worth it. Just my two cents :)
---------------------------------------------------------------------------
by stof at 2012-05-03T18:07:01Z
@schmittjoh the issue is that translating the label before the template would require injecting the translator in the form types (as the form label can be set there) and would force the user to duplicate the translation process if they pass the label explicitly in the template.
Commits
-------
64101ab separate numeric value from suffix in File constraint's error message `$uploadIniSizeErrorMessage`
ff122d3 fixed tests
868d649 separate numeric values from suffixes in File constraint's error message `$maxSizeMessage`
Discussion
----------
[Validator] separate numeric values from suffixes in File validation error messages
This change allows me to locale-aware format the numbers in a form theme, i.e., to use a comma instead of a dot. If there's a better way without re-implementing the entire validator, let me know.
Such changes also allow for using a different separator than the usual space in translations.
---------------------------------------------------------------------------
by travisbot at 2012-05-08T19:14:16Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1278845) (merged f7c50098 into e54f4e46).
---------------------------------------------------------------------------
by travisbot at 2012-05-08T19:23:31Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1278940) (merged ce1cdafc into e54f4e46).
---------------------------------------------------------------------------
by r1pp3rj4ck at 2012-05-10T11:05:18Z
I don't know if there is a better way to do this, but I like the idea anyway.
---------------------------------------------------------------------------
by craue at 2012-05-11T14:18:52Z
Separated numeric values and suffixes for `$maxSizeMessage` and `$uploadIniSizeErrorMessage` now. Can't find any other relevant places (in other validators). Might be merged if accepted.
---------------------------------------------------------------------------
by travisbot at 2012-05-11T14:19:10Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1305246) (merged 438da7dd into e54f4e46).
---------------------------------------------------------------------------
by travisbot at 2012-05-11T21:22:25Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1309277) (merged 64101aba into dd0da03c).
---------------------------------------------------------------------------
by sstok at 2012-05-13T13:29:07Z
Using the NumberFormatter class would be an option, but that would also add a dependency when using Validation as stand-alone so using the {{ suffix }} is a good idea.
---------------------------------------------------------------------------
by craue at 2012-05-13T14:15:54Z
Using a NumberFormatter (if available) directly in the validator might indeed be a good option. In either case, having the numeric value separated from its suffix looks cleaner.
---------------------------------------------------------------------------
by craue at 2012-05-19T13:36:00Z
@fabpot: Would you merge this?
Commits
-------
fe7b258 Removed unnecessary use statements
Discussion
----------
[BrowserKit] Removed unnecessary use statements
Merely a cosmetical fix. My IDE (PhpStorm) marks the `use Symfony\Component\BrowserKit\Client;` statement in `Symfony/Component/BrowserKit/Client.php` itself as an error.
I also removed the two other `use`s of classes from the same namespace.
---------------------------------------------------------------------------
by travisbot at 2012-05-18T11:27:02Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1365455) (merged fe7b2583 into 1e15f210).
Commits
-------
46be121 added tokenDataExists() method to prevent loading complete profile structures upon writes
Discussion
----------
[HttpKernel] prevent loading complete profile structures upon writes
The abstract class "PdoProfilerStorage" uses its ::read() method to decide if a profiler record has to be updated or initially created upon a ::write() call. This possibly causes huge memory consumption, as ::read() recursively reads all existing profiles using ::createProfileFromData() calls. When handling many sub-request this may lead into either a "out of memory" or XDebug's "maximum nesting level reached" - whichever comes first.
To prevent this issue, I added a new protected method ::tokenDataExists() that simply checks whether a record for the token in question already exists in storage.
---------------------------------------------------------------------------
by travisbot at 2012-05-18T08:56:56Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1364303) (merged 46be1212 into 1e15f210).
Commits
-------
bc7043f [Routing] removed unused constructor arguments in test classes
d81fdfe [Routing] fixed namespace of test classes
53aaf76 [Routing] removed unused property of Router
Discussion
----------
[Routing] removed unused property of Router
Test pass: yes
BC break: no
This PR removes an unused property ($defaults) of Router. The property was passed to the constructor of UrlMatcher and UrlGenerator although these classes don't accept this parameter at all. And the dumped equivalents of them (produced by PhpMatcherDumper/PhpGeneratorDumper) don't need this property either.
So passing the $defaults was wrong and the property is not used in any way.
The DI config does not define anything for this property either: [routing.xml](https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Resources/config/routing.xml#L52)
The second commit fixes the namespaces of test classes that were wrong.
The third commit removes false arguments in the test classes.
---------------------------------------------------------------------------
by travisbot at 2012-05-22T07:34:03Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1397749) (merged bc7043f1 into e4e3ce6c).
Commits
-------
f883953 TypeGuess fixed for Date/Time constraints
41bed29 [Form] fixed invalid 'type' option in ValidatorTypeGuesser for Date/TimeFields
Discussion
----------
[Form] fixed invalid 'type' option in ValidatorTypeGuesser for Date/TimeFields
Automatic field type guessing breaks, if you use any of the Date/Time
constraints (i.e. Symfony\Component\Validator\Constraints\DateTime), since these field types have no 'type' option defined.
(See getDefaultOptions() in DateTimeType.php)
---------------------------------------------------------------------------
by travisbot at 2012-05-10T15:25:16Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1296309) (merged 005bdbb0 into 68eca0f9).
---------------------------------------------------------------------------
by travisbot at 2012-05-18T15:50:39Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1367774) (merged f8839532 into a04acc89).
---------------------------------------------------------------------------
by TonyMalz at 2012-05-18T15:58:57Z
@bschussek Ok, changed it to 'input'
---------------------------------------------------------------------------
by bschussek at 2012-05-22T08:18:27Z
👍
Commits
-------
4fa8e68 Add support for javascript object notation in allowed JSONP callback
Discussion
----------
Add support for javascript object notation in allowed JSONP callback
---------------------------------------------------------------------------
by travisbot at 2012-05-18T23:09:45Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1371497) (merged 4fa8e68c into 18132c18).
This reverts commit 5182a0c2c4.
PropertyPath instances should be empty. If you have an empty property path string, there is no need to create a PropertyPath instance for it.
Conflicts:
tests/Symfony/Tests/Component/Form/PropertyPathTest.php
Commits
-------
5d1b366 [Form] fix PhpDoc
Discussion
----------
[Form] fix PhpDoc
---------------------------------------------------------------------------
by travisbot at 2012-05-21T02:31:56Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1385372) (merged 5d1b3669 into 1407f112).
---------------------------------------------------------------------------
by vicb at 2012-05-21T05:44:03Z
have you used inheritdoc (over inheritDoc) on purpose ? (I must admin I haven't followed the CS discussion very closely)
---------------------------------------------------------------------------
by Tobion at 2012-05-21T16:16:48Z
Yes, inheritdoc is used on symfony. It's also the recommended way of phpdoc itself (all lower-case).
Commits
-------
0a3dd0f [Console] Check for existence of proc_open to fix#4338
Discussion
----------
[Console] Check for existence of proc_open to fix#4338
It is quite common to disable proc_open for security purposes.
This PR checks for the existence of the proc_open function and fixes Issue #4338
Replacement for PR4356
---------------------------------------------------------------------------
by travisbot at 2012-05-21T10:49:59Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1387784) (merged dd991bea into 1407f112).
---------------------------------------------------------------------------
by travisbot at 2012-05-21T11:35:21Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1387991) (merged 0a3dd0fe into 1407f112).
---------------------------------------------------------------------------
by fabpot at 2012-05-21T12:17:32Z
I forgot to ask you to sent a new PR for the 2.0 branch instead of master. Thanks.
---------------------------------------------------------------------------
by davidwindell at 2012-05-21T13:03:36Z
proc_open is not used in 2.0?
Commits
-------
a450d00 [HttpFoundation] HTTP Basic authentication is broken with PHP as cgi/fastCGI under Apache
Discussion
----------
[HttpFoundation] HTTP Basic authentication is broken with php-cgi under Apache
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1813
Todo: -
In order to work, add this to the .htaccess:
RewriteEngine on
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ app.php [QSA,L]
---------------------------------------------------------------------------
by stof at 2012-03-10T17:34:26Z
you should also add a unit test for this
---------------------------------------------------------------------------
by kepten at 2012-03-11T15:34:04Z
Thanks for the feedback, I committed the changes.
---------------------------------------------------------------------------
by stof at 2012-04-04T01:59:53Z
@fabpot could you review it ?
---------------------------------------------------------------------------
by fabpot at 2012-04-04T07:15:34Z
My comments:
* `ServerBag` represents what we have in the `$_SERVER` global variables. As such, the code should be moved to the `getHeaders()` method instead like the other tweaks we do for the HTTP headers.
* A comment must be added explaining why this is needed and the configuration the user must have to make it work (then remove the Github URLs).
* The code should only be executed when `PHP_AUTH_USER` is not available (to not have any overhead when not needed).
---------------------------------------------------------------------------
by danielholmes at 2012-04-14T13:27:09Z
A quick note on that .htaccess/apache configuration required, if adding to the Symfony SE htaccess file, then it will need to look like this:
```
<IfModule mod_rewrite.c>
RewriteEngine On
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ app.php [QSA,L]
</IfModule>
```
NOTE: No **,L** in the Authorization Rewrite as in the original example - it prevents the front controller rewrite from happening
---------------------------------------------------------------------------
by towards at 2012-04-20T16:12:49Z
@kepten you were faster than me applying @fabpot's comments :) nevertheless part of the bug hunt day I also modified the ServerBag class and tested them on a productive LAMP hosting server using Apache and FastCGI
---------------------------------------------------------------------------
by kepten at 2012-04-20T16:15:57Z
ok, so is my PR is useless or should I still fix problems?
---------------------------------------------------------------------------
by towards at 2012-04-20T16:20:26Z
your PR is fine for sure and I don't want to interfere, just wanted to mention that part of the bug hunt day of Symfony I had a go at this PR as an "exercise" but just saw later on that you already fixed the problem, so you can ignore my pushes
---------------------------------------------------------------------------
by vicb at 2012-04-20T16:20:36Z
I have been working with @towards: your PR is useful, please implement his comments and squash your PR.
---------------------------------------------------------------------------
by kepten at 2012-04-20T16:59:07Z
never squashed before, is it okay now? :)
---------------------------------------------------------------------------
by stof at 2012-04-20T17:21:07Z
it is
---------------------------------------------------------------------------
by vicb at 2012-05-20T19:57:51Z
@fabpot this should be ready to be merged
