merged branch jfsimon/preg-bytes-conversion (PR #7413)

This PR was merged into the master branch.

Commits
-------

3674c22 changed bytes conversion method

Discussion
----------

Changed bytes conversion method

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes

**The old way:**

```php
switch (strtolower(substr($memory, -1))) {
    case 'g':
        $memory *= 1024;
    case 'm':
        $memory *= 1024;
    case 'k':
        $memory *= 1024;
}
```

**The new way:**

```php
if (preg_match('#^(\d+)([bkmgt])#i', $memory, $match)) {
    $shift = array('b' => 0, 'k' => 10, 'm' => 20, 'g' => 30, 't' => 40);
    $memory = ($match[1] * (1 << $shift[strtolower($match[2])]));
}
```

---------------------------------------------------------------------------

by bendavies at 2013-03-18T16:27:52Z

pretty unreadable, no?

---------------------------------------------------------------------------

by benja-M-1 at 2013-03-18T16:29:25Z

I agree, I would not like to have to debug it.

---------------------------------------------------------------------------

by pborreli at 2013-03-18T16:31:43Z

just for my culture, what does : `1 << $var` ?

---------------------------------------------------------------------------

by bendavies at 2013-03-18T16:33:23Z

@pborreli it's a left shift http://php.net/manual/en/language.operators.bitwise.php

---------------------------------------------------------------------------

by jfsimon at 2013-03-18T16:47:15Z

@bendavies @benja-M-1 it's concise and easily recognised (if you understood it the first time).
FYI I didn't find it myself... pretty clever isn't it?

---------------------------------------------------------------------------

by benja-M-1 at 2013-03-18T16:50:53Z

Clearly too much clever for me :)

And what about moving this code in its own class to avoid the copy/paste?

---------------------------------------------------------------------------

by jfsimon at 2013-03-18T16:52:51Z

