Commits
-------
b228942 fix for entity choice list when ->loaded is false and the class name is an entity shorthand name and updated tests to work with refactored choicelist
Discussion
----------
fix for entity choice list when ->loaded is false and the class name is ...
...an entity shorthand name
This bug was reintroduced after the latest choicelist refactoring and was originally fixed in 231e79ce0f
---------------------------------------------------------------------------
by stof at 2012-02-01T15:17:37Z
Please also add a unit test for this to avoid further regressions
---------------------------------------------------------------------------
by stof at 2012-02-01T17:05:31Z
btw, a better fix would be to put the real class name in ``$this->class`` to avoid doing it each time (you are doing it in a loop btw). We don't need to keep the short notation anyway
---------------------------------------------------------------------------
by Burgov at 2012-02-01T17:19:01Z
@stof done, thanks for the comments
Commits
-------
4bc0c67 [HttpFoundation] fix a small copy and paste error
Discussion
----------
Just a small copy and paste error...
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo:
Commits
-------
7b79cc2 Fixing typo in XLIFF Dumper
Discussion
----------
Fixing typo in XLIFF Dumper
There was a typo in the service name for the XLIFF Dumper
Commits
-------
57cc531 [Form] Improved PHPDocs of choice lists
9e7e2af [Form] Fixed PHPDoc: Used {@inheritdoc} where applicable
2c530cc Fixed typos in UPGRADE file
7899bea Added examples to UPGRADE
d346ae6 Improved choice list sections of UPGRADE and CHANGELOG
a676598 [Form] Added class LazyChoiceList
Discussion
----------
[Form] Added LazyChoiceList
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Adds a ChoiceList implementation that satisfies people who formerly extended ArrayChoiceList and loaded choices lazily in its `load` method.
---------------------------------------------------------------------------
by craue at 2012-01-30T12:56:49Z
👍
---------------------------------------------------------------------------
by craue at 2012-01-30T14:55:38Z
Not sure if it's a bug in this PR or in #3156, but the labels get replaced by their keys:
```php
<?php
use Symfony\Component\Form\Extension\Core\ChoiceList\ChoiceList;
use Symfony\Component\Form\Extension\Core\ChoiceList\LazyChoiceList;
use Symfony\Component\Form\Extension\Core\ChoiceList\SimpleChoiceList;
class MyChoiceList extends LazyChoiceList {
protected function loadChoiceList() {
$choices = array(
'bla' => 'blaaaaaahhhh',
);
return new SimpleChoiceList($choices, array(), ChoiceList::COPY_CHOICE, ChoiceList::COPY_CHOICE);
}
public function getChoices() {
$choices = parent::getChoices();
// $choices is array('bla' => 'bla')
return $choices;
}
}
```
If it's not this PR, I can of course open a new ticket for that. But I'm only working with `LazyChoiceList`s.
---------------------------------------------------------------------------
by stof at 2012-01-30T16:07:41Z
@craue the ``SimpleChoiceList`` is an implementation using the same string as label and value. If you need different ones, you need to use the ``ChoiceList`` implementation.
---------------------------------------------------------------------------
by craue at 2012-01-30T16:22:06Z
@stof: That would make `SimpleChoiceList` useless for almost any case. Are you sure?
---------------------------------------------------------------------------
by craue at 2012-01-30T16:33:31Z
The bug even occurs when using
```
return new ChoiceList(array_keys($choices), array_values($choices), array(), ChoiceList::COPY_CHOICE, ChoiceList::COPY_CHOICE);
```
---------------------------------------------------------------------------
by stof at 2012-01-30T16:40:19Z
well, the SimpleChoiceList is for simple cases (thus its naming) where you want the same for the label and the values. And if you look at the class, you will see it extends the ChoiceList implementation which is the generic one.
---------------------------------------------------------------------------
by craue at 2012-01-30T16:53:36Z
For me, "simple" would mean that it just takes the array given, using keys as indices and values as labels. No fancy stuff messing around with anything. :D But, is there anything wrong in this code or in mine? @bschussek: Please enlighten me.
---------------------------------------------------------------------------
by bschussek at 2012-01-30T17:16:58Z
You are both wrong :) `getChoices` does not return the choices with their associated labels anymore. What you get now are the choice indices as array keys and the choice values as array values. How both are determined depends on the index and value generation strategy, which, in your case, are both COPY_CHOICE.
The difference between simple and complex choice lists is, that simple choice lists can only contain scalar values as choices, while other choice lists (such as ObjectChoiceList, EntityChoiceList) may contain objects as choices.
Choice labels are now stored in ChoiceView objects, which are returned by the various `get*Views` methods.
---------------------------------------------------------------------------
by craue at 2012-01-30T18:07:43Z
It's pretty annoying having provided an array with keys and values when initializing the `ChoiceList` but being unable to retrieve it again. Guess I just over-used or even abused those choice lists as kind of (not only form related) lookup tables.
---------------------------------------------------------------------------
by bschussek at 2012-01-30T19:27:21Z
@craue: What's your use case?
---------------------------------------------------------------------------
by craue at 2012-01-30T20:10:16Z
I just used choice lists extensively, even for not directly form-related stuff. In one case, I'm using two of them (which are also used individually) to build up a third one. That went well using the old `ArrayChoiceList`s and their `getChoices` method. Just thinking about creating another set of model classes which just contain my lists. So for only one select field in a form it'll take three classes then: (A) a list, (B) a choice list based on A, (C) a choice form type based on B. Oh well ... :D
---------------------------------------------------------------------------
by craue at 2012-01-30T21:31:32Z
Anyway, this PR for `LazyChoiceList` is great, so please merge it. ;)
---------------------------------------------------------------------------
by craue at 2012-01-31T14:00:46Z
@bschussek: Is it ready to be merged?
---------------------------------------------------------------------------
by bschussek at 2012-01-31T16:59:17Z
Yes
Commits
-------
9db6c8d print info about environment and debug mode when running the `CacheWarmupCommand`
Discussion
----------
print info about environment and debug mode when running the `CacheWarmupCommand`
Adapted from `CacheClearCommand`. Replaces #3212.
Commits
-------
5aa5987 [Form] Fixed: form children are always validated in group "Default"
Discussion
----------
[Form] Fixed: form children are always validated in group "Default"
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3219
Todo: -
---------------------------------------------------------------------------
by bschussek at 2012-01-30T12:44:43Z
@craue: Can you verify whether this PR fixes your problem?
---------------------------------------------------------------------------
by craue at 2012-01-30T12:53:59Z
Yeah, it does. 👍
Commits
-------
8dc78bd [Form] Fixed YODA issues
600cec7 [Form] Added missing entries to CHANGELOG and UPGRADE
b154f7c [Form] Fixed docblock and unneeded use statement
399af27 [Form] Implemented checks to assert that values and indices generated in choice lists match their requirements
5f6f75c [Form] Fixed outstanding issues mentioned in the PR
7c70976 [Form] Fixed text in UPGRADE file
c26b47a [Form] Made query parameter name generated by ORMQueryBuilderLoader unique
18f92cd [Form] Fixed double choice fixing
f533ef0 [Form] Added ChoiceView class for passing choice-related data to the view
d72900e [Form] Incorporated changes suggested in PR comments
28d2f6d Removed duplicated lines from UPGRADE file
e1fc5a5 [Form] Restricted form names to specific characters to (1) fix generation of HTML IDs and to (2) avoid problems with property paths.
87b16e7 [Form] Greatly improved ChoiceListInterface and all of its implementations
Discussion
----------
[Form] Improved ChoiceList implementation and made form naming more restrictive
Bug fix: yes
Feature addition: yes
Backwards compatibility break: **yes**
Symfony2 tests pass: yes
Fixes the following tickets: #2869, #3021, #1919, #3153
Todo: adapt documentation
The changes in this PR are primarily motivated by the fact that invalid form/field names lead to various problems.
1. When a name contains any characters that are not permitted in HTML "id" attributes, these are invalid
2. When a name contains periods ("."), form validation is broken, because they confuse the property path resolution
3. Since choices in expanded choice fields are directly translated to field names, choices applying to either 1. or 2. lead to problems. But choices should be unrestricted.
4. Unless a choice field is not expanded and does not allow multiple selection, it is not possible to use empty strings as choices, which might be desirable in some occasions.
The solution to these problems is to
* Restrict form names to disallow unpermitted characters (solves 1. and 2.)
* Generate integer indices to be stored in the HTML "id" and "name" attributes and map them to the choices (solves 3.). Can be reverted to the old behaviour by setting the option "index_generation" to ChoiceList::COPY_CHOICE
* Generate integer values to be stored in the HTML "value" attribute and map them to the choices (solves 4.). Can be reverted to the old behaviour by setting the option "value_generation" to ChoiceList::COPY_CHOICE
Apart from these fixes, it is now possible to write more flexible choice lists. One of these is `ObjectChoiceList`, which allows to use objects as choices and is bundled in the core. `EntityChoiceList` has been made an extension of this class.
$form = $this->createFormBuilder()
->add('object', 'choice', array(
'choice_list' => new ObjectChoiceList(
array($obj1, $obj2, $obj3, $obj4),
// property path determining the choice label (optional)
'name',
// preferred choices (optional)
array($obj2, $obj3),
// property path for object grouping (optional)
'category',
// property path for value generation (optional)
'id',
// property path for index generation (optional)
'id'
)
))
->getForm()
;
---------------------------------------------------------------------------
by kriswallsmith at 2012-01-19T18:09:09Z
Rather than passing `choices` and a `choice_labels` arrays to the view would it make sense to introduce a `ChoiceView` class and pass one array of objects?
---------------------------------------------------------------------------
by stof at 2012-01-22T15:32:36Z
@bschussek can you update your PR according to the feedback (and rebase it as it conflicts according to github) ?
---------------------------------------------------------------------------
by bschussek at 2012-01-24T00:15:42Z
@kriswallsmith fixed
Fixed all outstanding issues. Would be glad if someone could review again, otherwise this PR is ready to merge.
---------------------------------------------------------------------------
by fabpot at 2012-01-25T15:17:59Z
Is it ready to be merged?
---------------------------------------------------------------------------
by Tobion at 2012-01-25T15:35:50Z
Yes I think so. He said it's ready to be merged when reviewed.
---------------------------------------------------------------------------
by bschussek at 2012-01-26T02:30:36Z
Yes.
---------------------------------------------------------------------------
by bschussek at 2012-01-28T12:39:00Z
Fixed outstanding issues. Ready for merge.
Commits
-------
a52c675 [WebProfilerBundle] Improve the logger panel
Discussion
----------
[WebProfilerBundle] Improve the logger panel
No more need to hit 'refresh'
Commits
-------
b177786 Make twig optimizations configurable
Discussion
----------
optimizations not configurable
Valid option for twig but missing in the configuration. I am currently hardsetting this in my own bundle.
Commits
-------
b879397 [Profiler] Optimize time panel IS
d4300b9 [WebProfilerBundle] Tweak the time view
416a2a4 [Stopwatch] Fix some logic
8c3505e [Profiler] Tweak PHPDoc
3bcd154 [HttpKernel] Tweak the Profile class - DRY
Discussion
----------
[Profiler] Stopwatch related tweaks
* Some fixes in the stopwatch logic,
* Some JS fixes,
* Make use of modern JS.
Commits
-------
0533c1b [Form] Fixed: IntegerToLocalizedStringTransformer does not accept "NaN" as valid number anymore
8c63d6d [Form] Fixed: NumberToLocalizedStringTransformer does not accept "NaN" as valid number anymore
aaa9de6 Added test case for checking that 'NaN' string converts into TransformationFailedException in NumberToLocalizedStringTransformer
Discussion
----------
[Form] Disallowed "NaN" as input in number fields
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #3161, #3196
Todo: nothing
Commits
-------
ed9c348 Authentication(Success|Failure)Handler can now return null
Discussion
----------
[Security] Authentication(Success|Failure)Handler can now return null
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Related to the following ticket: #838
[](http://travis-ci.org/odolbeau/symfony)
Correct me if I'm wrong but for now it's not possible to handle Authentication(Success|Failure) in some case only (for example to handle XmlHttpRequest on login form).
With this change, if the handler return null, the default behavior is kept.
---------------------------------------------------------------------------
by stof at 2012-01-24T17:28:49Z
👍
Commits
-------
746170b Make method non static
c3f637b PATCH support and tests for DELETE support
2dd4bf1 Support for PATCH method in forms
Discussion
----------
[form] Support for PATCH method in forms
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Adds support for PATCH method. Accidentally I refactored the test RequestTest a bit to verify PUT, DELETE and PATCH and not only PUT.
---------------------------------------------------------------------------
by lanthaler at 2012-01-25T13:26:26Z
Why do you classify this as something that breaks backwards compatibility? I don't think it breaks anything.
---------------------------------------------------------------------------
by lstrojny at 2012-01-25T13:28:28Z
It’s not that relevant here, but before using patch throws an exception which it no longer does.
---------------------------------------------------------------------------
by lanthaler at 2012-01-25T13:31:35Z
Oh OK..
---------------------------------------------------------------------------
by vicb at 2012-01-25T13:36:39Z
Maybe you can update the [DomCrawler](https://github.com/symfony/symfony/blob/master/src/Symfony/Component/DomCrawler/Form.php) at the same time ?
---------------------------------------------------------------------------
by lstrojny at 2012-01-25T13:55:21Z
Done
Commits
-------
43e0db5 [DomCrawler] Add support for multivalued form fields (fix#1579, #3012)
Discussion
----------
[DomCrawler] Support for multivalued fields
This is a tentative fix for #1579 by @kriswallsmith, also see #3012 for more info.
Any feedback is appreciated.
---------------------------------------------------------------------------
by vicb at 2012-01-05T08:44:51Z
@stof thanks for the valuable feedback I think most of it should be implemented should we use this solution.
The one thing I don't agree is PSR-0, I don't want this class to be public, that's is just a "private" helper class.
There are also missing type hints in the helper class, that should be added.
---------------------------------------------------------------------------
by alessandro1997 at 2012-01-05T10:05:15Z
Well, @vicb, I think it's up to the developer to not use "private" classes. Just write it in the documentation. But declaring two classes in the same file would be a big violation of the standards.
---------------------------------------------------------------------------
by vicb at 2012-01-05T11:28:53Z
What "standard"s ?
PSR-0 is about auto-loading, I don't want/need this to be autoloaded.
Sf coding standards ? Well relying on a developer reading the doc is more error prone than the current implementation. I sometimes favor pragmatism over theory.
edit: I am not trying to say I am right here but only that I don't see any added value in moving the helper class to a dedicated file. I appreciate any feedback, really.
---------------------------------------------------------------------------
by fabpot at 2012-01-06T11:55:09Z
FYI, we already have such a "private" class in https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php#L135
---------------------------------------------------------------------------
by vicb at 2012-01-06T16:36:04Z
@alessandro1997 if you need an example on why it is not safe to rely on developers reading comments, see #2892
---------------------------------------------------------------------------
by vicb at 2012-01-09T22:19:52Z
@fabpot I am waiting for your feedback on the [proposed API](https://github.com/symfony/symfony/pull/3035/files#L1R57) before finishing this PR.
---------------------------------------------------------------------------
by drak at 2012-01-10T05:12:16Z
@fabpot
> FYI, we already have such a "private" class in https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Security/Http/Firewall/DigestAuthenticationListener.php#L135
Why on is that necessary, it could just be another class file in the namespace. Unless you are making some kind of forward compatibility, e.g. for with a new class in PHP 5.4 then I see no reason to do that.
---------------------------------------------------------------------------
by vicb at 2012-01-10T07:40:32Z
What would be a good reason not to allow "private" classes ?
If the Sf coding standards are the only good reason let's change them then.
[Java](http://stackoverflow.com/questions/968347/can-a-java-file-have-more-than-one-class) and [ActionScript3](http://livedocs.adobe.com/flex/3/html/help.html?content=03_Language_and_Syntax_05.html) allow such construction
I would no say any better than the above link on Stack Overflow:
> The purpose of including multiple classes in one source file is to bundle related support functionality (internal data structures, support classes, etc) together with the main public class. Note that it is always ok not to do this -- the only effect is on the readability (or not) of your code.
---------------------------------------------------------------------------
by Tobion at 2012-01-10T09:35:09Z
There are also many private classes in the test cases.
---------------------------------------------------------------------------
by stof at 2012-01-10T13:29:08Z
@Tobion for tests, it is logical because there is no autoloader for the test classes.
---------------------------------------------------------------------------
by vicb at 2012-01-10T13:31:53Z
@stof by definition you do not want a "private" class to be autoloaded anyway.
---------------------------------------------------------------------------
by alessandro1997 at 2012-01-10T14:11:42Z
Sure, but what you're doing here is just making instantiating the class a bit more difficult. If a stubborn developer wants to use it, then he (or her) can include the file manually or autoload the "main class".
PHP does NOT have support for private/inner classes, and, until it does, all classes should be istantiable normally.
---------------------------------------------------------------------------
by stof at 2012-01-10T14:23:30Z
@vicb what about someone wanting to serialize the object ? (well, serializing is not the issue. unserializing is)
---------------------------------------------------------------------------
by vicb at 2012-01-10T14:57:52Z
@alessandro1997 you are absolutely right, it's not meant to be instantiated from the outside (it's **private**). You could argue the same with private properties & methods (using Reflection). Dead-end.
@stof Is unserializing really an issue as the file would have been loaded already ?
---------------------------------------------------------------------------
by fabpot at 2012-01-22T09:38:13Z
@vicb: I'm fine with the proposed API, but I fail to see why it would be more BC than #3012.
---------------------------------------------------------------------------
by vicb at 2012-01-22T10:06:56Z
For BC I have to check #3012 again but at some point if I remember correctly the public API had changed (not sure about the latest version in your branch)
By introducing the private helper class, it is quite easy to see that the public API is not modified by this PR.
Next steps:
* Stof the code,
* Add/fix phpdoc,
* Add tests for the helper class,
* Add/refactor tests for the `Form` class.
@fabpot if you agree with the above steps it could be ready sometime next week.
---------------------------------------------------------------------------
by fabpot at 2012-01-22T10:21:16Z
The API is perhaps not changed but the behavior will certainly changed. I agree with your steps.
---------------------------------------------------------------------------
by vicb at 2012-01-22T10:45:10Z
Which leads to the question: should we consider this as a change in behavior (2.1) or a bug fix (2.0) ?
_I am thinking of a form with multiple fields named `field[]`_
---------------------------------------------------------------------------
by fabpot at 2012-01-22T11:32:04Z
@vicb: this change should be done on master
---------------------------------------------------------------------------
by vicb at 2012-01-24T07:59:40Z
Should be ready now, let me know when I should squash after review.
---------------------------------------------------------------------------
by fabpot at 2012-01-24T08:18:03Z
@vicb: yes, can you squash your commits?
---------------------------------------------------------------------------
by vicb at 2012-01-24T08:29:58Z
@fabpot done
