Commits
-------
9b0245b [Form] Made prefix of adder and remover method configurable. Adders and removers are not called if "by_reference" is disabled.
49d1464 [Form] Implemented MergeCollectionListener which calls addXxx() and removeXxx() in your model if found
7837f50 [Form] Added FormUtil::singularify()
Discussion
----------
[Form] Forms now call addXxx() and removeXxx() in your model
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1540
Todo: adapt documentation
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue1540)
Adds functionality for calling `addXxx` and `removeXxx` method in your model. All types returning a collection of values are affected: "collection", "choice" (with multiple selection) and "entity" (with multiple selection).
Example:
class Article
{
public function addTag($tag) { ... }
public function removeTag($tag) { ... }
public function getTags($tag) { ... }
}
And the controller:
$form = $this->createFormBuilder($article)
->add('tags')
->getForm();
Upon modifying the form, addTag() and removeTag() are now called appropiately.
---------------------------------------------------------------------------
by stof at 2012-02-01T18:23:49Z
Really great
---------------------------------------------------------------------------
by vicb at 2012-02-01T18:24:04Z
Great !!
Two suggestions:
* make "add" and "remove" configurable,
* introduce a base class for the remove listeners with (final?) `::getSubscribedEvents()` and `::getEventPriorities()`
---------------------------------------------------------------------------
by haswalt at 2012-02-01T18:57:46Z
+1 this
---------------------------------------------------------------------------
by daFish at 2012-02-01T19:54:46Z
+1
---------------------------------------------------------------------------
by michelsalib at 2012-02-01T20:55:37Z
Can wait to have it!
It will save lots of time trying to solve WTF effects and making workarounds.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T09:37:12Z
@vicb: Your first point is done. The second, I don't understand.
---------------------------------------------------------------------------
by stof at 2012-02-02T09:40:50Z
@bschussek your branch conflicts with master according to github
---------------------------------------------------------------------------
by vicb at 2012-02-02T09:52:40Z
@bschussek my point is that I can stand hard-coded priorities which are error prone. A better solution might be to introduce constants (in `FormEvents` / `FormEventPriorities` ?) with meaningful names.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T10:21:52Z
@stof Rebased
@vicb I know, but who is responsible for managing priorities? There is no central entitty that can do this. (btw this is a general problem of the priority system of the EventDispatcher)
@fabpot Ready to merge.
---------------------------------------------------------------------------
by vicb at 2012-02-02T10:23:28Z
@bschussek doesn't each form has is own dispatcher so there is no need for a global registry here, something local to the form could be good enough.
The listener is used by the Collection type as well as the Choice and Entity type (with multiple
selection). The effect is that you can have for example this model:
class Article
{
public function addTag($tag) { ... }
public function removeTag($tag) { ... }
public function getTags($tag) { ... }
}
You can create a form for the article with a field "tags" of either type "collection" or "choice"
(or "entity"). The field will correctly use the three methods of the model for displaying and
editing tags.
Commits
-------
2e4ebe4 [Validator] Renamed methods addViolationAtRelativePath() and getAbsolutePropertyPath() in ExecutionContext
9153f0e [Validator] Deprecated ConstraintValidator methods setMessage(), getMessageTemplate() and getMessageParameters()
0417282 [Validator] Fixed typos
a30a679 [Validator] Made ExecutionContext immutable and introduced new class GlobalExecutionContext
fe85bbd [Validator] Simplified ExecutionContext::addViolation(), added ExecutionContext::addViolationAt()
f77fd41 [Form] Fixed typos
1fc615c Fixed string access by curly brace to bracket
a103c28 [Validator] The Collection constraint adds "missing" and "extra" errors to the individual fields now
f904a9e [Validator] Fixed: GraphWalker does not add constraint violation if error message is empty
1dd302c [Validator] Fixed ConstraintViolationList::__toString() to not include dots in the output if the root is empty
1678a3d [Validator] Fixed: Validator::validateValue() propagates empty validation root instead of the provided value
Discussion
----------
[Validator] Improved "missing" and "extra" errors of Collection constraint
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2615
Todo: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue2615)
Instead of a single violation
Array:
The fields "foo", "bar" are missing
various violations are now generated.
Array[foo]:
This field is missing
Array[bar]:
This field is missing
Apart from that, the PR contains various minor fixes to the Validator.
---------------------------------------------------------------------------
by bschussek at 2012-02-02T09:14:52Z
@fabpot Ready for merge.
Commits
-------
7cecb4e [Form] added support for parent of FormBuilder
Discussion
----------
[Form] added support for parent of FormBuilder
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
In some cases of building forms, it is good to know the attributes of the parent builder. In my case I want to automaticaly add the subscriber to the fields, whose parent builder has a concrete attribute.
Replace #2882
Commits
-------
e9b4c58 [Console] Enable process isolantion in Shell
Discussion
----------
[Console] Enable process isolantion in Shell
Bug fix: no
BC break: no
Feature addition: yes
Symfony2 test pass: yes
Fixes the following tickets: #2848#2847
Todo: Write unit tests
See tickets for reference, need help with unit testing, because I don't know how to test this :)
---------------------------------------------------------------------------
by canni at 2011-12-16T09:36:32Z
I've tested this with different scenarios like "inception" (invoking shell from shell - will not work) ;) and others, everything seems to work great.
As I have no idea on how to pack this with unit testing some help needed, also as I don't have any windows in home ;) need someone to test it on MS os.
And we should decide, do we want process isolation by default? (This will not break the BC, break only the "expected behavior" - colorful output and "interactivity")
---------------------------------------------------------------------------
by canni at 2011-12-18T15:14:26Z
I've rebased this branch to match current `HEAD` and I've added usage of new process builder, for better portability an shell arg escaping.
---------------------------------------------------------------------------
by fabpot at 2012-02-02T08:28:32Z
@canni: Can you squash your commits before I merge this PR? Thanks.
---------------------------------------------------------------------------
by canni at 2012-02-02T09:07:16Z
@fabpot @stof done.
Bug fix: no
BC break: no
Feature addition: yes
Symfony2 test pass: yes
Fixes the following tickets: #2848#2847
Todo: -
See tickets for reference, need help with testing, because I don't know how to test this :)
Commits
-------
de253dd [Form] read_only and disabled attributes
Discussion
----------
[Form] read_only and disabled attributes (closes#1974)
1. Removed ``readOnly`` property from ``Form``, as it is no longer required
2. Introduced ``disabled`` property to ``Form``, behaves exactly like ``readOnly`` used to
3. Added ``disabled`` property to fields, defaults to ``false``, renders as ``disabled="disabled"``
4. A field with positive ``read_only`` property now renders as ``readonly="readonly"``
---------------------------------------------------------------------------
by helmer at 2012-01-26T17:46:17Z
I changed ``Form`` and ``FormBuilder`` property ``readOnly`` to ``disabled``. On second thought, this is perhaps not such good change - while readOnly somewhat implied the use-case, disabled no longer does.
Perhaps something else, like ``bindable`` (as not to confuse with read_only attribute of Fields)?
@bschussek, others, any thoughts?
---------------------------------------------------------------------------
by bschussek at 2012-01-31T06:53:59Z
Please prefix commits with the affected component, if applicable.
---------------------------------------------------------------------------
by helmer at 2012-01-31T08:41:03Z
@bschussek Prefixed. Please also see see to [this question](https://github.com/symfony/symfony/pull/3193#issuecomment-3673074)
Commits
-------
6090dee [FormType] Adopted MoneyTypeTest::testMoneyPatternWorksForYen to CS
e814d27 [FormType] Fixed broken MoneyType regexp for JPY
Discussion
----------
[Bugfix][Form] Fixed broken MoneyType regexp for JPY
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: ![Build Status](https://secure.travis-ci.org/ManuelKiessling/symfony.png?branch=ticket_3124) Fixes the following tickets: #3124
Todo: -
The regexp in MoneyType doesn't work if currency format has no decimal
(like JPY) and doesn't work either if the currency symbol is unicode
This change fixes both issues and adds a unit test
Commits
-------
601d462 [Form] Added getValidators() to Form class
Discussion
----------
[Form] Added getValidators() to Form class
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
I am working implementing client side validation in a bundle that adds validation rules via a form builder. I am currently unable to pull the validators from the Form class which is making the implementation incredibly difficult.
I noticed that the transformers are currently exposed and based on some feedback I got on IRC I am guessing these weren't exposed simply because no one needed them at the time.
---------------------------------------------------------------------------
by mlively at 2011-12-28T20:54:41Z
Side note: It would be incredibly helpful to have this in the current branch of symfony. I implemented it on master because it is a new feature, but I would be more than happy to patch it into 2.0 if you would like.
---------------------------------------------------------------------------
by canni at 2012-01-11T10:15:46Z
👍
As this is new feature it cannot fit into 2.0 series, but 2.1 is just few clicks ahead, maybe this feature will fit into ;)
---------------------------------------------------------------------------
by bschussek at 2012-01-31T08:23:05Z
ping @fabpot
👍
---------------------------------------------------------------------------
by stof at 2012-01-31T08:51:26Z
@mlively could you rebase your branch ? it conflicts with the current master
---------------------------------------------------------------------------
by mlively at 2012-01-31T21:38:39Z
Yes, I can do that might be later today.
Commits
-------
2374e54 Break paths in exceptions hard with css if necessary
Discussion
----------
Exceptions: Break source-paths with CSS
Sometimes in exceptions absolute paths of files are pretty long and need more than one line.
eg.: `/Volumes/Macintosh HD/Users/blabla/Sites/project/files/src/SomeProjectsName/SomeProjectsNameFrontendBundle/Controller/CreateController.php`
This should be displayed within the width of the `h1`.
Using CSS `word-break: break-all;`.
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
A new ExecutionContext is now created everytime that GraphWalker::walkConstraint() is
launched. Because of this, a validator B launched from within a validator A can't break
A anymore by changing the context.
Because we have a new ExecutionContext for every constraint validation, there is no point
in modifying its state anymore. Because of this it is now immutable.
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: -
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3156)
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
-------
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
![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue1919)
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.
The regexp in MoneyType doesn't work if currency format has no decimal
(like JPY) and doesn't work either if the currency symbol is unicode
This change fixes both issues and adds a unit test
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
[![Build Status](https://secure.travis-ci.org/odolbeau/symfony.png)](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
-------
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
ad (1): HTML4 "id" attributes are limited to strings starting with a letter and containing only letters, digits, underscores, hyphens, periods and colons.
ad (2): Property paths contain three special characters needed for correct parsing: left/right bracket and period.
The rules for form naming are:
* Names may start with a letter, a digit or an underscore. Leading digits or underscores will be stripped from the "id" attributes.
* Names must only contain letters, digits, underscores, hyphens and colons.
* Root forms may have an empty name.
Solves #1919 and #3021 on a wider scope.
Commits
-------
e37783f [DoctrineBridge] Refactored the query sanitization in the collector
3b260d2 Refactored the collector to separate the loggers per connection
Discussion
----------
Doctrine collector
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes (for the end user, it will require deleting old profiler data)
Symfony2 tests pass: yes ![Build Status](https://secure.travis-ci.org/stof/symfony.png?branch=doctrine_collector)
This refactors the Doctrine collector to allow implementing doctrine/DoctrineBundle#7
The first commit splits the logging of queries per connection to be able to know which connection was used instead of using a shared stack.
The second commit refactors the sanitation of the parameters to apply the DBAL conversion and then keep the param whenever possible (i.e. when we are sure it is serializable). Such queries will then be explainable in the profiler as we will be able to use the parameters again. Due to the way PDO works, the only cases where we would get an unexplainable queries due to the parameters are queries using a LOB parameter (as it is a resource) or broken queries (passing an object to PDO for instance). And this second case does not make sense to explain the query of course.
---------------------------------------------------------------------------
by stof at 2012-01-23T12:32:16Z
Merging this PR should be synchronized with the DoctrineBundle PR due to the BC break in the collector