@benja-M-1 It would add a dependency to the components using it :(

---------------------------------------------------------------------------

by bendavies at 2013-03-18T16:55:26Z

@jfsimon clever indeed, but not necessarily better!

---------------------------------------------------------------------------

by jfsimon at 2013-03-18T16:57:18Z

@bendavies that's true.

---------------------------------------------------------------------------

by Tobion at 2013-03-18T17:00:56Z

There are other places where it could be used too (e.g. FileValidator).

---------------------------------------------------------------------------

by bendavies at 2013-03-18T17:06:01Z

on the other side of the argument, i *hate* the sneaky fall through on the switch statement.
very confusing the first time you see it!

---------------------------------------------------------------------------

by bendavies at 2013-03-18T17:19:42Z

this method has already made it into symfony here: https://github.com/symfony/symfony/pull/7395

---------------------------------------------------------------------------

by jfsimon at 2013-03-19T08:16:19Z

@Tobion I have some questions about the `FileValidator`:
* Why is th `k` in lower case and the `M` in upper case?
* Why is the size divided by 1000 and not 1024?

---------------------------------------------------------------------------

by Tobion at 2013-03-19T08:30:23Z

I was wondering the same. I guess this config (which is also displayed to users) uses the official metric prefixes (k = kilo, M = mega). So it's not about the computer terms where 1 KB = 1024 byte.

---------------------------------------------------------------------------

by vicb at 2013-03-19T16:03:21Z

kB =1000, kiB=1024.
Imo regexps should be case insensitive and account for the "i".
I am not in favor of the changes in this pr (the current way is also documented on php.net fwiw)
[HttpKernel] made the Kernel class more flexible (closes #7396)
Merge branch '2.1' into 2.2

* 2.1:
  updated VERSION for 2.0.23
  update CONTRIBUTORS for 2.0.23
  updated CHANGELOG for 2.0.23
  [Form] fixed failing test
  [DomCrawler] added support for query string with slash
Merge branch '2.2'

* 2.2: (70 commits)
  change wrapped exception message to be more usefull
  updated VERSION for 2.0.23
  update CONTRIBUTORS for 2.0.23
  updated CHANGELOG for 2.0.23
  [Form] fixed failing test
  [DomCrawler] added support for query string with slash
  Fixed invalid file path for hiddeninput.exe on Windows.
  fix xsd definition for strict-requirements
  [WebProfilerBundle] Fixed the toolbar styles to apply them in IE8
  [ClassLoader] fixed heredocs handling
  fixed handling of heredocs
  Add a public modifier to an interface method
  removing xdebug extension
  [HttpRequest] fixes Request::getLanguages() bug
  [HttpCache] added a test (cached content should be kept after purging)
  [DoctrineBridge] Fixed non-utf-8 recognition
  [Security] fixed HttpUtils class tests
  replaced new occurences of 'Request::create()' with '::create()'
  changed sub-requests creation to '::create()'
  fixed merge issue
  ...

Conflicts:
	src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php
	src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar.html.twig
	src/Symfony/Component/DomCrawler/Link.php
	src/Symfony/Component/Translation/Translator.php
[HttpFoudantion] fixed Request::getPreferredLanguage()
Update .travis.yml (closes #7355)

Add extension apc.so to Travis-CI testing
add missing typehint to RouterController
merged branch Tobion/typhint-routing-profiler (PR #7377)

This PR was submitted for the 2.2 branch but it was merged into the master branch instead (closes #7377).

Commits
-------

a58a443 [WebProfiler] add missing typehint to RouterController

Discussion
----------

[WebProfiler] add missing typehint to RouterController

It must be a RouteCollection because TraceableUrlMatcher expects it anyway.

| Q             | A
| ------------- | ---
| Bug fix?      | [no]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes]
| License       | MIT
merged branch jfsimon/issue-7378 (PR #7438)

This PR was merged into the 2.1 branch.

Commits
-------

c928ddc [HttpFoudantion] fixed Request::getPreferredLanguage()
839c78a Revert "merged branch jfsimon/issue-6928 (PR #7378)"

Discussion
----------

[HttpFoundation] fixed Request::getPreferredLanguage()

Previous PR #7378 was wrong and adding BC break. Resolution for short languages codes is now done in `Request::getPreferredLanguage()` method.

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7378
[HttpKernel] moved the Container compilation for more flexibility
[DependencyInjection] added a way to automatically update scoped services

A service can now be marked as synchronized; when set, all method calls
involving this service will be called each time this service is set.
When in a scope, methods are also called to restore the previous version of the
service.
[FrameworkBundle] made the Request service synchronized

This change allows any service to depend on the Request (via a method
call) and always have the right Request instance without the need for
the service to be in the request scope (you still need to set the
Request reference as non-strict).
[HttpKernel] ensured that the Request is null when outside of the Request scope
fixed Request management for RequestListener
fixed Request management for LocaleListener
fixed Request management for FragmentHandler
[FrameworkBundle] added some functional tests
[HttpKernel] added a safeguard for when a fragment is rendered outside the context of a master request
Added unit tests to Dumper
merged branch eddiejaoude/master (PR #7144)

This PR was submitted for the master branch but it was merged into the 2.1 branch instead (closes #7144).

Commits
-------

448983c [YAML] Added unit tests to Dumper

Discussion
----------

[YAML] Added unit tests to Dumper

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | []
| License       | MIT
| Doc PR        | no

---------------------------------------------------------------------------

by stof at 2013-02-21T11:28:55Z

I don't like the fact that you are adding a getter for the only purpose of reaching 100% coverage (which could be achieved differently by checking that the dumper can indeed use 8 spaces when dumping)

---------------------------------------------------------------------------

by eddiejaoude at 2013-02-21T11:33:03Z

Ok, fair point, I will amend.

---------------------------------------------------------------------------

by eddiejaoude at 2013-02-21T11:35:14Z

I also thought of using reflection for the private property, as checking 8 space dump is less of a unit test as using multiple methods, thoughts?

---------------------------------------------------------------------------

by eddiejaoude at 2013-02-21T13:42:30Z

Another way to look at it, is if the property has a 'setter' why should it not have a 'getter' too? i.e. If the developer can 'set' it, why cant they 'get' it too. Just another thought, once the best way to move forward is confirmed, I will update my other tests accordingly & submit them.

---------------------------------------------------------------------------

by Baachi at 2013-02-21T13:49:25Z

Another solution would be, to extend the `Dumper` class and move the `getIndentation` to this class. This class should be located into the `tests/` folder.

@stof What do you think?

---------------------------------------------------------------------------

by stof at 2013-02-21T14:21:54Z

@Baachi IMO, the unit test should ensure that we can actually change the indentation of the dumped code (which is what we want to do). We don't bother about being able to get the indentation (we don't even have a method for it currently), we want it to be applied. The Dumper is not a configuration object. It is an object doing some work.

So testing that a new getter returns the value will not ensure that changing the indentation is working.

---------------------------------------------------------------------------

by Baachi at 2013-02-21T15:07:23Z

@stof Ah yes, I understand you. So my solution is wrong, @eddiejaoude should call `setIndentation` and check the dumped yaml` if the string has the right indentation.

---------------------------------------------------------------------------

by eddiejaoude at 2013-02-22T07:35:08Z

Ok, thanks for the clarification guys. I will get on the case shortly!