Commits
-------
6e98730 added forwards compatibility for the Filesystem component
Discussion
----------
added forwards compatibility for the Filesystem component
see #2949 and #2946
---------------------------------------------------------------------------
by willdurand at 2011/12/25 05:24:01 -0800
👍
---------------------------------------------------------------------------
by ManuelAC at 2011/12/25 06:38:44 -0800
👍
---------------------------------------------------------------------------
by NAYZO at 2011/12/25 07:45:55 -0800
👍
---------------------------------------------------------------------------
by avalanche123 at 2011/12/25 09:52:59 -0800
I think extracting interface and committing it to both branches is preferable.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/25 12:17:59 -0800
what interface?
---------------------------------------------------------------------------
by avalanche123 at 2011/12/25 12:19:03 -0800
FilesystemInterface
---------------------------------------------------------------------------
by lsmith77 at 2011/12/25 12:22:31 -0800
an interface doesn't really relate to this PR .. aka its a separate topic. the purpose of this PR is to make it easier to write code targeting with 2.0 and 2.1 with minimal impact.
---------------------------------------------------------------------------
by willdurand at 2011/12/25 12:23:51 -0800
Seems overkill to add an interface here. I dont think there are different implementations.
Le 25 déc. 2011 à 21:19, Bulat Shakirzyanov<reply@reply.github.com> a écrit :
> FilesystemInterface
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2956#issuecomment-3271576
---------------------------------------------------------------------------
by avalanche123 at 2011/12/25 12:28:49 -0800
@lsmith77 point taken, makes sense
@willdurand there are plenty of alternatives to default filesystem (databases, s3, etc.)
---------------------------------------------------------------------------
by willdurand at 2011/12/25 12:52:58 -0800
Gaufrette (KnpLabs) is suitable for what you describe. I don't know if this component is designed to handle various adapters.
This is just a layer for files manipulations on disk.
Le 25 déc. 2011 à 21:28, Bulat Shakirzyanov<reply@reply.github.com> a écrit :
> @lsmith77 point taken, makes sense
> @willdurand there are plenty of alternatives to default filesystem (databases, s3, etc.)
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2956#issuecomment-3271596
Commits
-------
8235848 [HttpFoundation][File] Add flv file default extension
Discussion
----------
[HttpFoundation][File] Add flv file default extension
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes (doesn't affect tests)
Fixes the following tickets: -
Todo: -
This commit adds default extension for video/x-flv mime-type, which is a Flash Video file. Lack of this definition causes File::guessExtension() return null for this type of file, which breaks uploaded file in some user cases.
Commits
-------
d1fa8cc [WebProfiler] Fix some design glitches (closes#2867)
Discussion
----------
[WebProfiler] Fix some design glitches
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2867
---------------------------------------------------------------------------
by jalliot at 2011/12/17 04:54:11 -0800
Thanks @stloyd.
However it only fixes the second and third issues related in #2867. The filesystem provider bug is much more annoying!
---------------------------------------------------------------------------
by stloyd at 2011/12/17 05:38:13 -0800
@jalliot Did you cleaned profiler dir after update to latest master commit ? This is mentioned and kinda known BC break... I'm asking because I cannot reproduce this issue.
---------------------------------------------------------------------------
by jalliot at 2011/12/17 08:17:21 -0800
@stloyd It has nothing to do with your recent PR adding HTTP method as I had the bug even before that.
I've tested on a fresh Symfony (master) install (before and after the merge) and have the bug each time.
I thought maybe it was related to some of the bundles I installed but the bug occurs even in a freshly installed SE.
Commits
-------
e417153 [BugFix][Console] Fix type hint for output formatter
Discussion
----------
[BugFix][Console] Fix type hint for OutputFormatter in OutputStream constructors
I consider this as a bug, cause it disables ability to change formatter implementation
BugFix: yes
Feature addition: no
Sysmfony2 test pass: yes
---------------------------------------------------------------------------
by fabpot at 2011/12/18 00:23:05 -0800
It's not a bug fix as existing code that extend these classes will now fail.
---------------------------------------------------------------------------
by stof at 2011/12/18 03:51:38 -0800
@fabpot does the constructor enforces its signature for child classes ? I don't think so.
Commits
-------
1e370d7 typo fix
93d8d44 added some more infos about Config
27efd59 added READMEs for the bridges
34fc866 cosmetic tweaks
d6af3f1 fixed README for Console
6a72b8c added basic README files for all components
Discussion
----------
added basic README files for all components and bridges
heavily based on http://fabien.potencier.org/article/49/what-is-symfony2 and the official Symfony2 documentation
---------------------------------------------------------------------------
by jmikola at 2011/11/03 13:36:07 -0700
Great work. For syntax highlighting on the PHP snippets, you could add "php" after the three backticks.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/03 13:41:29 -0700
done
---------------------------------------------------------------------------
by stealth35 at 2011/11/03 13:49:31 -0700
Nice job, but you also need to add `<?php`
ex :
``` php
<?php
use Symfony\Component\DomCrawler\Crawler;
$crawler = new Crawler();
$crawler->addContent('<html><body><p>Hello World!</p></body></html>');
print $crawler->filter('body > p')->text();
```
---------------------------------------------------------------------------
by lsmith77 at 2011/11/03 13:56:57 -0700
done
---------------------------------------------------------------------------
by ericclemmons at 2011/11/03 19:57:57 -0700
@lsmith77 Well done! This makes consumption of individual components that much easier, *especially* now that `composer.json` files have been added.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/04 01:18:23 -0700
ok .. fixed the issues you mentioned @fabpot
---------------------------------------------------------------------------
by lsmith77 at 2011/11/11 15:00:27 -0800
@fabpot anything else left? seems like an easy merge .. and imho there is considerable benefit for our efforts to spread the word about the components with this PR merged.
---------------------------------------------------------------------------
by drak at 2011/11/11 18:54:13 -0800
You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
---------------------------------------------------------------------------
by lsmith77 at 2011/11/12 00:59:14 -0800
i did that in some. but i might have missed a few places.
On 12.11.2011, at 03:54, Drak <reply@reply.github.com> wrote:
> You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2561#issuecomment-2715762
---------------------------------------------------------------------------
by breerly at 2011/11/21 10:28:36 -0800
Pretty excited with this.
---------------------------------------------------------------------------
by dbu at 2011/11/24 00:02:50 -0800
is there anything we can help with to make this ready to be merged?
---------------------------------------------------------------------------
by lsmith77 at 2011/12/18 02:39:23 -0800
@fabpot: seriously .. if you are not going to deliver something "better" and don't provide a reason what is wrong with this .. then its beyond frustrating. i obviously do not claim that these README's are perfect (and certainly still no replacement for proper documentation), but I do claim that in their current form they are a radical step forward to potential users of the Symfony2 components.
Commits
-------
5c41ec9 [HttpKernel][Client] Only simple (name=value without any other params) cookies can be stored in same line, so lets add every as standalone to be compliant with rfc6265
Discussion
----------
[HttpKernel][Client] Set cookie fix
Bug fix: yes
Feature addition: no
Backwards compatibility break: no(?)
Symfony2 tests pass: yes
Fixes the following tickets: #2881
Only simple cookies can be stored in same line:
* Used by now (__wrong__): `Set-Cookie: name1=value, name2=value`
* Proper according to RFCs: `Set-Cookie: name1=value; name2=value`
So lets add every as standalone ([next header](http://tools.ietf.org/html/rfc6265#section-3.1)) to be compliant with [RFC6265](http://tools.ietf.org/html/rfc6265). This fixes#2881.
After the changes in #2733, the parameters to Doctrine queries were
always shown as 'Array' in the profiler. This commit puts back the
yaml_encode that is still needed after all.
Commits
-------
62f3dc4 [SecurityBundle] Changed environment to something unique.
Discussion
----------
[SecurityBundle] Fix name clash with functional tests
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Functional tests need a unique environment or it will produce class redeclare errors when multiple bundles are functional tested at the same time.
Previously, Boolean defaults were printed as strings, which lead to true and false being printed as "1" and "", respectively. With this change, they are now printed as "true" and "false".
Commits
-------
f3e92c4 [TwigBundle] Fix the exception message escaping
Discussion
----------
[2.0][TwigBundle] Fix exception new line
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Commits
-------
4d64d90 Allow empty result; change default *choices* value to **null** instead of **array()**. - added *testEmptyChoicesAreManaged* test - `null` as default value for choices. - is_array() used to test if choices are user-defined. - `null` as default value in __construct too. - `null` as default value for choices in EntityType.
Discussion
----------
[Doctrine][Bridge] EntityType: Allow empty result; default `choices` value changed to null
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
References the following tickets: #2504
- added *testEmptyChoicesAreManaged* test
- `null` as default value for choices.
- is_array() used to test if choices are user-defined.
- `null` as default value in __construct too.
- `null` as default value for choices in EntityType.
I squashed commits from PR #2504 as requested.
- added *testEmptyChoicesAreManaged* test
- `null` as default value for choices.
- is_array() used to test if choices are user-defined.
- `null` as default value in __construct too.
- `null` as default value for choices in EntityType.
Commits
-------
cd24fb8 change explode's limit parameter based on known variable content
b3cc270 minor optimalisations for explode
Discussion
----------
[FrameworkBundle][CssSelector][HttpFoundation][HttpKernel] [Security][Validator] Minor optimizations for "explode" function
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
I added limit parameter in some places, where it may be usefull. I did not check the context of what values may have been exploded. So to not break anything, I added +1 to limit parameter.
If you find out that in some places limit (or limit+1) is not important or meaningless, write a comment please and I will fix it.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 06:56:49 -0800
Adding +1 just to be sure to not break anything is clearly something we won't do. What is the benefit of doing that anyway?
---------------------------------------------------------------------------
by pulzarraider at 2011/12/07 13:50:24 -0800
The main idea of making this PR was to notify about some places that may run faster with just adding one parameter to explode function.
If in code is someting like: ```list($a, $b) = explode(':', $s);```
Function ```explode``` will create n-items (depends on ```$s```), but we need in code only the first two items. There is no reason to let ```explode``` create more items in memory that are NEVER used in our code. The limit parameter is there for these situations, so let's use it.
I know that it is microoptimization and may look unimportant, but we are writing a framework - so people expect that code will be as fast as possible without this kind of mistakes.
As I've noticed above, I know that +1 is not ideal solution, but the fastest without debugging the code. I expect that someone (with good knowledge of that code) will look at it and write in comments if variable may contain 1 comma (dot or someting on what is doing the explode) or maybe 2 in some situations or more.
Anyway, +1 will not break anything, because same items are created as it is now, but no unnecessary item is created.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 23:14:59 -0800
I'm +1 for adding the number to avoid problems but I'm -1 on the optimization side of things as it won't optimize anything.
---------------------------------------------------------------------------
by helmer at 2011/12/08 12:46:49 -0800
*.. The main idea of making this PR was to notify about some places that **may** run faster ..*
I am also unsure the optimization is really an optimization, care to benchmark (with meaningful inputs)? As for the limit+1 thing, why would you want to +1 it? The number of ``list`` arguments should always reflect the ``limit`` parameter, no?
---------------------------------------------------------------------------
by pulzarraider at 2011/12/08 23:11:34 -0800
@helmer please try this simple benchmark:
```
<?php
header('Content-Type: text/plain; charset=UTF-8');
define('COUNT', 10000);
$source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc:dddddddddddddddddddddd:eeeeeeeeeeeeeeeeeeeeeeeee:fffffffffffffffffffffffffff';
$start = microtime(true);
for ($i = 0; $i < COUNT; $i++) {
list($a, $b) = explode(':', $source_string);
}
$end = microtime(true)-$start;
echo 'without limit: '.$end."\n";
$start = microtime(true);
for ($i = 0; $i < COUNT; $i++) {
list($a, $b) = explode(':', $source_string, 2);
}
$end = microtime(true)-$start;
echo 'with limit: '.$end."\n";
```
My results are:
```
without limit: 0.057228803634644
with limit: 0.028676986694336
```
That is 50% difference (with APC enabled). Of course the result depends on the length of source string and if it's too short, the difference may be none or very very small. That's why I said, that it **may** run faster and is just a micro optimization.
---------------------------------------------------------------------------
by pulzarraider at 2011/12/08 23:18:12 -0800
@helmer And why +1? It depends on a code:
```
$source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc';
list($a, $b) = explode(':', $source_string, 2);
var_dump($a, $b);
```
and
```
$source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc';
list($a, $b) = explode(':', $source_string, 3);
var_dump($a, $b);
```
gives different results. That's why the content of the variable must be known.
---------------------------------------------------------------------------
by helmer at 2011/12/09 00:08:28 -0800
@pulzarraider Thanks for the benchmark, seems like a gain enough. Although, we are more likely having a scenario of:
``explode(':', 'a🅱️c')`` vs ``explode(':', 'a🅱️c', 3)`` with a ``COUNT`` of 10, where the difference is not even in microseconds anymore :)
The limit addition alters the behaviour though, ie suddenly you can define a controller [logical name](http://symfony.com/doc/current/book/routing.html#controller-string-syntax) as ´´AcmeBlogBundle:Blog:show:something``, and things go downhill from there on.
All that aside, I'm +1 for setting the limit to the exact number of ``list`` parameters, but certainly not number+1, this is just too wtfy (as you said, this was a safety thing, but I reckon for this PR to be merged it needs to be +0).
---------------------------------------------------------------------------
by drak at 2011/12/09 08:28:58 -0800
Overall `list()` is ugly as it's not very explicit. Even though it would mean extra lines, it's better to `explode()` then explicitly assign variables:
```
$parts = explode(':', $foo);
$name = $parts[0];
$tel = $parts[1];
```
`list()` is one of those bad relics from the PHP past...
---------------------------------------------------------------------------
by fabpot at 2011/12/11 10:07:47 -0800
@drak: why is `list` not explicit? It is in fact as explicit as the more verbose syntax you propose.
---------------------------------------------------------------------------
by pulzarraider at 2011/12/11 13:08:50 -0800
@drak: I agree with @fabpot. In speech of benchmarks ```list``` is faster then using a helper variable.
@fabpot, @helmer I've changed explode's limit to be correct (without +1) and removed some changes from this PR, where I can't find out what the content of variable may be. Unit tests pass, so I think it's ready for merge.
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
