Commits
-------
5f22268 [Profiler] Sync with master
1aef4e8 Adds collecting info about request method and allowing searching by it
Discussion
----------
[WebProfiler] Add ability to filter data by request method
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #1515
For discussion & description checkout: #1515 & #2279
---------------------------------------------------------------------------
by fabpot at 2011/12/11 10:02:41 -0800
After merging this PR, the toolbar is not displayed anymore for me.
---------------------------------------------------------------------------
by stof at 2011/12/12 14:18:20 -0800
@fabpot the toolbar works for me using this branch
Commits
-------
6548354 fixed data-url
Discussion
----------
[WebProfilerBundle] fixed and adjusted HTML5 markup
I corrected some markup errors that I found when validating the pages of the WebProfilerBundle.
Along the way I also improved the semantic structure of HTML5 like table header and body, lang attribute.
Removed type="text/css" that is the default with rel="stylesheet". Also no need for media="screen"!? Otherwise style does not apply when debugging with handheld device or when printing.
---------------------------------------------------------------------------
by fabpot at 2011/12/12 23:37:15 -0800
@Tobion: Can you squash your commit before I merge your PR? Thanks.
---------------------------------------------------------------------------
by Tobion at 2011/12/13 03:14:51 -0800
@fabpot I would appreciate if you could do this.
I see two problems with pull requests on @github that occur again and again. It's pretty annoying compared to the otherwise very user-friendly Github.
1. Squashing commits of a pull request: If you've already pushed commits to GitHub, and then squash them locally, you will not be able to push that same branch to GitHub again. So you need to create a new branch and a new pull request.
So there should be a button on Github that simply squashes all commits and allows you to enter a new commit message.
2. Opening a pull request based on the master branch instead of the 2.0 branch where bug fixes should be made. So people must rebase their stuff and open a new pull request again. All this back and forth is taking time unnecessarily (both for admins and contributors) and cluttering Githubs news feed.
There should be the possibility to allow switching the pull request base branch. Or at least give the users a configurable hint about the best practice of contributing to a specific repo when they open a pull request.
---------------------------------------------------------------------------
by henrikbjorn at 2011/12/13 03:16:10 -0800
@Tobion
1. Solved by doing a git push -f remote_name branch_name
2. Yes here you need to open a new PR
---------------------------------------------------------------------------
by fabpot at 2011/12/13 03:21:47 -0800
@Tobion: I'm more than aware of these issues but unfortunately, there is nothing I can do if we want to continue using the Github PRs (and automatic closing).
---------------------------------------------------------------------------
by Tobion at 2011/12/13 03:51:47 -0800
That's why I hope that @github will provide a convenient solution to these issues.
---------------------------------------------------------------------------
by stof at 2011/12/13 04:08:07 -0800
@Tobion send a feature request to github. Commenting here will not make them implement it
---------------------------------------------------------------------------
by Tobion at 2011/12/13 04:18:31 -0800
@fabpot I squashed commits.
@stof I will do it. But there is no public issue tracker for the Github software, is there? So need to use the contact form I suppose.
fixed markup: <pre> not valid inside <p>
adjusted base html structure for HTML5
improved table markup in bag.html.twig
improved table markup in results.html.twig
update exception.html.twig
Commits
-------
d97d7e9 Added a check to see if the type is a string if it's not a FormTypeInterface
Discussion
----------
Add exception on wrong type
When you forget to extend AbstractType in your form type, and then try to create a named builder from it, the error message is quite confusing:
Expected argument of type "string", "Samson\InvoiceBundle\Form\Type\PaymentTermsType" given (from the getType() method)
This PR checks for the right type at the relevant place
---------------------------------------------------------------------------
by stloyd at 2011/12/13 03:00:29 -0800
IMO you should add an test for this.
---------------------------------------------------------------------------
by Burgov at 2011/12/13 03:11:50 -0800
@stloyd done
---------------------------------------------------------------------------
by fabpot at 2011/12/13 03:27:08 -0800
@Burgov: Looks good to me. Can you squash your commits before I merge this PR? Thanks.
---------------------------------------------------------------------------
by Burgov at 2011/12/13 03:29:00 -0800
@fabpot done!
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
Commits
-------
41872cd [Security] added SecurityContextInterface::getUser()
Discussion
----------
[Security] added SecurityContextInterface::getUser()
This changes helps the common use case of fetching the current user and better complies with the [Law of Demeter][1].
Before (still works):
$token = $context->getToken();
$user = $token ? $token->getUser() : null;
After:
$user = $context->getUser();
The fine print:
```
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: ~
Todo: ~
```
[1]: http://en.wikipedia.org/wiki/Law_of_Demeter
Commits
-------
2c3e9ad [DependencyInjection] Made the reference case insensitive
Discussion
----------
[DependencyInjection] Made the reference case insensitive
The container is case insensitive so using capital letters in a reference
made it fail in some cases when checking the dependencies.
Closes#2807
Replaces #2811 by targeting the good branch
This changes helps the common use case of fetching the current user and better complies with the Law of Demeter (http://en.wikipedia.org/wiki/Law_of_Demeter).
Before (still works):
$token = $context->getToken();
$user = $token ? $token->getUser() : null;
After:
$user = $context->getUser();
