This PR was merged into the master branch.
Commits
-------
005a9a3 [Routing] fixed RouteCompiler for adjacent and nested placeholders
Discussion
----------
[2.2] [Routing] fixed RouteCompiler for adjacent placeholders
Tests pass: yes
Feature addition: no
Fixes: #4215
BC break: no
- Nesting placeholders works now properly, e.g. `/{foo{bar}}`. Only `bar` is a variable, the rest is static text.
- Variables without separator work now correctly too, e.g. `/{w}{x}{y}{z}.{_format}`. All variables will have the correct default regex in the current manner, that is exclude the previous static char and the next static char (which means disregarding the placeholder in between that have no seperator).
As you can see, I have not modified any tests, only added some. So this change is BC. The compiler now works under all conditions and does not fail for such patterns.
---------------------------------------------------------------------------
by Tobion at 2012-05-08T22:34:58Z
ready for review / merging
Thanks @snc for giving a helpful tip.
---------------------------------------------------------------------------
by Tobion at 2012-06-12T23:22:54Z
fabpot told me, he doesn't like PRs that both fix and enhance stuff. So I reworked this PR so that it only contains the bug fixes.
The new PR #4563 contains the feature addition.
---------------------------------------------------------------------------
by Tobion at 2012-06-26T12:33:43Z
ping @fabpot
---------------------------------------------------------------------------
by Tobion at 2012-08-14T16:29:27Z
Why wait for 2.2? It's a bugfix only, without BC break.
---------------------------------------------------------------------------
by Tobion at 2012-08-31T17:04:37Z
@fabpot I think this should go into 2.1
Commits
-------
02516de [Routing] fix variable with a requirement of '0'
1f5b793 [Routing] fix setting empty requirement in Route
Discussion
----------
[Routing] fix setting empty requirement
First commit: A requirement of "^$" was overlooked and wasn't recognized as empty after stripping it in Route.
Second commit: Fixes a requirement of '0' that was ignored by the Compiler.
Commits
-------
be28e56 [Routing] disallow numeric named variables in pattern
Discussion
----------
[Routing] compile check for numeric named variables in pattern
Because PHP raises an error for such subpatterns in PCRE and thus would break matching, e.g. this is not allowed as regex `(?<123>.+)`.
So add a compile time check for a non-working pattern like '/{123}'.
---------------------------------------------------------------------------
by sstok at 2012-09-06T08:31:42Z
Strangely enough Regex buddy gives no warning or error with the pattern.
Is the name all numeric invalid or just the beginning?
1e4 and 0xFF would be perfectly valid but returns true with is_nummeric()
---------------------------------------------------------------------------
by Tobion at 2012-09-06T08:59:07Z
Any numeric is not valid. I guess this limitation is unique to PHP's binding to PCRE.
I think it's because the returned matches array of of preg_match contains both the subpattern as integer index and as named variable. So having a numeric named variable would conflict as `array['1'] === array[1]`.
Commits
-------
7c5cfeb [Routing] added test why #5238 is not that easy
Discussion
----------
[Routing] added test why #5238 is not that easy
This just adds a test that wasn't covered yet and shows why #5238 is not that easy as I thought at first.
Commits
-------
0706d18 [Routing] fixed 4 bugs in the UrlGenerator
Discussion
----------
[Routing] UrlGenerator: fixed missing query param and some ignored requirements
This was pretty hard to figure out. I could fix 4 bugs and refactor the code to safe 2 variables and several assignments. Sorry for doing this in one commit, but they were highly interdependent.
See the added tests for what was fixed. The most obvious bug was that a query param was ignored if it had by accident the same name as a default param (but wasn't used in the path).
In 3 cases it generated the wrong URL that wouldn't match this route. The generator wrongly ignored either the requirements or the passed parameter. I had to adjust one test that was asserting something wrong (see comments).
---------------------------------------------------------------------------
by Tobion at 2012-08-13T14:22:35Z
ping @fabpot
---------------------------------------------------------------------------
by Tobion at 2012-08-29T17:53:07Z
@fabpot I think it's important to merge this before 2.1 final.
* 2.0:
updated VERSION for 2.0.17
updated CHANGELOG for 2.0.17
updated vendors for 2.0.17
fixed XML decoding attack vector through external entities
prevents injection of malicious doc types
disabled network access when loading XML documents
refined previous commit
prevents injection of malicious doc types
standardized the way we handle XML errors
Redirects are now absolute
Conflicts:
CHANGELOG-2.0.md
src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
src/Symfony/Component/DomCrawler/Crawler.php
src/Symfony/Component/HttpKernel/Kernel.php
tests/Symfony/Tests/Component/DependencyInjection/Loader/XmlFileLoaderTest.php
tests/Symfony/Tests/Component/Routing/Loader/XmlFileLoaderTest.php
tests/Symfony/Tests/Component/Serializer/Encoder/XmlEncoderTest.php
tests/Symfony/Tests/Component/Translation/Loader/XliffFileLoaderTest.php
tests/Symfony/Tests/Component/Validator/Mapping/Loader/XmlFileLoaderTest.php
vendors.php
Commits
-------
933e821 Add minimum-stability (dev) in each component
Discussion
----------
Add minimum-stability (dev) in each component
This fixes the ability to run the test suite in each component if a `composer install` is needed.
---------------------------------------------------------------------------
by stof at 2012-08-22T13:57:14Z
If you really want to run the testsuite standalone, some dev requirements are missing (SecurityBundle needs the FrameworkBundle for its functional tests for instance). If you have some time to check the missing dev requirement, it would be great.
Anyway, 👍 for this
---------------------------------------------------------------------------
by willdurand at 2012-08-22T13:59:15Z
Yes I already did that once. I'll try to fix more components later.
On Wed, Aug 22, 2012 at 3:57 PM, Christophe Coevoet <
notifications@github.com> wrote:
> If you really want to run the testsuite standalone, some dev requirements
> are missing (SecurityBundle needs the FrameworkBundle for its functional
> tests for instance). If you have some time to check the missing dev
> requirement, it would be great.
> Anyway, [image: 👍] for this
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/5318#issuecomment-7934886>.
>
>
---------------------------------------------------------------------------
by stof at 2012-08-22T14:02:23Z
Well, I think most components should be good now (as some work has been done on them). But the bridges and bundles may need some work (bundles were not having any dev requirements until yesterday when @guilhermeblanco added some on FrameworkBundle)
---------------------------------------------------------------------------
by pborreli at 2012-08-22T14:14:00Z
what about having for each READ-ONLY repo his own .travis.yml and travisci hook activated ?
---------------------------------------------------------------------------
by fabpot at 2012-08-22T14:30:13Z
please, don't add more travis files. The main already tests everything, and that's all we need.
---------------------------------------------------------------------------
by stof at 2012-08-22T14:33:46Z
@pborreli tests should not be different for subtree split repos as the code is the same and the tests are the same (except that more tests could be skipped because of missing deps).
Note that for the bundles, it is likely to be different currently as I think some skip tests are missing (just like dev requirements are). But fixing this does not require enablign travis.
---------------------------------------------------------------------------
by pborreli at 2012-08-22T14:42:30Z
ok, i was just thinking about a way to be sure each component is usable individually but yeah that would require to relaunch each tests and add a bunch of travis files + hook
---------------------------------------------------------------------------
by hason at 2012-08-24T13:12:04Z
@stof, @eriksencosta, @fabpot: Tests are different for Locale component, see #5235
---------------------------------------------------------------------------
by stof at 2012-08-24T13:35:07Z
@hason no. You also need to do it when running the tests of the Locale component as part of the full run.
Commits
-------
3466896 [Routing] fix encoding of static static, so UrlGenerator produces valid URLs
Discussion
----------
[Routing] fix encoding of static text
Fixes#4166
As requested by Fabien, I split #4205 into multiple PRs.
---------------------------------------------------------------------------
by stof at 2012-06-09T12:49:32Z
github tells me this PR cannot be merged automatically. Could you rebase it ?
---------------------------------------------------------------------------
by Tobion at 2012-06-09T13:12:55Z
Done
---------------------------------------------------------------------------
by travisbot at 2012-06-09T13:18:18Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1576266) (merged 3466896a into 15b5aa4f).
Commits
-------
7464dcd added phpdoc
c413e7b [Routing] remove RequestContextAwareInterface from RequestMatcherInterface
921be34 [Routing] fix phpdoc
Discussion
----------
[Routing] RequestMatcherInterface doesn't need context
Matchers that implement RequestMatcherInterface should match a Request, thus they don't need the request context.
---------------------------------------------------------------------------
by travisbot at 2012-06-14T21:39:48Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1624496) (merged f5ff1fe0 into 7c91ee57).
---------------------------------------------------------------------------
by schmittjoh at 2012-06-15T13:32:59Z
I think it makes sense to remove the RequestContext from the RequestMatcher.
---------------------------------------------------------------------------
by travisbot at 2012-06-15T15:54:28Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1628931) (merged 7464dcd2 into f881d282).
---------------------------------------------------------------------------
by Tobion at 2012-06-26T12:32:06Z
Anything missing?
Commits
-------
49e9957 added test to ensure matching is eager
Discussion
----------
[Routing] added test to ensure matching is eager
This just adds a passing test that wasn't covered yet, so we don't break this scenario in the future.
---------------------------------------------------------------------------
by travisbot at 2012-06-13T01:04:09Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1605738) (merged 49e99572 into 37550d23).
Commits
-------
31843cf [FrameworkBundle] Add info to config
d5ab4c1 [Routing] Update changelog
bbef65e [Routing] Add strict_parameters option to disable exceptions when a route generation fails due to an invalid parameter
Discussion
----------
[Routing] Add strict_parameters option to disable exceptions on invalid parameters
---------------------------------------------------------------------------
by travisbot at 2012-06-09T15:07:26Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577025) (merged bbef65e6 into 37678e17).
---------------------------------------------------------------------------
by stof at 2012-06-09T15:43:48Z
Seems good, but you forgot to update the Changelog of the component. Anyway, let's wait for @vicb's review as he knows the Routing component better than me.
---------------------------------------------------------------------------
by Seldaek at 2012-06-09T16:35:56Z
I updated the changelog
---------------------------------------------------------------------------
by travisbot at 2012-06-09T16:38:31Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577716) (merged d5ab4c1d into 37678e17).
---------------------------------------------------------------------------
by travisbot at 2012-06-11T10:10:37Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1590901) (merged a54e59e4 into 37678e17).
---------------------------------------------------------------------------
by travisbot at 2012-06-11T13:50:21Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1591926) (merged 31843cf0 into 0995b1f2).
Commits
-------
680e732 [Routing] fix phpDoc
Discussion
----------
[Routing] fix phpDoc
using inheritdoc where possible and removing api tag when parent interface has one
as requested by stof and fabpot
---------------------------------------------------------------------------
by travisbot at 2012-06-09T16:14:53Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1577508) (merged 0a44632a into f8a09db5).
---------------------------------------------------------------------------
by travisbot at 2012-06-10T19:36:25Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1585766) (merged 680e732a into 7bec0786).
Commits
-------
2277500 [Routing][HttpKernel] Add RequestMatcherInterface.
Discussion
----------
[Routing][HttpKernel] Add RequestMatcherInterface.
First try at implementing #4351.
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Fixes the following tickets: -
Todo: -
License of the code: MIT
---------------------------------------------------------------------------
by travisbot at 2012-05-21T19:37:07Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1392716) (merged 457496db into ea33d4d3).
---------------------------------------------------------------------------
by travisbot at 2012-05-21T19:47:51Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1392939) (merged 78effa98 into ea33d4d3).
---------------------------------------------------------------------------
by everzet at 2012-05-21T20:17:03Z
No tests?
---------------------------------------------------------------------------
by travisbot at 2012-05-21T20:18:18Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1393392) (merged 6564fb6a into ea33d4d3).
---------------------------------------------------------------------------
by schmittjoh at 2012-05-21T20:35:14Z
You need to remove the type-hint from the constructor, and probably add an exception instead where the matching methods are called to ensure that either ``UrlMatcherInterface``, or ``RequestMatcherInterface`` were passed. Alternatively, you could also add such a check to the constructor.
---------------------------------------------------------------------------
by fabpot at 2012-05-22T06:52:01Z
related to symfony/symfony#4020
---------------------------------------------------------------------------
by niklasf at 2012-05-25T11:11:45Z
Reverted the changes to UrlMatcher.php.
---------------------------------------------------------------------------
by fabpot at 2012-05-25T12:46:06Z
@niklasf: it looks good now except for the listener constructor (see @schmittjoh suggestion above). Can you fix that and add some unit tests to ensure that everything works as expected? Thanks.
---------------------------------------------------------------------------
by stof at 2012-05-25T12:52:59Z
Another solution could be to make the ``RequestMatcherInterface`` extend the ``MatcherInterface`` to keep the typehint in the constructor
---------------------------------------------------------------------------
by fabpot at 2012-05-25T13:52:26Z
I thought about that as well, but that does not make sense.
---------------------------------------------------------------------------
by stof at 2012-05-25T14:12:19Z
Well, the RouterInterface extends UrlMatcherInterface anyway (and it should stay this way as it would be a huge BC break) so I guess most people will implement both interfaces when implementing the RquestMatcherInterface
---------------------------------------------------------------------------
by travisbot at 2012-05-25T15:26:22Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1433963) (merged 8f36204c into ff4d446c).
---------------------------------------------------------------------------
by travisbot at 2012-05-25T15:33:13Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1433996) (merged 6d2f2cd9 into ff4d446c).
---------------------------------------------------------------------------
by travisbot at 2012-05-25T15:39:01Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1434060) (merged 3c1d89e2 into ff4d446c).
---------------------------------------------------------------------------
by travisbot at 2012-05-25T22:06:34Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1437398) (merged 3ab997c1 into ff4d446c).
---------------------------------------------------------------------------
by travisbot at 2012-05-25T22:06:47Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1437385) (merged d8c0e387 into ff4d446c).
---------------------------------------------------------------------------
by fabpot at 2012-05-26T06:41:31Z
Can you add a note in the CHANGELOG of the component?
---------------------------------------------------------------------------
by travisbot at 2012-05-26T08:12:40Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1440435) (merged c7458733 into 9e951991).
---------------------------------------------------------------------------
by niklasf at 2012-05-26T08:14:41Z
@fabpot: Sorry, not sure how: Under 2.1.0 or in a new section? As the first or last entry of the list?
---------------------------------------------------------------------------
by stof at 2012-05-26T10:20:23Z
@niklasf the new interface should be mentioned in the changelog of the Routing component
---------------------------------------------------------------------------
by travisbot at 2012-05-26T10:30:06Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1440837) (merged 34ea86a9 into 9e951991).
---------------------------------------------------------------------------
by niklasf at 2012-06-02T15:27:01Z
Ah ... so there were two pitfalls:
- PHPUnit clones the arguments of mocked functions. So they wouldn't equal.
- createGetResponseEventForUri() disables routing on purpose. So not using that helper, now.
Tests should be passing.
---------------------------------------------------------------------------
by travisbot at 2012-06-02T15:30:28Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1509054) (merged 7fab0118 into 1541fe26).
Commits
-------
a196ca0 [Routing] Compiler: remove lazy quantifiers with no effect
8232aa1 [Routing] Compiler: fix in the computing of the segment separators
Discussion
----------
[Routing] Fix the matching process
This PR is based on the PR #3678, #4139.
[![Build Status](https://secure.travis-ci.org/vicb/symfony.png?branch=routingmatcher)](http://travis-ci.org/vicb/symfony)
**The spec**
A pattern is composed of both text and variable segments: `/{variable}-test/{other_variable}`.
A variable segment will match anything until a separator is encountered. The separator is the character following the variable segment when available or preceding the variable otherwise (i.e. at the end of the pattern).
That is:
* the separator is `-` for the `variable`,
* the separator is `/` for the `other_variable`.
*Note: This default matching behavior can be overridden if a requirement is specified for a variable)*
**Fixes**
* The current behavior is to consider booth the preceding and following characters as separators (considering availability),
* The "preceding" separator of the first variable is always set to `/` whatever the preceding character is (due to `$pos = 0` for the first iteration).
**Todo**
Update the doc once this is merged
Commits
-------
c195957 [Components] Tests/Autoloading fixes
Discussion
----------
Fix components
See #4141
----
This PR:
* configures each component to use composer to manage "dev" dependencies instead of env variables;
* adds phpunit configuration file on Filesystem component;
* fixes READMEs.
It's mergeable without any problems, but I would recommend to wait a fix in Composer in order to use `self.version` in `require`/`require-dev` sections.
Note: I kept `suggest` sections because it makes sense but this PR doesn't aim to provide useful explanations for each entry. It could be another PR, not that one.
---------------------------------------------------------------------------
by willdurand at 2012-04-30T20:43:13Z
@fabpot I reviewed each component, one by one. Now `phpunit` always works, even if tests are skipped. A simple `composer install --dev` allows to run the complete test suite. Each commit is well separated from the others. I guess, everything is ok now.
---------------------------------------------------------------------------
by Tobion at 2012-04-30T20:47:00Z
Please squash, as it makes no sense to have the same commit for each component.
---------------------------------------------------------------------------
by fabpot at 2012-05-01T14:26:11Z
Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by willdurand at 2012-05-01T14:29:38Z
done
---------------------------------------------------------------------------
by fabpot at 2012-05-01T15:48:25Z
It does not seem that the commits are squashed.
---------------------------------------------------------------------------
by willdurand at 2012-05-01T15:54:08Z
done
* Switched to Composer to manage "dev" dependencies
* Fixed READMEs
* Excluded vendor in phpunit.xml.dist files
* Fixed message in bootstrap.php files
* Added autoloader for the component itself
Commits
-------
cb47b03 [Routing] small refactoring + language fixes
Discussion
----------
[Routing] small refactoring + language fixes
tests pass, no bc break
---------------------------------------------------------------------------
by vicb at 2012-04-12T06:42:19Z
@Tobion could you squash your commits ?
---------------------------------------------------------------------------
by Tobion at 2012-04-12T19:25:03Z
done
Commits
-------
382b083 [Routing] improved generated class by PhpGeneratorDumper
27a05f4 [Routing] small optimization of PhpGeneratorDumper
Discussion
----------
[Routing] improved PhpGeneratorDumper
Test pass: yes
BC break: no
The first commit only replaces arrays with strings and makes some cosmetic changes, so that it's more readable. This makes the `PhpGeneratorDumper` consistent in style with `PhpMatcherDumper` that I fixed recently and should slightly improve performance of the generation of the class.
The second commit changes the output of the `PhpGeneratorDumper->dump` and tries to optimize the resulting class that is used to generate URLs. It's best explained with an example.
Before my changes:
```php
class ProjectUrlGenerator extends Symfony\Component\Routing\Generator\UrlGenerator
{
static private $declaredRouteNames = array(
'Test' => true,
'Test2' => true,
);
/**
* Constructor.
*/
public function __construct(RequestContext $context)
{
$this->context = $context;
}
public function generate($name, $parameters = array(), $absolute = false)
{
if (!isset(self::$declaredRouteNames[$name])) {
throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
}
$escapedName = str_replace('.', '__', $name);
list($variables, $defaults, $requirements, $tokens) = $this->{'get'.$escapedName.'RouteInfo'}();
return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $absolute);
}
private function getTestRouteInfo()
{
return array(array ( 0 => 'foo',), array (), array (), array ( 0 => array ( 0 => 'variable', 1 => '/', 2 => '[^/]+?', 3 => 'foo', ), 1 => array ( 0 => 'text', 1 => '/testing', ),));
}
private function getTest2RouteInfo()
{
return array(array (), array (), array (), array ( 0 => array ( 0 => 'text', 1 => '/testing2', ),));
}
}
```
After my changes in second commit:
```php
class ProjectUrlGenerator extends Symfony\Component\Routing\Generator\UrlGenerator
{
static private $declaredRoutes = array(
'Test' => array ( 0 => array ( 0 => 'foo', ), 1 => array ( ), 2 => array ( ), 3 => array ( 0 => array ( 0 => 'variable', 1 => '/', 2 => '[^/]+?', 3 => 'foo', ), 1 => array ( 0 => 'text', 1 => '/testing', ), ),),
'Test2' => array ( 0 => array ( ), 1 => array ( ), 2 => array ( ), 3 => array ( 0 => array ( 0 => 'text', 1 => '/testing2', ), ),),
);
/**
* Constructor.
*/
public function __construct(RequestContext $context)
{
$this->context = $context;
}
public function generate($name, $parameters = array(), $absolute = false)
{
if (!isset(self::$declaredRoutes[$name])) {
throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
}
list($variables, $defaults, $requirements, $tokens) = self::$declaredRoutes[$name];
return $this->doGenerate($variables, $defaults, $requirements, $tokens, $parameters, $name, $absolute);
}
}
```
As you can see, there is no need to escape the route name and invoke a special method anymore. Instead the route properties are included in the static route array directly, that existed anyway. Is also easier to read as defined routes and their properties are in the same place.
Commits
-------
77185e0 [Routing] Allow spaces in the script name for the apache dumper
6465a69 [Routing] Fixes to handle spaces in route pattern
Discussion
----------
[Routing] Handling of space characters in the dumpers
The compiler was using the 'x' modifier in order to ignore extra spaces and line feeds but the code was flawed:
- it was actually ignoring all the spaces, not only the extra ones added by the compiler,
- all the spaces were stripped in the php and apache matchers.
The proposed fix:
- do not use the 'x' modifier any more (and then do no add extra spaces / line feeds),
- do not strip the spaces in the matchers,
- escapes the spaces (both in regexs and script name) for the apache matcher.
It also include [a small optimization](https://github.com/vicb/symfony/pull/new#L9L89) when the only token of a route is an optional variable token - the idea is to make the regex easier to read.
---------------------------------------------------------------------------
by vicb at 2012-04-10T13:59:45Z
@Baachi fixed now. Thanks.
---------------------------------------------------------------------------
by Tobion at 2012-04-10T16:01:31Z
+1, I saw no reason for pretty printing the regex in the first place (just for debugging I guess).
@vicb since you want to make the regex easier to read, I propose the remove the `P` from the variable regex `?P<bar>`, which is not needed anymore in PHP 5.3 (and we only support PHP 5.3+ anyway).
---------------------------------------------------------------------------
by vicb at 2012-04-10T16:08:36Z
@Tobion could you make a PR to this branch for the named parameters ?
---------------------------------------------------------------------------
by Tobion at 2012-04-10T16:12:34Z
I can include it in #3754 because I'm about the add 2 more fixes to it anyway.
But when I proposed to apply these fixes to 2.0 Fabien rejected it. So not sure what branch you want me to apply this.
---------------------------------------------------------------------------
by vicb at 2012-04-10T16:25:38Z
May be the best is to put it on hold while I am reviewing your PRs. There are already enough changes, we'll make an other PR after all have been sorted out.
What's the difference between 3754 and 3810 ? (3810 + 3763 = 3754 ?)
---------------------------------------------------------------------------
by Tobion at 2012-04-10T16:39:32Z
Lol you forget to link the PR numbers. At first sight I thought it's some sort of mathematical riddle. Haha
#3810 is for 2.0 = #3763 (already merged) + #3754 for master
---------------------------------------------------------------------------
by vicb at 2012-04-10T16:52:18Z
I didn't link on purpose... the question is if '=' means strictly or loosely equal (any diffs - beside master vs 2.0) ?
---------------------------------------------------------------------------
by Tobion at 2012-04-10T17:06:04Z
It just applies my changes to 2.0. Nothing more. So master still differs from 2.0 by the addional features that were already implemented (e.g. `RouteCollection->addCollection` with optional requirements and options). But since my changes are bug fixes (except the performance improvement in #3763 but that doesn't break anything and makes 2.0 easier to maintain) I thought they should go into 2.0 as well.
---------------------------------------------------------------------------
by vicb at 2012-04-10T17:14:27Z
@Tobion only bug fixes mean "only bug fixes". You should re-open a PR for 2.0 with "only bug fixes", you might want to wait for me to review 3754.
---------------------------------------------------------------------------
by Tobion at 2012-04-10T17:21:00Z
Without #3763 it's much harder to apply the bug fixes. And now that I found 2 more bugs which requiresome rewriting of the PhpMatcherDumper, I don't want to apply all the commits by hand again for 2.0...
- The route compiler does not add extra space or line-feed,
- The generated regex does not use the 'x' modified any more,
- The PHP and apache matchers do not need to strip any chars (vs space and line feed before),
- The space characters are escaped according to the apache format
Commits
-------
56c1e31 performance improvement in PhpMatcherDumper
Discussion
----------
performance improvement in PhpMatcherDumper
Tests pass: yes
The code generation uses a string internally instead of an array. The array wasn't used for random access anyway.
I also removed 4 unneeded iterations this way (when imploding, when merging and twice when applying the extra indention). A `preg_replace` could also be saved under certain circumstances by moving it.
And there was a small code errror in line 139.
Commits
-------
bafcaaf Removed version field
f9d9dc7 Add branch-alias for composer
Discussion
----------
Add branch-alias for composer
This should restore the 2.1-dev version (as an alias of dev-master) so that `2.*` or `2.1.*` constraints work again. I'll adjust packagist soon to also display those aliases.
Commits
-------
e6e9b5a [Routing] Return the _route parameter from ApacheUrlMatcher
Discussion
----------
[Routing] Return the _route parameter from ApacheUrlMatcher
---------------------------------------------------------------------------
by fabpot at 2012-02-22T23:13:49Z
Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by arnaud-lb at 2012-02-23T09:12:45Z
sure, done
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
-------
7c1cbb9 [Config] Use LoaderResolverInterface for type-hinting
48b084e fixed typo
8ad94fb merged branch hhamon/doctrine_bridge_cs (PR #2775)
240796e [Bridge] [Doctrine] fixed coding conventions.
7cfc392 check for session before trying to authentication details
648fae7 merged branch proofek/domcrawlerform-radiodisabled (PR #2768)
3976b7a [DoctrineBridge] fixed CS
9a04783 merged branch beberlei/SecurityEntityRepositoryIdentifierFix (PR #2765)
3c83b89 [DoctrineBridge] Catch user-error when the identifier is not serialized with the User entity.
36c7d03 Fixed GH-2720 - Fix disabled atrribute handling for radio form elements
Discussion
----------
[Config] Use LoaderResolverInterface for type-hinting
```
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
```
I've listed this as a BC break because we're changing the argument type-hint, but I think it's unlikely to affect anyone.
Commits
-------
731b28b [composer] add missing deps for FrameworkBundle
9c8f100 [composer] change ext/intl to the new ext-intl syntax
d535afe [composer] fix monolog-bridge composer.json, add more inter-component deps
9ade639 [composer] add composer.json
Discussion
----------
Composer
This PR adds a composer.json file for [composer](https://github.com/composer/composer) ([more info](packagist.org/about-composer)).
For discussion you can also go into #composer-dev on freenode and argue with naderman, seldaek and everzet.
---------------------------------------------------------------------------
by naderman at 2011/09/26 15:51:51 -0700
You haven't entered any keywords, they might come in handy when searching for packages on packagist.
But really this is just a +1 ;-)
---------------------------------------------------------------------------
by stof at 2011/09/26 16:12:21 -0700
See my comments on your previous (non-rebased) commit: f1c0242b5a
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:04:36 -0700
Following dependencies do not have a composer.json yet: Twig, Doctrine (orm, dbal, common), swiftmailer.
Also missing from the standard edition: assetic, twig-extensions, jsm-metadata, SensioFrameworkExtraBundle, JMSSecurityExtraBundle, SensioDistributionBundle, SensioGeneratorBundle, AsseticBundle.
The point is, those can be added later on. Having the components composerized is already a leap forward. Also, doctrine depends on some symfony components, we've got to start somewhere.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 00:36:41 -0700
Also, just for information, the plan is to have `symfony/framework-bundle` be the "framework", with all dependencies to doctrine etc, though we should really only have strict requirements in there, and then in symfony-standard we ship a composer.json that requires the framework-bundle, doctrine-orm and things like that that are not essential to core. Otherwise people don't have a choice about what they use anymore.
Just a comment btw, the json is invalid, all / should be escaped. However json_decode is nice enough to parse those without complaining, browsers do too, even Crockford's json2.js does, so I'm not sure if we should privilege readability over strictness, since it seems nobody really cares about this escaping.
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:41:39 -0700
So, I've implemented all of @stof's suggestions, except (for reasons stated above):
* doctrine to DoctrineBundle
* swiftmailer to SwiftmailerBundle
* twig to TwigBundle
* doctrine-common to Validator
* FrameworkBundle (what exactly does it depend on?)
---------------------------------------------------------------------------
by stof at 2011/09/27 00:52:31 -0700
@igorw at least HttpKernel, Routing, Templating, EventDispatcher, Doctrine Common (annotations cannot be disabled), Translator, Form (optional), Validator (optional), Console (optional). See the service definitions to see the others
@Seldaek FrameworkBundle does not depend on Doctrine, except for Common
---------------------------------------------------------------------------
by beberlei at 2011/09/27 03:15:34 -0700
What does the symfony/ or ext/ prefix control in composer?
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 03:33:52 -0700
symfony/ is just the (mandatory) vendor namespace. Also ext/ has been renamed to ext- now, so it's not in any vendor, and should avoid potential issues.
---------------------------------------------------------------------------
by beberlei at 2011/09/27 05:07:03 -0700
@Seldaek Mandatory? So every package name is "vendor/package"? I like that because previously i thought package names are not namespaced, and thus clashes could occur between different communities easily.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 05:16:20 -0700
@beberlei: Mandatory. As of yesterday http://packagist.org/ will tell you you have an invalid package name if there's no slash in it. See 1306d1ca82 (diff-3)
Doctrine caches annotations. For methods, it uses PHP reflection and the getDeclaringClass() to create
a unique cache key. Unfortunately, if you have 2 classes that extend another one, the cache will be shared.
It's not a problem except that before this patch, the default route name was also cached (as the cache is serialized
after we changed the object). So, all other classes inherited this default route name. The fix is quite easy:
just don't change the read annotation object.
Commits
-------
03c7cfe UrlGenerator no longer appends '?' if query string is empty
Discussion
----------
UrlGenerator no longer appends '?' if query string is empty
If you generate a URL using null parameters (`array('foo' => null, 'bar' => null')`), `http_build_query` returns an empty string, resulting in a trailing `?` at the end of the generated URL.
This fixes that so that, if there are `$extra` params & `http_build_query` is empty, the URL is no longer appended.
---------------------------------------------------------------------------
by fabpot at 2011/07/22 10:15:26 -0700
Can you add unit tests?
---------------------------------------------------------------------------
by ericclemmons at 2011/07/22 10:52:21 -0700
Yes sir, will do.
-Eric Clemmons
Sent from my iPad Nano
On Jul 22, 2011, at 12:15 PM, fabpot<reply@reply.github.com> wrote:
> Can you add unit tests?
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1773#issuecomment-1633515
---------------------------------------------------------------------------
by ericclemmons at 2011/07/22 11:55:30 -0700
**Added passing test.**
Currently `master` fails test:
```
1) Symfony\Tests\Component\Routing\Generator\UrlGeneratorTest::testUrlWithNullExtraParameters
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-http://localhost/app.php/testing
+http://localhost/app.php/testing?
//tests/Symfony/Tests/Component/Routing/Generator/UrlGeneratorTest.php:114
```
Commits
-------
418d6a0 [Routing] Fix syntax error when dumping routes with single quotes in the requirements or pattern
2b5e22d [Routing] Fix ApacheDumper when a space appears in a default value
6c7f484 [Routing] Fix dumper so it doesn't print trailing whitespace
761724a [Routing] Adjust urlescaping rules, fixes#752
Discussion
----------
[Router] Bunch o' Fixes
The first commit changes the escaping rule to fix issues I had previously, and #752 as well, here's from the full commit message:
Only + and % are now encoded in generated routes, since they are the only characters that, if not encoded, could cause problems/conflicts when decoded. Namely + turns into a space, and % followed by numbers could do funky things.
The matcher decodes everything which works since nothing will have %NN without being escaped, and + are escaped as well.
Second commit is just a test fix for the first
Third and fourth are simply dumper escaping issues, nothing to argue about.
Note that all changes have had test cases added, and I spent a few hours torturing/testing all this stuff with both Apache and PHP dumpers, in many browsers, and with URLs as wonky as `/%01%02%03%04%05%06%07%08%0E%0F%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%20!%22$%25&%27%28%29*+,-./0123456789:;%3C=%3E@ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B|%7D~/baz` which essentially represent the 1-255 char range minus ? and #.
The only issues I really encountered after all the patches were applied is that Apache refuses to match `%22` (= `"`) and `*` in a url. I guess it's just because they're not allowed chars in windows paths, but | and < > works fine though. Anyway this works with the PHP dumper, and it didn't work either without my patches so it's not like I broke it, I'm just saying for the record.
Only + and % are now encoded in generated routes, since they are the only characters that, if not encoded, could cause problems/conflicts when decoded. Namely + turns into a space, and % followed by numbers could do funky things.
The matcher decodes everything which works since nothing will have %NN without being escaped, and + are escaped as well.
Commits
-------
49dd558 Couple more CS fixes
5a986bf Add $keysCount & minor CS fix
91f4097 [Routing] Better nesting for RouteCollections in dumped URL matcher classes With this change, a route prefixed with '/blogger' will be nested inside '/blog' (for example)
Discussion
----------
[Routing] Better nesting for RouteCollections in dumped URL matcher classes
Consider the following routing file:
_blog:
resource: "@AcmeDemoBundle/Resources/config/routing/BlogController.yml"
prefix: /blog
_blogger:
resource: "@AcmeDemoBundle/Resources/config/routing/BloggerController.yml"
prefix: /blogger
_bloggeroo:
resource: "@AcmeDemoBundle/Resources/config/routing/BloggerooController.yml"
prefix: /bloggeroo
_blogtest:
pattern: /blogtest
defaults: { _controller: AcmeDemoBundle:Blog:test }
login:
pattern: /login
defaults: { _controller: AcmeDemoBundle:Security:login }
Note: Each imported file contains two simple blog/blogger/bloggeroo routes (e.g. blog_index & blog_view)
With this change, the cached URL matcher looks something like this:
```php
<?php
class appprodUrlMatcher
{
public function match($pathinfo)
{
$allow = array();
if (0 === strpos($pathinfo, '/blog')) {
// blog_index
if (preg_match('#^/blog/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {...}
// blog_view
if (preg_match('#^/blog/(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {...}
if (0 === strpos($pathinfo, '/blogger')) {
// blogger_index
if (preg_match('#^/blogger/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {...}
// blogger_view
if (preg_match('#^/blogger/(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {...}
if (0 === strpos($pathinfo, '/bloggeroo')) {
// bloggeroo_index
if (preg_match('#^/bloggeroo/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {...}
// bloggeroo_view
if (preg_match('#^/bloggeroo/(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {...}
throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
// _blogtest
if ($pathinfo === '/blogtest') {...}
throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
// login
if ($pathinfo === '/login') {...}
throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
}
```
As far as I can see, you can throw the 404 earlier (as I've done), as by nesting all these possibilities, there is no chance another route will be matched outside the parent if statement.
---------------------------------------------------------------------------
by stloyd at 2011/06/11 08:37:15 -0700
You should follow Symfony [CS rules] (http://symfony.com/doc/current/contributing/code/standards.html), also you should modify tests.
---------------------------------------------------------------------------
by lmcd at 2011/06/11 08:46:32 -0700
Besides the inline `continue` statement, I can't see spot any other CS violations. I'm on to the updated tests.
---------------------------------------------------------------------------
by fabpot at 2011/06/13 02:19:14 -0700
What if you have this:
_blog:
resource: "@AcmeDemoBundle/Resources/config/routing/BlogController.yml"
prefix: /blog
login:
pattern: /login
defaults: { _controller: AcmeDemoBundle:Security:login }
_blogger:
resource: "@AcmeDemoBundle/Resources/config/routing/BloggerController.yml"
prefix: /blogger
You cannot send 404 early because the same `blog` prefix is used in two different places. I can see many things that will break with this patch. I'm pretty sure we can enhance the performance of the existing code but let's do that after 2.0.
---------------------------------------------------------------------------
by lmcd at 2011/06/13 05:04:03 -0700
@fabpot: The code was designed so it would work in these kind of circumstances. I ran the dumper against the routing file you provided and this is the output below.
I see no reason why you can't throw the 404 earlier, as by nesting everything with similar prefixes, we're eliminating the possibility that a route will ever be matched outside of that if statement.
```php
public function match($pathinfo)
{
$allow = array();
if (0 === strpos($pathinfo, '/blog')) {
// blog_index
if (preg_match('#^/blog/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {
...
}
// blog_view
if (preg_match('#^/blog/(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {
...
}
if (0 === strpos($pathinfo, '/blogger')) {
// blog_index
if (preg_match('#^/blogger/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {
...
}
// blog_view
if (preg_match('#^/blogger/(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {
...
}
throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
// login
if ($pathinfo === '/login') {
...
}
throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
}
```
---------------------------------------------------------------------------
by fabpot at 2011/06/13 07:48:49 -0700
Can you add some more tests to ensure that everything works fine and that we won't have regressions later on? Thanks.
* lmcd/master:
$code referenced but not defined in compileRoute()
[Routing] Optimised the PHP URL matcher dumper The cached URL matcher classes contain some unneeded logic. Consider the following example:
The cached URL matcher classes contain some unneeded logic. Consider the following example:
if (0 === strpos($pathinfo, '/Blog')) {
// blog_index
if (0 === strpos($pathinfo, '/Blog') && preg_match('#^/Blog/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {
return array_merge($this->mergeDefaults($matches, array ( '_action' => 'index',)), array('_route' => 'blog_index'));
}
}
The 2nd strpos is not required, as we have already satisfied this condition in the parent if statement.
My change will produce the following code for the same routing setup::
if (0 === strpos($pathinfo, '/Blog')) {
// blog_index
if (preg_match('#^/Blog/(?P<slug>[^/]+?)$#x', $pathinfo, $matches)) {
return array_merge($this->mergeDefaults($matches, array ( '_action' => 'index',)), array('_route' => 'blog_index'));
}
}
Here are the new simplified rules:
* Required cache warmers are *always* executed when the Kernel boots for the first time;
* Optional cache warmers are *only* executed from the CLI via cache:warmup
These new rules means that all the configuration settings for the cache
warmers have been removed. So, if you want the best performance, remember to
warmup the cache when going to production.
This also fixed quite a few bugs.
* Chekote/xmlfileloader_overload_refactor:
Refactored the processing of each individual node into it's own method, enabling easier overloading of behavior for Bundles such as FriendsOfSymfony/RestBundle
* alexandresalome/feat-routing-exceptions:
[Routing] Fix the exception inheritance + Add the LICENCE block in new files
[Routing] Change the Exception namespacing + base class for every exception + update PHPDoc
[Routing] Add specific exceptions for the UrlGenerator
When generating URL, thrown exceptions are InvalidArgumentException and
distinction of errors is quite difficult. This modification brings different
exceptions for different cases
The _scheme requirement can be used to force routes to always match one given scheme
and to always be generated with the given scheme.
So, if _scheme is set to https, URL generation will force an absolute URL if the
current scheme is http. And if you request the URL with http, you will be redirected
to the https URL.
This improves, for example, the exception one would receive if they tried to import a resource from a bundle that doesn't exist.
Previously, the deep "bundle is not activated" exception would be thrown. That has value, however there is no indication of where
the exception is actually occurring.
In this new implementation, we throw an exception that explains exactly which resource, and from which source resource, cannot be
loaded. The deeper exception is still thrown as a nested exception.
Two caveats:
* The `HttpKernel::varToString` method was replicated
* This introduces a new `Exception` class, which allows us to prevent lot's of exceptions from nesting into each other in the case
that some deeply imported resource cannot be imported (each upstream import that fails doesn't add its own exception).