Commits
-------
73ac773 [Config] added ability to set info message and example to node definition
Discussion
----------
[2.1] [Config] added ability to set info message to node definition
The configuration TreeBuilder lends itself to be hooked into for reference documentation generation. This ``setInfo()`` method allows the addition of a message to a node for use in doc generation.
Example (``Symfony/Bundle/WebProfilerBundle/DependencyInjection/Configuration.php``):
```php
<?php
// ...
$treeBuilder = new TreeBuilder();
$rootNode = $treeBuilder->root('web_profiler');
$rootNode
->children()
->booleanNode('verbose')->defaultTrue()
->setInfo('Setting to false hides some secondary information to make the toolbar shorter.')->end()
->booleanNode('toolbar')->defaultFalse()
->setInfo('Enable/Disable the display of the web debug toolbar containing a summary of the profiling data.')->end()
->booleanNode('intercept_redirects')->defaultFalse()
->setInfo('Intercepts the redirects and gives you the opportunity to look at the collected data before following the redirect.')->end()
->end()
;
return $treeBuilder;
// ...
```
I think a core console command would be great (ie: ``config:reference:dump web_profiler``) or even a frame in the profiler. The way bundle configuration processing is not enforced makes this difficult currently. Perhaps adding a ``getConfiguration()`` method to ``ExtensionInterface``. Thoughts?
Certainly this change would allow for a third party bundles or sites (ie symfony.com) to generate bundle reference docs.
---------------------------------------------------------------------------
by Seldaek at 2011/05/25 10:36:10 -0700
👍 finally some effort in this direction, although as @schmittjoh said without generation this won't bring us very far. But if it allows everyone to document their stuff already, I think it's still a plus, otherwise we'll have generation without content. And content without generation is still useful for people looking at the sources.
---------------------------------------------------------------------------
by Problematic at 2011/05/25 10:52:42 -0700
+1, even if the only thing it does is save me some WTFs later looking through my own code.
---------------------------------------------------------------------------
by weaverryan at 2011/05/25 10:59:25 -0700
@kbond and I talked about this a bit over the weekend and decided that he should at least get this first step going. It *is* of limited use in its current state (thought @Problematic and @Seldaek bring up a good points), but we need some ideas on where to go next.
As @kbond says, nothing really ties the DI extension to a config class now. Should we add a `getConfiguration()` or `getTreeBuilder()`? How can we make it so that we *know* which `TreeBuilder` to use for each DI extension config alias?
---------------------------------------------------------------------------
by lsmith77 at 2011/05/25 11:21:05 -0700
I think this is great and I dont think we need to require a generator if the API makes sense.
One thing the API should also cover is setting a custom error message in case of a validation error. Just something to keep in mind.
---------------------------------------------------------------------------
by stof at 2011/05/25 11:26:05 -0700
@weaverryan I think that ``getConfiguration`` would be better than ``getTreeBuilder`` as this is still the method implemented by the ``ConfigurationInterface``. Of course this method should be optionnal (defaulting to return ``null``)
---------------------------------------------------------------------------
by kbond at 2011/05/25 13:09:26 -0700
The ``ConfigurationInterface`` API would need to be locked down more. For example, FrameworkBundle's ``Configuration`` class has a constructor. I am afraid if we lock it down too much we could lose its flexibility.
---------------------------------------------------------------------------
by stof at 2011/05/25 13:24:51 -0700
@kbond the constructor **cannot** be enforced by an interface. This is why I was talking about adding a ``getConfiguration`` method in the extension. This way, the extension can do whatever it wants to instantiate the Configuration, passing whatever param you want.
---------------------------------------------------------------------------
by kbond at 2011/05/25 14:12:03 -0700
Sorry meant the ``ExtensionInterface``. I would like the ``getConfiguration`` method enforced. How would we tackle the FrameworkBundle? To retrieve it's configuration you need the ``ContainerBuilder``.
---------------------------------------------------------------------------
by kbond at 2011/05/25 14:38:02 -0700
The only way I can see the bundle method working is if the method was manually overridden for each bundle (returns null by default). The fact that ``Configuration`` classes may or may not have constructors prevent it from being automated.
---------------------------------------------------------------------------
by kbond at 2011/05/26 06:24:37 -0700
I think we should avoid *searching* if at all possible.
The only real thing you might need to build your configuration is the ``ContainerBuilder``. What about passing it to the ``Extension`` class constructor? Then it could be accessed in a ``getConfiguration()`` method. Looking at the internals, I can't see how I could do that but thought I would throw it out there.
---------------------------------------------------------------------------
by stof at 2011/05/26 06:31:46 -0700
The constructor is not a good idea IMO as the ContainerBuilder passed to an extension is not the main one when using the ``load`` method. So passing it in the constructor is bad IMO. Passing it to the ``getConfiguration`` method would be better IMO.
---------------------------------------------------------------------------
by kbond at 2011/05/26 08:04:08 -0700
Based on the comments I am seeing 2 ways of implementing a ``getConfiguration()`` method. I have started branches for both ways:
1. ``getConfiguration()`` in Bundle class: 3fb1c889af
2. ``getConfiguration()`` in Extension class: cf05ec20fc
I have updated the ``FrameworkBundle`` in both.
On another note, I think one ``Configuration`` class per Bundle should be enforced. Any reason why we can't do this? The only core bundle that has more than one is ``SecurityBundle`` and I don't get why it needs two (not saying it shouldn't, I just don't know why).
---------------------------------------------------------------------------
by stof at 2011/05/26 08:15:52 -0700
One configuration class per bundle does not make any sense as each configuration class is responsible to merge an array of configurations. So a bundle providing several extension (not the common use case at all but supported) would need several configuration classes as each extension would get a different set of configuration.
and if you look at what a Configuration class do, it merges several configurations in an intelligent way and normalizes it. Look at why a second one is used in the SecurityBundle and you will see that this is a perfectly valid use of it. The Config component is not limited to DI extensions configuration.
And for the 2 implementations, I prefer the one putting it in the extension. Here are my reasons:
- a bundle supports having several extensions so returning only one Configuration class for the bundle is broken in this case (for which extension is it ? and what about the others ?)
- the extension is the class using the Configuration, not the bundle. So it is logical to put it in it
- this removes code duplication to instantiate the Configuration as the extension can use the method.
---------------------------------------------------------------------------
by kbond at 2011/05/27 08:41:58 -0700
Created a *rough* initial ``config:dump`` console command. Works for all core bundles. Still needs some work for the more complex Node types.
Had some issues getting it to work with ``AsseticBundle`` and ``SecurityBundle``. There is some code duplication I couldn't figure out how to avoid.
@weaverryan I tried to mimic the YAML format you have in the reference docs.
---------------------------------------------------------------------------
by kbond at 2011/06/02 08:09:57 -0700
I reduced the scope of this PR to just the API to document configuration nodes and access the configuration object from an extension. Like stated initially, if it makes sense we should add it and worry about the generation later.
I am still going to work on a console ``config:dump`` command but in another branch.
---------------------------------------------------------------------------
by stof at 2011/09/04 05:36:09 -0700
@kbond could you rebase your PR ? It conflicts with the current master branch.
Apart from that, the way to add info on the node seems good to me. @schmittjoh what do you think ?
---------------------------------------------------------------------------
by kbond at 2011/09/04 06:59:29 -0700
Will do, give me a few days (on vacation)
---------------------------------------------------------------------------
by kbond at 2011/09/06 08:48:55 -0700
@stof should i squash the commits into 1?
---------------------------------------------------------------------------
by stof at 2011/09/06 09:00:46 -0700
If you want, but it is not necessary here IMO. the history is not a mess as in some other PR with lots of changes.
---------------------------------------------------------------------------
by kbond at 2011/09/06 10:14:32 -0700
Ok, rebased.
---------------------------------------------------------------------------
by lsmith77 at 2011/09/15 07:49:17 -0700
@kbond will you be around for the IRC meeting in 10mins?
---------------------------------------------------------------------------
by kbond at 2011/09/22 08:13:37 -0700
There is a rudimentary config dump command based on this PR available for testing here: https://github.com/kbond/symfony/tree/config_dump_command
---------------------------------------------------------------------------
by stof at 2011/10/16 11:03:57 -0700
@fabpot @schmittjoh ping
---------------------------------------------------------------------------
by stof at 2011/11/11 07:01:12 -0800
@fabpot ping
---------------------------------------------------------------------------
by kbond at 2011/11/22 07:14:58 -0800
I added a new interface as discussed in the irc meeting. Is this ok?
---------------------------------------------------------------------------
by stof at 2011/12/12 12:38:38 -0800
@fabpot this PR conflicts with master once again, needing to rebase it. It would be good to review it so that we don't need to keep it pending for each further change in master
@kbond can you do the rebase ?
---------------------------------------------------------------------------
by kbond at 2011/12/12 19:35:05 -0800
rebased.
---------------------------------------------------------------------------
by fabpot at 2011/12/13 02:56:02 -0800
@kbond: Can you squash your commits before I merge this PR? Thanks.
---------------------------------------------------------------------------
by kbond at 2011/12/13 03:09:09 -0800
@fabpot done.
Commits
-------
7827f72Fixes#2817: ensure that the base loader is correctly initialised
Discussion
----------
[TwigBundle] Ensure base Filesystem loader paths are initialised
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Fixes the following tickets: #2817
Originated from #2817.
Commits
-------
7fadd08 static::$privateField is an OOP non-sense (extending the class is not possible)
Discussion
----------
static::$privateField is an OOP non-sense (extending the class is not possible)
static::$privateField is an OOP non-sense (extending the class is not possible)
Commits
-------
9c1fbb8 [DoctrineBridge] fixed the refreshing of the user for invalid users
Discussion
----------
[DoctrineBridge] fixed the refreshing of the user for invalid users
A user provider is not allowed to return ``null`` when the user is not found.
This bug is the reason why #2845 has been submitted
---------------------------------------------------------------------------
by stof at 2011/12/12 04:47:04 -0800
it closes#2822 btw
Commits
-------
171f2d5 fixed failing test in windows because
Discussion
----------
fixed failing test in windows
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
fixed failing test in windows because
1) PHP_BINDIR is not secure to rely on
2) the assertion doesn't actually check for the suffix as the test implies
Commits
-------
45bba7b Added a hint about a possible cause for why no mime type guesser is be available
Discussion
----------
Added a hint about a possible cause for why no mime type guesser is be available
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Commits
-------
db2d773 [FrameworkBundle] Improve the TemplateLocator exception message
Discussion
----------
Template locator/exception message
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Improve the error message to include the error message from the File Locator which is more accurate : the File Locator might also look in some fallback folder(s) (i.e. `%kernel.root_dir%/Resources`)
Commits
-------
b1ca0cd added several tests to the serializer (mainly for deserialization)
Discussion
----------
Serializer tests
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [](http://travis-ci.org/lsmith77/symfony)
Fixes the following tickets: -
The state of the serializer tests wasn't as bad as I thought.
Was mostly missing tests for some edge cases as well as deserialization.
Once this is merged to 2.0 and master, I will rebase #2530 and make sure the tests still pass.
Commits
-------
40e5b60 [FrameworkBundle] added return type for getContainer()
Discussion
----------
added return type for getContainer()
IDE type inference support
Commits
-------
3759ff0 [Locale] StubNumberFormatter allow to parse 64bit number in 64bit mode
Discussion
----------
[Locale] StubNumberFormatter allow to parse 64bit number in 64bit mode
Bug fix: yes
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: [](http://travis-ci.org/stealth35/symfony)
Fixes the following tickets: #2735
---------------------------------------------------------------------------
by stealth35 at 2011/12/01 06:47:32 -0800
@Seldaek should be better now
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 04:22:42 -0800
@fabpot done
---------------------------------------------------------------------------
by fabpot at 2011/12/02 04:28:24 -0800
Tests do not pas for me (on a Mac):
1) Symfony\Tests\Component\Locale\Stub\StubNumberFormatterTest::testParseTypeInt64StubWith64BitIntegerInPhp64Bit
->parse() TYPE_INT64 does not use true 64 bit integers, using only the 32 bit range.
Failed asserting that 2147483648 matches expected -2147483648.
.../tests/Symfony/Tests/Component/Locale/Stub/StubNumberFormatterTest.php:819
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 04:50:20 -0800
@fabpot, could you send me the return of this code
``` php
<?php
$formatter = new \NumberFormatter('en', \NumberFormatter::DECIMAL);
$value = $formatter->parse('2,147,483,648', \NumberFormatter::TYPE_INT64);
var_dump($value);
$value = $formatter->parse('-2,147,483,649', \NumberFormatter::TYPE_INT64);
var_dump($value);
```
---------------------------------------------------------------------------
by fabpot at 2011/12/02 06:06:21 -0800
int(-2147483648)
int(2147483647)
---------------------------------------------------------------------------
by stealth35 at 2011/12/02 06:10:28 -0800
It's nosens, but the Stub should follow Intl ... so I fix that
---------------------------------------------------------------------------
by stealth35 at 2011/12/11 08:48:25 -0800
It's OK now
Commits
-------
bb0d202 Switched sanitizeParameter() for existing varToString()-method; now always stores a string representation of each parameter
4fe4dfd Fixed vendor version mismatch in tests
28730e9 [DoctrineBridge] Added unit tests
4535abe [DoctrineBridge] Fixed attempt to serialize non-serializable values
Discussion
----------
[DoctrineBridge] Fixed attempt to serialize non-serializable values
Bug fix: yes
Feature addition: no
Backwards compatibility break: no (99%)
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
The Doctrine DBAL type system does not pose any restrictions on the php-types of parameters in queries. Hence one could write a doctrine-type that uses a resource or an `\SplFileInfo` as its corresponding php-type. Parameters of these types are logged in the `DoctrineDataCollector` however, which is then serialized in the profiler. Since resources or `\SplFileInfo` variables cannot be serialized this throws an exception.
This PR fixes this problem (for known cases) by sanitizing the query parameters to only contain serializable types. The `isNotSerializable`-check surely is not complete yet, but more non-serializable classes can be added on a case-by-case basis.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 07:04:43 -0800
Tests do not pass for me.
Furthermore, let's reuse what we already have in the framework (see https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpKernel.php#L187 -- yes you can just copy/paster the existing code).
---------------------------------------------------------------------------
by aboks at 2011/12/09 01:41:14 -0800
@fabpot I fixed the tests (seems I had the wrong vendor versions in my copy) and reused the `varToString()`-code. This introduces a tiny BC break in the rare case that someone writes his own templates for the web profiler (the parameters returned by the data collector are now always a string; could be any type before).
After merging this PR, merging 2.0 into master would give a merge conflict and failing tests (because of the changes related to the introduction of the `ManagerRegistry` interface). To prevent this, please merge #2820 into master directly after merging this PR (so before merging 2.0 into master). After that 2.0 can be cleanly merged into master.
---------------------------------------------------------------------------
by stof at 2011/12/09 03:43:38 -0800
it is not a BC break. Using ``yaml_encode`` on a string will not break the template
