* 2.2:
Fix finding ACLs from ObjectIdentity's with different types
[HttpKernel] tweaked previous merge
#7531: [HttpKernel][Config] FileLocator adds NULL as global resource path
Fix autocompletion of command names when namespaces conflict
Fix timeout in Process::stop method
fixed CS
Round stream_select fifth argument up.
Fix Process timeout
[HttpKernel] Remove args from 5.3 stack traces to avoid filling log files, fixes#7259
bumped Symfony version to 2.2.2-DEV
updated VERSION for 2.2.1
updated CHANGELOG for 2.2.1
Fixed phpdoc blocks to show that $uri can be passed as a string or ControllerReference (rather than just as a string)
[HttpFoundation] Fixed copy pasted comment from FlashBag in AttributeBag
[FrameworkBundle] fixed the discovery of the PHPUnit configuration file when using aggregate options like in -vc app/ (closes#7562)
[WebProfilerBundle] removed next pointer class in a template
fix overwriting of request's locale if attribute _locale is missing
Conflicts:
src/Symfony/Component/HttpKernel/Debug/ErrorHandler.php
src/Symfony/Component/HttpKernel/EventListener/LocaleListener.php
src/Symfony/Component/HttpKernel/Kernel.php
* 2.1:
[HttpFoundation] Fixed copy pasted comment from FlashBag in AttributeBag
[FrameworkBundle] fixed the discovery of the PHPUnit configuration file when using aggregate options like in -vc app/ (closes#7562)
[WebProfilerBundle] removed next pointer class in a template
This PR was merged into the master branch.
Discussion
----------
[HttpKernel] adds possibility to add attributes to hx:include tag
This PR follows #7439 and adds escaping to attributes.
This introduces 2 options to the `HIncludeFragmentRenderer::render()` method:
* `id`: an optional `id` attribute for the `hx:include` tag
* `attrs`: an optional array of attributes for the `hx:include` tag
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #7439
Commits
-------
670a502 [HttpKernel] added escaping for hx:include tag attributes
342d089 Allow to set id + custom attributes to hinclude tag
* 2.2: (22 commits)
fixed doc references (closes#7515)
fixed doc references (closes#7515)
On OS X, sys_get_tmp_dir() returns /var/private/..., which really is below /private/var.
Doctrine cannot handle bare random non-utf8 strings
small changes
[SecurityBundle] Fixed configuration exemple
idAsIndex should be true with a smallint or bigint id field.
[PropertyAccess] Remove trailing periods from doc blocks
Fix param docs for PropertyAccessor read method
Fixed long multibyte parameter logging in DbalLogger:startQuery
Keep the file extension in the temporary copy and test that it exists (closes#7482)
bumped Symfony version to 2.1.10-DEV
[Validator][translation][japanese]replaced period to japanese one [Validator][translation][japanese]fixed japanese translation to more practical one [Validator][translation][japanese]fixed message ordering to be consistent with other languages [Validator][translation][japanese]added new validation messages in japanese translation
updated VERSION for 2.1.9
update CONTRIBUTORS for 2.1.9
updated CHANGELOG for 2.1.9
[Security] fixed wrong interface
Remove already defined arguments
Add missing use
[FrameworkBundle] Reuse definition variable in FormPass
...
Conflicts:
src/Symfony/Bridge/Doctrine/Logger/DbalLogger.php
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Collector/config.html.twig
* 2.1:
bumped Symfony version to 2.1.10-DEV
updated VERSION for 2.1.9
update CONTRIBUTORS for 2.1.9
updated CHANGELOG for 2.1.9
[FrameworkBundle] Reuse definition variable in FormPass
Conflicts:
src/Symfony/Component/HttpKernel/Kernel.php
This PR was merged into the master branch.
Discussion
----------
[2.3] [FrameworkBundle] [Templating] added Stopwatch support to the PHP engine
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
I did not include tests nor documentation because there weren't any for TimedTwigEngine (I took it as an example). If I'm mistaken and they are needed, I'll gladly write them.
Commits
-------
3c3d34d [FrameworkBundle] [Templating] added Stopwatch support to the PHP engine
This PR was merged into the master branch.
Discussion
----------
[2.3] [WIP] Synchronized services...
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #5300, #6756
| License | MIT
| Doc PR | symfony/symfony-docs#2343
Todo:
- [x] update documentation
- [x] find a better name than contagious (synchronized)?
refs #6932, refs #5012
This PR is a proof of concept that tries to find a solution for some problems we have with scopes and services depending on scoped services (mostly the request service in Symfony).
Basically, whenever you want to inject the Request into a service, you have two possibilities:
* put your own service into the request scope (a new service will be created whenever a sub-request is run, and the service is not available outside the request scope);
* set the request service reference as non-strict (your service is always available but the request you have depends on when the service is created the first time).
This PR addresses this issue by allowing to use the second option but you service still always has the right Request service (see below for a longer explanation on how it works).
There is another issue that this PR fixes: edge cases and weird behaviors. There are several bug reports about some weird behaviors, and most of the time, this is related to the sub-requests. That's because the Request is injected into several Symfony objects without being updated correctly when leaving the request scope. Let me explain that: when a listener for instance needs the Request object, it can listen to the `kernel.request` event and store the request somewhere. So, whenever you enter a sub-request, the listener will get the new one. But when the sub-request ends, the listener has no way to know that it needs to reset the request to the master one. In practice, that's not really an issue, but let me show you an example of this issue in practice:
* You have a controller that is called with the English locale;
* The controller (probably via a template) renders a sub-request that uses the French locale;
* After the rendering, and from the controller, you try to generate a URL. Which locale the router will use? Yes, the French locale, which is wrong.
To fix these issues, this PR introduces a new notion in the DIC: synchronized services. When a service is marked as synchronized, 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 when the scope leaves.
If you have a look at the router or the locale listener, you will see that there is now a `setRequest` method that will called whenever the request service changes (because the `Container::set()` method is called or because the service is changed by a scope change).
Commits
-------
17269e1 [DependencyInjection] fixed management of scoped services with an invalid behavior set to null
bb83b3e [HttpKernel] added a safeguard for when a fragment is rendered outside the context of a master request
5d7b835 [FrameworkBundle] added some functional tests
ff9d688 fixed Request management for FragmentHandler
1b98ad3 fixed Request management for LocaleListener
a7b2b7e fixed Request management for RequestListener
0892135 [HttpKernel] ensured that the Request is null when outside of the Request scope
2ffcfb9 [FrameworkBundle] made the Request service synchronized
ec1e7ca [DependencyInjection] added a way to automatically update scoped services
This PR was merged into the master branch.
Discussion
----------
moved the request scope creation to the ContainerAwareHttpKernel class
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | symfony/symfony-docs#2343
While updating the scope documentation, I realized that the request scope was created in the FrameworkBundle while the HttpKernel that manages it was in the HttpKernel component. So, this PR makes things more consistent.
Commits
-------
cec98c1 [DependencyInjection] fixed PHP notice when the scope is not defined
550df5a moved the request scope creation to the ContainerAwareHttpKernel class
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).
* 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
* 2.1:
sub-requests are now created with the same class as their parent
[FrameworkBundle] removed BC break
[FrameworkBundle] changed temp kernel name in cache:clear
[DoctrineBridge] Avoids blob values to be logged by doctrine
[Security] use current request attributes to generate redirect url?
[Validator] fix showing wrong max file size for upload errors
[TwigBridge] removed double var initialization (refs #7344)
[2.1][TwigBridge] Fixes Issue #7342 in TwigBridge
[FrameworkBundle] fixed cahe:clear command's warmup
[TwigBridge] now enter/leave scope on Twig_Node_Module
[TwigBridge] fixed fixed scope & trans_default_domain node visitor
[TwigBridge] fixed non probant tests & added new one
[BrowserKit] added ability to ignored malformed set-cookie header
[Translation] removed wriong 'use'
[Translation] added xliff loader/dumper with resname support
[TwigBridge] fixes
Conflicts:
src/Symfony/Bundle/FrameworkBundle/HttpKernel.php
src/Symfony/Component/Security/Http/HttpUtils.php
src/Symfony/Component/Translation/Loader/XliffFileLoader.php
src/Symfony/Component/Translation/Tests/Loader/XliffFileLoaderTest.php
* 2.1:
[FrameworkBundle] Fix code status in dockblock
Fixed test to use Reflection
[Finder] fixed a potential issue on Solaris where INF value is wrong (refs #7269)
Update RouteCompiler.php
[FrameworkBundle] avoids cache:clear to break if new/old folders already exist
[HttpKernel] Fixed possible profiler token collision (closes#7272, closes#7171)
[ClassLoader] tweaked test
[ClassLoader] made DebugClassLoader idempotent
[DomCrawler] Fix relative path handling in links
Conflicts:
src/Symfony/Component/DomCrawler/Link.php
src/Symfony/Component/Finder/Iterator/DepthRangeFilterIterator.php
src/Symfony/Component/Routing/RouteCompiler.php
This PR was squashed before being merged into the master branch (closes#7202).
Commits
-------
817453c [2.2] add http_method_override option to ease setup
Discussion
----------
[2.2] add http_method_override option to ease setup
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| License | MIT
This PR was squashed before being merged into the 2.1 branch (closes#7260).
Commits
-------
1d3da29 [FrameworkBundle] avoids cache:clear to break if new/old folders already exist
Discussion
----------
[FrameworkBundle] avoids cache:clear to break if new/old folders already exist
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #6502
---------------------------------------------------------------------------
by mahmouds at 2013-03-04T19:07:20Z
👍
* 2.2: (26 commits)
[FrameworkBundle] Fixes invalid serialized objects in cache
remove dead code in yaml component
Fixed typo in UPGRADE-2.2
fixed typo
RedisProfilerStorage wrong db-number/index-number selected
[DependencyInjection] added a test for the previous merge (refs #7261)
Unset loading[$id] in ContainerBuilder on exception
Default validation message translation fix.
remove() should not use deprecated getParent() so it does not trigger deprecation internally
adjust routing tests to not use prefix in addCollection
add test for uniqueness of resources
added tests for addDefaults, addRequirements, addOptions
adjust RouteCollectionTest for the addCollection change and refactor the tests to only skip the part that really needs the config component
added tests for remove() that wasnt covered yet and special route name
refactor interator test that was still assuming a tree
adjust tests to no use addPrefix with options
adjusted tests to not use RouteCollection::getPrefix
[Routing] trigger deprecation warning for deprecated features that will be removed in 2.3
[Console] fixed StringInput binding
[Console] added string input test
...
This PR was merged into the master branch.
Commits
-------
0ef08f5 [Translator] fixed inconsistency in Translator
Discussion
----------
[2.3] [Translator] fixed inconsistency in Translator
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | yes
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
---------------------------------------------------------------------------
by stof at 2013-02-17T20:44:32Z
shouldn't you make ``setFallbackLocale`` call ``setFallbackLocales`` ?
---------------------------------------------------------------------------
by fabpot at 2013-02-18T14:04:39Z
@stof: done
---------------------------------------------------------------------------
by stof at 2013-02-18T14:18:10Z
shoudn't you also update the place where the method is used (in FrameworkBundle probably) to use the non-deprecated one ?
* 2.2: (24 commits)
Options small typo
[Console] fixed unparsed StringInput tokens
Mask PHP_AUTH_PW header in profiler
[TwigBridge] fixed trans twig extractor
[Finder] adds adapter selection/unselection capabilities
[DomCrawler] fix handling of schemes by Link::getUri()
[Console] Fixed comment
[TwigBridge] fixed the translator extractor that were not trimming the text in trans tags (closes#7056)
Fixed handling absent href attribute in base tag
fixed paths/notPaths regex for shell adapters
fix issue 4911
Adds expandable globs support to shell adapters
[HttpFoundation] Fixed messed up headers
Fixes AppCache + ESI + Stopwatch problem
added a DebuClassLoader::findFile() method to make the wrapping less invasive
bumped Symfony version to 2.2.0-RC4-DEV
updated VERSION for 2.2.0-RC3
updated CHANGELOG for 2.2.0-RC3
fixed CHANGELOG
bumped Symfony version to 2.1.9-DEV
...
* 2.1:
Options small typo
[Console] fixed unparsed StringInput tokens
[TwigBridge] fixed trans twig extractor
[DomCrawler] fix handling of schemes by Link::getUri()
[Console] Fixed comment
[TwigBridge] fixed the translator extractor that were not trimming the text in trans tags (closes#7056)
Fixed handling absent href attribute in base tag
added a DebuClassLoader::findFile() method to make the wrapping less invasive
fixed CHANGELOG
bumped Symfony version to 2.1.9-DEV
updated VERSION for 2.1.8
updated CHANGELOG for 2.1.8
StringInput resets the given options.
Conflicts:
src/Symfony/Component/HttpKernel/Kernel.php
* 2.2:
Defined stable version point of Doctrine.
[HttpFoundation] Remove Cache-Control when using https download via IE<9 (fixes#6750)
Update composer.json
[Form] Fixed TimeType not to render a "size" attribute in select tags
[Form] Added test for "label" option to accept the value "0"
Expanded fault-tolerance for unusual cookie dates
Fix docblock type
[Form] Fixed "label" option to accept the value "0"
Added greek translation
merged branch jfcixmedia/2.1 (PR #5838)
added a note about a BC break for the path info of sub-request (closes#7138)
[DomCrawler] lowered parsed protocol string (fixes#6986)
[FrameworkBundle] Fix a BC for Hinclude global template
[HttpKernel] fixed locale management when exiting sub-requests
fixed HInclude renderer (closes#7113)
Removed some leaking deprecation warning in the Form component
[HttpKernel] hinclude fragment renderer must escape URIs properly to return valid html
Conflicts:
src/Symfony/Bundle/FrameworkBundle/composer.json
src/Symfony/Component/Security/composer.json
* 2.1:
Defined stable version point of Doctrine.
[HttpFoundation] Remove Cache-Control when using https download via IE<9 (fixes#6750)
Update composer.json
[Form] Fixed TimeType not to render a "size" attribute in select tags
[Form] Added test for "label" option to accept the value "0"
Expanded fault-tolerance for unusual cookie dates
Fix docblock type
[Form] Fixed "label" option to accept the value "0"
merged branch jfcixmedia/2.1 (PR #5838)
[DomCrawler] lowered parsed protocol string (fixes#6986)
Conflicts:
composer.json
src/Symfony/Bridge/Twig/Resources/views/Form/form_div_layout.html.twig
src/Symfony/Bundle/FrameworkBundle/Resources/views/Form/time_widget.html.php
src/Symfony/Bundle/FrameworkBundle/composer.json
src/Symfony/Component/Form/Tests/Extension/Csrf/EventListener/CsrfValidationListenerTest.php
src/Symfony/Component/Routing/composer.json
src/Symfony/Component/Security/composer.json
src/Symfony/Component/Validator/composer.json
* 2.2: (22 commits)
[Process] Fix regression introduced in #6620 / 880da01c49, fixes#7082
[HttpKernel] added a unit for the previous commit (closes#7025)
[HttpFoundation] fixed, overwritten CONTENT_TYPE
[BrowserKit] fixed test added in the previous merge (refs #7059)
[FrameworkBundle] tweaked reference dumper command (see #7093)
Remove unnecessary comment and change test name
[Config] tweaked dumper to indent multi-line info
[HttpKernel] added some tests for previous merge
Fix REMOTE_ADDR for cached subrequests
[FrameworkBundle] CSRF should be on by default
[WebProfilerBundle] removed dependency on FrameworkBundle (closes#6949)
[HttpKernel] added error display suppression when using the ErrorHandler (if not, errors are displayed twice, refs #6254)
[HttpFoundation] tweaked previous merge
[HttpFoundation] Added getter for httpMethodParameterOverride state
Create validators.lv.xlf
[Process] Warn user with a useful message when tmpfile() failed
[BrowserKit] added a test to make sure HTTP authentication is preserved when submitting a form
Remove array type hint from GetResponseForControllerResultEvent::setControllerResult()
bumped Symfony version to 2.2.0-DEV
Revert "merged branch povilas/issue_6101 (PR #6708)"
...
This PR was merged into the 2.2 branch.
Commits
-------
a313188 added a proper setter for the templating servicein HInclude
Discussion
----------
added a proper setter for the templating servicein HInclude
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
---------------------------------------------------------------------------
by stof at 2013-02-17T12:44:40Z
👍
* 2.2:
Fixed XmlFileLoaderTest::testLoadThrowsExceptionWithInvalidFileEvenWithoutSchemaValidation
moved file hash calculation to own method
[Validator] Add check for existing metadata on property
added support for the X-Forwarded-For header (closes#6982, closes#7000)
fixed the IP address in HttpCache when calling the backend
[EventDispatcher] Added assertion.
[EventDispathcer] Fix removeListener
[DependencyInjection] Add clone for resources which were introduced in 2.1
[DependencyInjection] Allow frozen containers to be dumped to graphviz
Fix 'undefined index' error, when entering scope recursively
[Security] fixed session creation on login (closes#7011)
replaced usage of the deprecated pattern routing key (replaced with path)
Add dot character `.` to legal mime subtype regular expression
[HttpFoundation] fixed the creation of sub-requests under some circumstancies (closes#6923, closes#6936)
This PR was merged into the 2.2 branch.
Commits
-------
73aa7d1 replaced usage of the deprecated pattern routing key (replaced with path)
Discussion
----------
replaced usage of the deprecated pattern routing key (replaced with path)
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | n/a
| License | MIT
| Doc PR | n/a
---------------------------------------------------------------------------
by lsmith77 at 2013-02-07T13:35:54Z
do we have tests to cover the BC behavior?
---------------------------------------------------------------------------
by fabpot at 2013-02-07T16:30:31Z
I've just added some tests for the legacy way.
* 2.2: (30 commits)
[HttpFoundation] Added support for partial ranges in the BinaryFileResponse.
[HttpFoundation] Fixed byte ranges in the BinaryFileResponse.
updated required versions when depending on the HttpFoundation component
updated required versions when depending on the HttpKernel component
updated required versions when depending on the Config component
updated required versions when depending on the Form component
updated required versions when depending on the DependencyInjection component
updated required versions when depending on the Validator component
updated required versions when depending on the Translation component
updated required versions when depending on the Routing component
updated required versions when depending on the EventDispatcher component
updated required versions when depending on the OptionsResolver component
updated required versions when depending on the PropertyAccess component
updated required versions when depending on the Security component
updated required versions when depending on the Templating component
updated required versions when depending on the Stopwatch component
updated required versions when depending on the Process component
updated required versions when depending on the Finder component
updated required versions when depending on the Dom Crawler component
use ~2.0 when depending on the Dom Crawler component
...
* 2.2:
[HttpFoundation] fixed Request::create() method
[HttpKernel] fixed the creation of the Profiler directory
[HttpKernel] fixed the hinclude fragment renderer when the template is empty
bumped Symfony version to 2.2.0-RC2-DEV
[DependencyInjection] enhanced some error messages
[FrameworkBundle] fixed typo
fixed typo
tweaked previous merge
[Security] fixed interface implementation (closes#6974)
Add "'property_path' => false" deprecation message for forms
fixed CS
Added BCrypt password encoder.
updated VERSION for 2.2.0-RC1
Removed underscores from test method names to be consistent with other components.
[Security] fixed session creation when none is needed (closes#6917)
[FrameworkBundle] removed obsolete comment (see 2e356c1)
Micro-optimization
[FrameworkBundle] removed extra whitespaces
[Security] renamed Constraint namespace to Constraints for validator classes in order to be consistent with the whole current validator API.
[FrameworkBundle] fixed wrong indentation on route debug output
* 2.1:
[HttpKernel] fixed the creation of the Profiler directory
[Security] fixed session creation when none is needed (closes#6917)
[FrameworkBundle] removed obsolete comment (see 2e356c1)
* 2.2:
fixed regression in the Finder component (it was possible to use it without using exec before, closes#6357)
fixed a circular call (closes#6864)
typo
[Security] [Tests] added unit tests for the UserPasswordValidator class and made the validator service for the UserPassword constraint configurable.
fixed wrong indentation
tweaked previous commit
[HttpKernel] Fix the URI signer (closes#6801)
Add Arabic translations.
[HttpKernel] fixed regression when rendering an inline controller and passing some objects (closes#6822)
[FrameworkBundle] fixed typo
renamed some classes and Twig functions to more descriptive names (refs #6871)
Classcollectionloader: fix traits + enhancements
Fix a deprecated method call in the tests
Update `composer.json` files: - to allow versions ~2.2 (>=2.2,<3.0) of Doctrine DBAL, ORM & Common - fixed Propel1 versions difference between main and bridge files - fixed Twig versions difference between main and bridge files - to allow versions ~1.11 (>=1.11,<2.0) of Twig - fixed Locale ext-intl version to accept all, not non-existing version
Correct comment in NativeSessionStorage regarding session.save_handler
[Security] Add PHPDoc to AuthenticationEvents
HttpContentRenderer has been renamed to FragmentHandler.
The RendererStrategy subnamespace has been renamed to Fragment.
The strategy classes now have Fragment in their names.
ProxyRouterListener has been renamed to FragmentListener
The router_proxy configuration entry has been renamed to fragments.
This PR was squashed before being merged into the master branch (closes#6852).
Commits
-------
fde7585 [DIC] Better handling of enableable configurations
Discussion
----------
[DIC] Better handling of enableable configurations
| Q | A
| ------------- | ---
| Bug fix? | yes
| New feature? | no
| BC breaks? | no, this feature has not been released yet
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | -
| License | MIT
| Doc PR | -
My definition of bug fix might be discussable. The thing which I think is not discussable is that this PR fixes the semantic - and I think it is important for a "semantic configuration": before this PR, some nodes had `->canBeDisabled` for nodes that were actually disabled by default. Those nodes now have `->canBeEnabled` which sounds right.
**Edit: Jan 28, 2013** - history:
See [the related comments](https://github.com/symfony/symfony/pull/6829#discussion_r2727742).
I think Symfony **must** get the configuration right as we can expect of lot of devs to use this as a template when writting their own configuration.
@schmittjoh could you please give me your feedback on [this change](https://github.com/symfony/symfony/pull/6852/files#L4R224) considering [the rationale](https://github.com/symfony/symfony/pull/6852/files#L3R7).
---------------------------------------------------------------------------
by stof at 2013-01-23T16:10:33Z
@vicb your links are broken as they are pointing to the PR creation page
---------------------------------------------------------------------------
by stof at 2013-01-23T16:10:55Z
and to create a TODO list, it has to be a list first
---------------------------------------------------------------------------
by vicb at 2013-01-23T16:31:10Z
@stof thanks for reporting the broken links, they are fixed /cc @schmittjoh
---------------------------------------------------------------------------
by vicb at 2013-01-23T16:31:50Z
@Tobion please submit a PR to my repo, I don't have much time to work on this. Thanks !
---------------------------------------------------------------------------
by vicb at 2013-01-25T15:14:47Z
@fabpot @schmittjoh I'd like your feedback on the latest commit, rationale is in the method phpDoc. It better matches what we do now and seem the most sensible thing to do.
edit: with this you can no more disable the node explicitly, I have to find a better solution
---------------------------------------------------------------------------
by schmittjoh at 2013-01-25T15:20:13Z
Looks good.
On Fri, Jan 25, 2013 at 4:15 PM, Victor Berchet <notifications@github.com>wrote:
> @fabpot <https://github.com/fabpot> @schmittjoh<https://github.com/schmittjoh>I'd like your feedback on the latest commit, rationale is in the method
> phpDoc. It better matches what we do now and seem the most sensible thing
> to do.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/6852#issuecomment-12704585>.
>
>
---------------------------------------------------------------------------
by vicb at 2013-01-28T14:37:57Z
@fabpot I know I keep insisting on this one and I am sorry for that but I think this should be considered as a bug fix (see the PR header for details) and should be merged in 2.2. I think the Symfony core should be exemplary as it is used by many developers as a template when creating their own bundle. *This PR is no more a WIP and can be merged right now*.
In addition to fixing the enableable nodes, this PR contain new UTs and some fixes to the code / tests.
---------------------------------------------------------------------------
by fabpot at 2013-01-28T16:43:42Z
@vicb As explained in a comment, this is not a BC break as this feature does not exist in 2.1. So, I can make the change to the CHANGELOG if you want after merging, or I can let you make the change.
---------------------------------------------------------------------------
by vicb at 2013-01-28T16:46:33Z
I am going to change it right now !
---------------------------------------------------------------------------
by vicb at 2013-01-28T16:46:56Z
(and thanks for having checked this)
---------------------------------------------------------------------------
by vicb at 2013-01-28T16:54:37Z
@fabpot I have updated the changelog and the PR header.
I am not sure if the commits should be squashed or not. On one side the multiple commits can help understand the changes but on the other side that's a lot of small commits which could pollute history. I let you choose what to do.
* 2.1:
[DependencyInjection] fixed the creation of synthetic services in ContainerBuilder
[Security] PHPDoc in SecurityEvents
Fix typos in README
Added an error message in the DebugClassLoader when using / instead of \.
KNOWN_ISSUES with php 5.3.16
[FrameworkBundle] fixed Client::doRequest that must call its parent method (closes#6737)
[Yaml] fixed ignored text when parsing an inlined mapping or sequence (closes#6786)
[Yaml] fixed#6773
[Yaml] fixed#6770
bumped Symfony version to 2.1.8-DEV
bumped Symfony version to 2.0.23-DEV
Conflicts:
src/Symfony/Bundle/FrameworkBundle/Client.php
src/Symfony/Component/HttpKernel/Kernel.php
This PR was squashed before being merged into the master branch (closes#6734).
Commits
-------
4d51ec0 Fix for hardcode (#6384) in choice widget
Discussion
----------
Fix for hardcode (#6384) in choice widget
empty_value should not be disabled if field is not required!
#6384
---------------------------------------------------------------------------
by sstok at 2013-01-15T15:51:08Z
You need to revert the file mode changes (100644 → 100755)
---------------------------------------------------------------------------
by MaksSlesarenko at 2013-01-18T16:44:42Z
fixed tests
---------------------------------------------------------------------------
by MaksSlesarenko at 2013-01-21T15:36:59Z
ping @fabpot
---------------------------------------------------------------------------
by fabpot at 2013-01-21T15:58:26Z
ping @bschussek
---------------------------------------------------------------------------
by MaksSlesarenko at 2013-01-23T11:15:37Z
ping @fabpot @bschussek
---------------------------------------------------------------------------
by Tobion at 2013-01-23T12:08:19Z
I think it's good to squash and merge.
---------------------------------------------------------------------------
by fabpot at 2013-01-23T12:16:37Z
Can you rebase and squash before I merge? Thanks.
---------------------------------------------------------------------------
by MaksSlesarenko at 2013-01-23T19:51:36Z
@fabpot done
* 2.0:
[DependencyInjection] fixed the creation of synthetic services in ContainerBuilder
[Security] PHPDoc in SecurityEvents
[FrameworkBundle] fixed Client::doRequest that must call its parent method (closes#6737)
[Yaml] fixed ignored text when parsing an inlined mapping or sequence (closes#6786)
[Yaml] fixed#6773
[Yaml] fixed#6770
bumped Symfony version to 2.0.23-DEV
Conflicts:
src/Symfony/Component/DependencyInjection/ContainerBuilder.php
src/Symfony/Component/HttpKernel/Kernel.php
src/Symfony/Component/Yaml/Inline.php
src/Symfony/Component/Yaml/Tests/InlineTest.php
As explained in #6775, this has been done for the following reasons:
1. It's also Request::getHost()
2. The term hostname has been obsoleted in
http://tools.ietf.org/html/rfc3986#appendix-D.2 and uses the host only
3. hostname in the RFC was defined as the registered domain name, but we
probably also want to match IP-Adresses with the pattern which is the
host = IP-literal / IPv4address / reg-name for.
* 2.1:
[Yaml] fixed default value
Added Yaml\Dumper::setIndentation() method to allow a custom indentation level of nested nodes.
added a way to enable/disable object support when parsing/dumping
added a way to enable/disable PHP support when parsing a YAML input via Yaml::parse()
fixed CS
[Process] Fix docblocks, remove `return` from `PhpProcess#start()` as parent returns nothing, cleaned up `ExecutableFinder`
fixes a bug when output/error output contains a % character
[Console] fixed input bug when the value of an option is empty (closes#6649, closes#6689)
[Profiler] [Redis] Fix sort of profiler rows.
Fix version_compare() calls for PHP 5.5.
Removed underscores from test method names to be consistent with other components.
[Process] In edge cases `getcwd()` can return `false`, then `proc_open()` should get `null` to use default value (the working dir of the current PHP process)
Fix version_compare() calls for PHP 5.5.
Handle the deprecation of IntlDateFormatter::setTimeZoneId() in PHP 5.5.
removed the .gitattributes files (closes#6605, reverts #5674)
[HttpKernel] Clarify misleading comment in ExceptionListener
Conflicts:
src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar_style.html.twig
src/Symfony/Component/Form/Tests/Extension/Core/Type/DateTimeTypeTest.php
src/Symfony/Component/Form/Tests/Extension/Core/Type/TimeTypeTest.php
src/Symfony/Component/Form/Tests/Util/PropertyPathTest.php
src/Symfony/Component/HttpKernel/Profiler/RedisProfilerStorage.php
src/Symfony/Component/Process/Process.php
This PR was merged into the master branch.
Commits
-------
9fc7def added the UPGRADE file for Symfony 3.0
e84cad2 [Routing] updated CHANGELOG
65eca8a [Routing] added new schemes and methods options to the annotation loader
5082994 [Routing] renamed pattern to path
b357caf [Routing] renamed hostname pattern to just hostname
e803f46 made schemes and methods available in XmlFileLoader
d374e70 made schemes and methods available in YamlFileLoader
2834e7e added scheme and method setter in RouteCollection
10183de make scheme and method requirements first-class citizen in Route
Discussion
----------
Routing options
| Q | A
| ------------- | ---
| Bug fix? | no
| New feature? | no
| BC breaks? | no
| Deprecations? | yes
| Tests pass? | yes
| Fixed tickets | #5989, #5990, #6049
| License | MIT
In #5989, it has unanimously been decided to renamed `hostname_pattern` to `hostname` and `pattern` to `path`. That makes a lot of sense and I would like to do the renaming now as `hostname_pattern` is new in Symfony 2.2, so I'd like to avoid breaking BC just after the release. As we are modifying the route options, I've also included changes introduced by @Tobion in #6049 which were discussed in #5990.
As everything is BC, I think it's wise to include that in 2.2. What do you think?
---------------------------------------------------------------------------
by Tobion at 2013-01-14T18:25:53Z
I agree it should be done in 2.2. Thanks for working on it.
---------------------------------------------------------------------------
by vicb at 2013-01-14T23:11:12Z
@fabpot "Everything is BC" until it breaks BC in 3.0, that's why I'd like to see [deprecations in PR summary](https://github.com/symfony/symfony-docs/pull/2116) what do you think ?
---------------------------------------------------------------------------
by vicb at 2013-01-14T23:16:40Z
it would also be great to update the CHANGELOG with deprecations (it could also help people answering your question)
---------------------------------------------------------------------------
by fabpot at 2013-01-15T07:07:03Z
@vicb: I've just updated the CHANGELOG and created the UPGRADE file for 3.0.
---------------------------------------------------------------------------
by vicb at 2013-01-15T07:15:32Z
@fabpot thanks.
This PR was merged into the master branch.
Commits
-------
76fefe3 updated CHANGELOG and UPGRADE files
f7da1f0 added some unit tests (and fixed some bugs)
f17f586 moved the container aware HTTP kernel to the HttpKernel component
2eea768 moved the deprecation logic calls outside the new HttpContentRenderer class
bd102c5 made the content renderer work even when ESI is disabled or when no templating engine is available (the latter being mostly useful when testing)
a8ea4e4 [FrameworkBundle] deprecated HttpKernel::forward() (it is only used once now and not part of any interface anyway)
1240690 [HttpKernel] made the strategy a regular parameter in HttpContentRenderer::render()
adc067e [FrameworkBundle] made some services private
1f1392d [HttpKernel] simplified and enhanced code managing the hinclude strategy
403bb06 [HttpKernel] added missing phpdoc and tweaked existing ones
892f00f [HttpKernel] added a URL signer mechanism for hincludes
a0c49c3 [TwigBridge] added a render_* function to ease usage of custom rendering strategies
9aaceb1 moved the logic from HttpKernel in FrameworkBundle to the HttpKernel component
Discussion
----------
[WIP] Kernel refactor
Currently, the handling of sub-requests (including ESI and hinclude) is mostly done in FrameworkBundle. It makes these important features harder to implement for people using only HttpKernel (like Drupal and Silex for instance).
This PR moves the code to HttpKernel instead. The code has also been refactored to allow easier integration of other rendering strategies (refs #6108).
The internal route has been re-introduced but it can only be used for trusted IPs (so for the internal rendering which is managed by Symfony itself, or by a trusted reverse proxy like Varnish for ESI handling). For the hinclude strategy, when using a controller, the URL is automatically signed (see #6463).
The usage of a listener instead of a controller to handle internal sub-requests speeds up things quite a lot as it saves one sub-request handling. In Symfony 2.0 and 2.1, the handling of a sub-request actually creates two sub-requests.
Rendering a sub-request from a controller can be done with the following code:
```jinja
{# default strategy #}
{{ render(path("partial")) }}
{{ render(controller("SomeBundle:Controller:partial")) }}
{# ESI strategy #}
{{ render(path("partial"), { strategy: 'esi' }) }}
{{ render(controller("SomeBundle:Controller:partial"), { strategy: 'esi' }) }}
{# hinclude strategy #}
{{ render(path("default1"), { strategy: 'hinclude' }) }}
```
The second commit allows to simplify the calls a little bit thanks to some nice syntactic sugar:
```jinja
{# default strategy #}
{{ render(path("partial")) }}
{{ render(controller("SomeBundle:Controller:partial")) }}
{# ESI strategy #}
{{ render_esi(path("partial")) }}
{{ render_esi(controller("SomeBundle:Controller:partial")) }}
{# hinclude strategy #}
{{ render_hinclude(path("default1")) }}
```
---------------------------------------------------------------------------
by fabpot at 2013-01-03T17:58:49Z
I've just pushed a new version of the code that actually works in my browser (but I've not yet written any unit tests). I've updated the PR description accordingly.
All comments welcome!
---------------------------------------------------------------------------
by Koc at 2013-01-03T20:11:43Z
what about `render(controller="SomeBundle:Controller:partial", strategy="esi")`?
---------------------------------------------------------------------------
by stof at 2013-01-04T09:01:01Z
shouldn't we have interfaces for the UriSigner and the HttpContentRenderer ?
---------------------------------------------------------------------------
by lsmith77 at 2013-01-04T19:28:09Z
btw .. as mentioned in #6213 i think it would make sense to refactor the HttpCache to use a cache layer to allow more flexibility in where to cache the data (including clustering) and better invalidation. as such if you are refactoring HttpKernel .. it might also make sense to explore splitting off HttpCache.
---------------------------------------------------------------------------
by fabpot at 2013-01-04T19:30:07Z
@lsmith77 This is a totally different topic. This PR is just about moving things from FrameworkBundle to HttpKernel to make them more reusable outside of the full-stack framework.
---------------------------------------------------------------------------
by fabpot at 2013-01-05T09:39:52Z
I think this PR is almost ready now. I still need to update the docs and add some unit tests. Any other comments on the whole approach? The class names? The `controller` function thingy? The URI signer mechanism? The proxy protection for the internal controller? The proxy to handle internal routes?
---------------------------------------------------------------------------
by sstok at 2013-01-05T10:08:25Z
Looks good to me 👍
---------------------------------------------------------------------------
by sdboyer at 2013-01-07T18:17:08Z
@Crell asked me to weigh in, since i'm one of the Drupal folks who's likely to work most with this.
i think i've grokked about 60% of the big picture here, and i'm generally happy with what i see. the assumption that the HInclude strategy makes about working with templates probably isn't one that we'll be able to use (and so, would need to write our own), but that's not a big deal since the whole goal here is to make strategies pluggable.
so, yeah. +1.
---------------------------------------------------------------------------
by winzou at 2013-01-09T20:21:44Z
Just for my information: will this PR be merged for 2.2 version? Thanks.
---------------------------------------------------------------------------
by stof at 2013-01-09T20:41:04Z
@winzou according to the blog post announcing the beta 1 release, yes. It is explicitly listed as being one of the reason to make it a beta instead of the first RC.
---------------------------------------------------------------------------
by winzou at 2013-01-09T20:49:36Z
OK thanks, I've totally skipped this blog post.
---------------------------------------------------------------------------
by fabpot at 2013-01-10T15:26:15Z
I've just added a bunch of unit tests and fix some bugs I found while writing the tests.
This PR was merged into the master branch.
Commits
-------
6b1652e [PropertyAccess] Property path, small refactoring, read/writeProperty to read/write Property/Index.
1bae7b2 [PropertyAccess] Extracted PropertyAccess component out of Form
Discussion
----------
[PropertyAccess] Extracted PropertyAccess component out of Form
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
TODO: adapt DoctrineBundle/PropelBundle to pass the "property_accessor" service to EntityType/ModelType
Usage:
```php
$accessor = PropertyAccess::getPropertyAccessor();
// equivalent to $object->getFoo()->setBar('value')
$accessor->setValue($object, 'foo.bar', 'value');
// equivalent to $object->getFoo()->getBar()
$accessor->getValue($object, 'foo.bar');
// equivalent to $object->getFoo()['bar']
$accessor->getValue($object, 'foo[bar]');
// equivalent to $array['foo']->setBar('value')
$accessor->setValue($array, '[foo].bar', 'value');
// equivalent to $array['foo']['bar']
$accessor->getValue($array, '[foo][bar]');
```
Later on, once we have generation and caching of class-specific accessors, configuration will be something like this (consistent with the Form and Validator component):
```php
$accessor = PropertyAccess::getPropertyAccessorBuilder()
->setCacheDirectory(__DIR__ . '/cache')
->setCacheLifeTime(86400)
->enableMagicGetSet()
->enableMagicCall()
->getPropertyAccessor();
```
or
```php
$accessor = PropertyAccess::getPropertyAccessorBuilder()
->setCache($cache)
->getPropertyAccessor();
```
etc.
---------------------------------------------------------------------------
by Burgov at 2013-01-07T08:48:15Z
+1. I use this feature outside of the Form context a lot
---------------------------------------------------------------------------
by stof at 2013-01-07T08:49:34Z
The classes in the Form component should be kept for BC (and deprecated) for people using the feature
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T10:02:19Z
YES YES YES 👍. Sorry for my enthusiasm, but I already copy pasted the PropertyPath class to some of my libraries to avoid linking to the whole Form component. I thus will be glad to officially use this component into my libraries via composer.
---------------------------------------------------------------------------
by norzechowicz at 2013-01-07T10:17:39Z
Same as @michelsalib to avoid linking full Form component I was using copied parts of code. Can't wait to use this component in my lib. 👍
---------------------------------------------------------------------------
by bschussek at 2013-01-07T10:43:41Z
I split away `getValue()` and `setValue()` from `PropertyPath` into a new class `ReflectionGraph`. The component is also named ReflectionGraph now.
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T10:47:10Z
I am not found of the name. What do you intend to do in the component more than what PropertyPath does ?
---------------------------------------------------------------------------
by bschussek at 2013-01-07T10:58:59Z
@michelsalib A `PropertyPath` is simply a string like `foo.bar[baz]`. `getValue()` and `setValue()` interpret this path. There may be different interpretations for the same path, so these methods were split into a new class.
I chose the name `ReflectionGraph` because the functionality is very similar to `ReflectionProperty`.
```php
$reflProperty = new ReflectionProperty('Vendor/Class', 'property');
$reflProperty->setValue($object, 'foo');
$reflGraph = new ReflectionGraph();
$reflGraph->setValue($object, 'property.path', 'foo');
```
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T11:00:42Z
What about naming it `Reflection`, maybe sometime we will want to add more reflection tools for classes, interfaces... ?
---------------------------------------------------------------------------
by bschussek at 2013-01-07T11:02:32Z
@michelsalib I doubt that we will do that. PHP's implementation is sufficient.
---------------------------------------------------------------------------
by vicb at 2013-01-07T11:03:57Z
> Backwards compatibility break: no
Really ?
---------------------------------------------------------------------------
by michelsalib at 2013-01-07T11:05:07Z
Well, that is just a suggestion. If I am the only one to oppose, I won't complain.
---------------------------------------------------------------------------
by bschussek at 2013-01-07T11:09:08Z
> Really ?
@vicb Would you please refrain from such meanginless comments in the future? I'm getting a bit tired of them. If you think that BC is broken somewhere, tell me where so that I can fix it.
---------------------------------------------------------------------------
by stof at 2013-01-07T11:09:43Z
@vicb There is no BC break as he kept deprecated classes for BC
---------------------------------------------------------------------------
by norzechowicz at 2013-01-07T11:13:12Z
@bschussek what do you think about some kind of factory for Reflection? This will prevent creating new Reflection objects each time you want to access properties values.
---------------------------------------------------------------------------
by vicb at 2013-01-07T11:18:47Z
@bschussek my point is that my comment is no more meaningless than closing #6453 because it will break BC.We could also keep BC by extending the classes in the new ns but in both cases BC will ultimately be broken (when the legacy classes are removed)
---------------------------------------------------------------------------
by vicb at 2013-01-07T12:23:45Z
@bschussek @stof I think that modifying the constructor signatures of `EntityChoiceList`, `FormType` are BC breaks (this is not an exhaustive list)
---------------------------------------------------------------------------
by bschussek at 2013-01-07T12:35:13Z
@vicb You are right. I added corresponding entries to the CHANGELOG and adapted the above description.
---------------------------------------------------------------------------
by vicb at 2013-01-08T13:39:13Z
@bschussek looking at this PR, I was wondering if an alternate syntax would make sense:
```php
<?php
$reflGraph = new ReflectionGraph($object);
// equivalent to $object->getFoo()->setBar('value')
$reflGraph['foo.bar'] = 'value';
// equivalent to $object->getFoo()->getBar()
$reflGraph['foo.bar'];
```
_Sorry for the off topic_
---------------------------------------------------------------------------
by vicb at 2013-01-08T13:49:46Z
The advantage of using such a `ReflectionGraph` factory is that it might be easier to return specialized reflection graphs, ie optimized instances (that would be cached).
---------------------------------------------------------------------------
by Toflar at 2013-01-08T14:49:54Z
I was also puzzled by the fact that there will be many `ReflectionGraph` instances although they don't have to. I'm with @vicb and I'd also vote for using the constructor to set the subject you're working on. Otherwise you'll repeat yourself over and over again by passing the subject - say `$object` - to `getValue()` or `setValue()`. If however you don't like the constructor thing then why do we have to have an instance of `ReflectionGraph` rather than just go for static methods and use `ReflectionGraph::getValue()` and `ReflectionGraph::setValue()`?
In my opinion there are a few methods that could be static anyway (especially some private ones) :)
But probably I misunderstood something as I'm just about to discover the SF components and don't have any experience working with them (so basically I just read the PR because of @bschussek's tweet :D)
Couldn't come up with any intuitive name for the component though :(
Generally when we talk about "getting" and "setting" values we call those things "mutators"...so `GraphMutator` might be more intuitive than the word `Reflection` :)
---------------------------------------------------------------------------
by Taluu at 2013-01-08T14:57:42Z
I like the last proposition made by @vicb (implementing `ArrayAccess` on `ReflectionGraph` - or whatever name will be chosen (`PathMutator` for example :D), and also specify which object should be worked on in the constructor rather than in each method).
Would this also be used in the `Validator` component ?
---------------------------------------------------------------------------
by stof at 2013-01-08T15:16:12Z
@Toflar A static ``ReflectionGraph::getValue()`` means you have a coupling to the implementation (as with any static call). The current implementation allows you to replace it with your own implementation as long as you implement the interface as it follows the DI pattern (as done in other places in Symfony).
@vicb The issue with ``$reflGraph = new ReflectionGraph($object);`` is that you cannot inject the ReflectionGraph anymore, as you need a new one each time. This would mean adding a ``ReflectionGraphFactory`` to be injected (and which could then be replaced by a factory using code generation). Using the constructor directly would not allow using a replacement based on code generation later. So the resulting code would more likely be
```php
$reflGraph = new ReflectionGraph();
$mutator = $reflGraph->getMutator($object);
// equivalent to $object->getFoo()->setBar('value')
$mutator['foo.bar'] = 'value';
// equivalent to $object->getFoo()->getBar()
$mutator['foo.bar'];
```
Btw, writing this, I find the naming Mutator suggested by @Taluu good when it concerns the setter, but quite weird when getting the value.
---------------------------------------------------------------------------
by Taluu at 2013-01-08T15:21:00Z
I was not the one to suggest though, it was @everzet. But then something like `PathAccessor`, as it is both a mutator and a getter ? I also like @stof suggestion, still in the idea of avoiding to have to put the object as an argument and also allowing to use an `ArrayAccess` interface..
---------------------------------------------------------------------------
by vicb at 2013-01-08T15:21:54Z
@stof your remark makes sense.
What about `Accessor`, the benefit being that it might well be the name of a coming PHP feature: https://wiki.php.net/rfc/propertygetsetsyntax-v1.2
---------------------------------------------------------------------------
by everzet at 2013-01-08T15:27:02Z
```php
$manager = new PropertyManager(new PropertyPath());
$num = $manager->getValue($object, 'foo.num');
$manager->setValue($object, 'foo.num', $num + 1);
$objectManager = new ObjectPropertyManager($object[, $manager]);
$num = $objectManager->getValue('foo.num');
$objectManager->setValue('foo.num', $num + 1);
$objectManager['foo.num'] += 1;
```
---------------------------------------------------------------------------
by bschussek at 2013-01-08T15:28:01Z
It might be me, but I don't like `ArrayAccess` to be misused for features like that. If I access a key in an array access structure, I expect it to be something like a collection, an associative array or a key value store. This class is neither.
Putting that aside, an accessor for a specific object might make sense, but I'm not sure about that yet.
```php
$reflObject->setValue('foo.bar', 'value');
$reflObject->getValue('foo.bar');
```
---------------------------------------------------------------------------
by stof at 2013-01-08T15:28:52Z
@vicb I would vote for PathAccessor then, as we are not doing simple accessor but accessors through a path in an object graph.
In my snippet above, we would then have a ReflectionGraph instance and a PathAccessor instance (``$mutator``).
Btw, I would also keep the methods ``setValue`` and ``getValue``. I find it more clear.
---------------------------------------------------------------------------
by bschussek at 2013-01-08T15:32:07Z
@stof But then we're rather left with the question of ReflectionGraph **vs.** PathAccessor. I don't think that the tiny interface difference (one global, one object-based) justifies the big naming difference.
---------------------------------------------------------------------------
by vicb at 2013-01-08T15:33:24Z
> This class is neither.
It might be `$pa['foo.bar[baz]'] = $pa['foo.bar']['baz'];` I don't know if it would help though.
---------------------------------------------------------------------------
by stof at 2013-01-08T15:35:51Z
@bschussek In my suggestion, ``ReflectionGraph`` is a factory for the PathAccessor objects. It is not accessing anymore itself (which would probably continue to cause issues to implement it with code generation). But the naming could indeed be changed to something else.
This PR was merged into the master branch.
Commits
-------
67d7423 Remove use of deprecated HttpKernel LoggerInterface
dca4528 [HttpKernel] Extend psr/log's NullLogger class
1e5a890 [Monolog] Mark old non-PSR3 methods as deprecated
91a86f8 [HttpKernel][Monolog] Add PSR-3 support to the LoggerInterface
Discussion
----------
[HttpKernel][MonologBridge] PSR-3 support
This enables PSR-3 support and monolog 1.3+. The first commit is the main part. The rest deals with deprecation of short-hand methods (warn/err/crit/emerg) that are fully expanded in PSR-3 (warning/error/critical/emergency).
The downside of deprecating them is that for bundles it's a bit harder to support older and newer versions. If that is too much of a hassle you can drop that for now and cherry pick the first commit.
The upside is that it forces people to move towards PSR-3 compatible stuff, which means eventually we could completely drop the LoggerInterface from the framework. In any case I think the documentation should only mention the `Psr\Log\LoggerInterface` and people should start hinting against that. The change should be done in core as well I suppose.
Anyway I wanted to throw this out there as it is to get feedback.
---------------------------------------------------------------------------
by stof at 2013-01-09T09:15:15Z
@Seldaek I also think you should change the typehint to use the PSR LoggerInterface in all classes using the logger
---------------------------------------------------------------------------
by Seldaek at 2013-01-09T09:54:55Z
OK updated according to all the feedback. I tested it in an app and it still seems to work so there shouldn't be any major issues.
---------------------------------------------------------------------------
by Seldaek at 2013-01-09T09:59:55Z
@fabpot if you merge please merge also the bundle PR, otherwise it won't be possible to update without conflict.
---------------------------------------------------------------------------
by frosas at 2013-01-10T14:59:20Z
I'm trying to understand why a `composer update` of a Symfony 2.1.* resulted in a fatal error. Shouldn't a stable version don't break like this?
As @olaurendeau points, why Symfony depends 1.* instead of 1.2.*? Or why Monolog 1.3 breaks its public interface (EDIT: I'm not sure about it)? Or why isn't this PR being merged (into branch 2.1) at the same time Monolog 1.3 is released?
Please, understand I'm not looking for who to blame, it's just I want to know if this situation is unexpected or if otherwise a `composer update` on a stable branch is not as innocent as it seems.
---------------------------------------------------------------------------
by stof at 2013-01-10T15:06:51Z
@frosas it cannot be merged into 2.1 as it is a BC break. The 2.1 branch has been updated to forbid Monolog 1.3 already
---------------------------------------------------------------------------
by Seldaek at 2013-01-10T15:11:58Z
@frosas you can blame me for releasing as 1.3.0 and not 2.0, but technically for monolog this isn't really a BC break, I just added an interface. The problem is due to the way it's used in symfony, it ended up as a fatal error. In any case the situation is now sorted out I think.
---------------------------------------------------------------------------
by frosas at 2013-01-10T15:26:43Z
@stof now I see this `>=1.0,<1.3-dev` change in the 2.1 branch. Now, shouldn't a new (2.1.7) version be released for all of us not in the dev minimum-stability?
@Seldaek then do you see feasible to rely only in X.Y.* versions to avoid this kind of errors?
---------------------------------------------------------------------------
by Seldaek at 2013-01-10T15:45:22Z
@frosas relying on X.Y.* is painful because you always need to wait until someone updates the constraint to get the new version. Of course using ~1.3 like in this PR means if I fuck up and break BC people will update to it, but that's a less likely occurrence than the alternative I think, so I would rather not use X.Y.*
---------------------------------------------------------------------------
by frosas at 2013-01-10T15:50:50Z
@Seldaek you are right about this, but I was thinking more in changing it only for the stable versions. EDIT: I mean, how often do you need a new feature in a branch you only apply fixes to?
---------------------------------------------------------------------------
by stof at 2013-01-10T15:57:32Z
@frosas Monolog and Symfony have separate release cycles. Foorcing Symfony users to use an old version of Monolog until they update to a new version of Symfony whereas the newer Monolog is compatible is a bad idea. Thus, as Monolog keeps BC, it does not maintain bugfix releases for all older versions (just like Twig does too). So it would also forbid you to get the fixes done in newer Monolog versions.
The incompatibility between Symfony 2.1 LoggerInterface and PSR-3 (whereas they expect exactly the same behavior and signature for methods with the same name) is unfortunate and is the reason why we get some issues here.
---------------------------------------------------------------------------
by frosas at 2013-01-10T16:21:06Z
@stof I appreciate you prefer to allow newer versions at the price of having to be constantly monitoring its changes to avoid breaks.
Another similar but safer strategy would be to stick to X.Y.* versions and upgrade to X.Y+1.* once the new version integration is tested, but I understand this is discutible in projects as close to Symfony as Monolog.
Returning to the issue, what do you say to release this 2.1.7 version? Or is it only me who is having issues here?
---------------------------------------------------------------------------
by stof at 2013-01-10T16:26:20Z
@frosas a minor release should not break BC when following smeantic versionning (Symfony warned about the fact it is not strictly followed for the first releases of 2.x). But as far as monolog is concerned, 1.3 is BC with 1.2.
---------------------------------------------------------------------------
by Seldaek at 2013-01-10T16:49:55Z
@frosas sorry I didn't get you still had the problem. I tagged a 2.1.7 of monologbundle which hopefully fixes your issue.
This PR was merged into the master branch.
Commits
-------
73db84f [Security] Move translations file to 'security' domain
324703a [Security] Switch to English messages as message keys
aa74769 [Security] Fix CS + unreachable code
2d7a7ba [Security] Fix `AuthenticationException` serialization
50d5724 [Security] Introduced `UsernameNotFoundException#get/setUsername`
39da27a [Security] Removed `get/setExtraInformation`, added `get/set(Token|User)`
837ae15 [Security] Add note about changed constructor to changelog
d6c57cf [FrameworkBundle] Register security exception translations
d7129b9 [Security] Fix exception constructors called in `UserChecker`
0038fbb [Security] Add initial translations for AccountStatusException childs
50e2cfc [Security] Add custom `getMessageKey` AccountStatusException childs
1147977 [Security] Fix InsufficientAuthenticationException constructor calls
79430b8 [Security] Fix AuthenticationServiceException constructor calls
42cced4 [Security] Fix AuthenticationException constructor calls
963a1d7 [Security] Add initial translations for the exceptions
ed6eed4 [Security] Add `getMessageKey` and `getMessageData` to auth exceptions
694c47c [Security] Change signature of `AuthenticationException` to match `\Exception`
Discussion
----------
[2.2][Security] AuthenticationException enhancements
Bug fix: semi
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: [](http://travis-ci.org/asm89/symfony)
Fixes the following tickets: #837
License of the code: MIT
This PR adds the functionality discussed in #837 and changes the constructor of the `AuthenticationException` to match that of `\Exception`. This PR will allow developers to show a translated (save) authentication exception message to the user. :)
*Todo:*
- Add some functional test to check that the exceptions can indeed be translated?
- Get feedback on the current English messages
---------------------------------------------------------------------------
by asm89 at 2012-07-15T14:04:11Z
ping @schmittjoh
---------------------------------------------------------------------------
by schmittjoh at 2012-07-15T14:57:32Z
Looks good to me.
While you are at the exceptions, I think we can also get rid of the "extra information" thing and replace it by explicit getters/setters. Mostly that will mean adding set/getToken, set/getUser, set/getUsername. Bundles might add custom exceptions which have other data. This will make it a bit more useful and predictable.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T15:40:45Z
@schmittjoh I removed the `get/setExtraInformation` and added the more explicit getters/setters as you suggested.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T19:33:15Z
@fabpot Did you reschedule this for 2.2? Why? It was originally a 2.1 ticket. I think it is an important one because at the moment there is no reliable way to show users the cause of an `AuthenticationException` without the threat of exposing sensitive information. This issue has been around for a while, see the original issue this PR refers to, or for example [this TODO comment in FOSUB](https://github.com/FriendsOfSymfony/FOSUserBundle/blob/master/Controller/SecurityController.php#L37).
The PR itself is ready to merge now. My only question that remains is about whether the actual translations should be functional tested?
---------------------------------------------------------------------------
by fabpot at 2012-07-15T19:43:19Z
We need to stop at some point. If not, we never release anything. beta3 was scheduled for today and I don't plan any other one before the first RC and I won't have time to review this PR next week. So, if you, @schmittjoh, @vicb, @stof, and a few other core devs "validate" this PR, I might consider merging it before 2.1.
---------------------------------------------------------------------------
by asm89 at 2012-07-15T19:46:09Z
@fabpot I totally agree with your point of view. I just have been trying to pickup some security issues that were still open. :)
---------------------------------------------------------------------------
by stof at 2012-07-15T19:50:29Z
This looks good to me
---------------------------------------------------------------------------
by asm89 at 2012-08-12T09:06:24Z
Since the beta period is over I assume the window was missed to get this security related PR in 2.1. If I have feedback from @fabpot I'll still try to make it mergeable asap though.
---------------------------------------------------------------------------
by fabpot at 2012-08-13T10:10:32Z
@asm89 This would indeed be considered for merging in 2.2.
---------------------------------------------------------------------------
by Antek88 at 2012-10-03T10:30:46Z
+1
---------------------------------------------------------------------------
by stof at 2012-10-04T21:27:15Z
@asm89 could you rebase this PR ? It conflicts with master
---------------------------------------------------------------------------
by fabpot at 2012-10-05T17:16:44Z
What's the status of this PR? @asm89 Have you taken all the feedback into account?
---------------------------------------------------------------------------
by stof at 2012-10-13T17:48:48Z
@asm89 ping
---------------------------------------------------------------------------
by fabpot at 2012-10-29T09:48:40Z
@asm89 If you don't have time, I can finish the work on this PR, but can you just tell me what's left?
---------------------------------------------------------------------------
by asm89 at 2012-10-29T10:02:22Z
I can pick this up, but I have two outstanding questions:
- One about adding `::create()`? https://github.com/symfony/symfony/pull/4935#discussion_r1358297
- And what is the final verdict on the messages? https://github.com/symfony/symfony/pull/4935#discussion_r1165701 The initial idea was that the exception itself have an exception message which is plain english and informative for the developer. If you want to display the 'safe' user messages you have the optional dependency on the translator. There is a comparison made with the Validator component, but in my opinion that's a different case because the violations always contain the message directed at the user and have no plain english message for the developer. Apart from that the Validator component contains it's own code for replacing `{{ }}` variables in messages (duplication? not as flexible as the translator). Concluding I'd opt for: optional dependency on translator component if you want to show 'safe' user messages + message keys.
@schmittjoh Any things to add?
---------------------------------------------------------------------------
by schmittjoh at 2012-10-29T10:14:09Z
Message keys sound good to me. I wouldn't add the ``create`` method for now.
On Mon, Oct 29, 2012 at 11:02 AM, Alexander <notifications@github.com>wrote:
> I can pick this up, but I have two outstanding questions:
>
> - One about adding ::create()? symfony/symfony#4935<https://github.com/symfony/symfony/issues/4935#discussion_r1358297>
> - And what is the final verdict on the messages? symfony/symfony#4935<https://github.com/symfony/symfony/issues/4935#discussion_r1165701>The initial idea was that the exception itself have an exception message
> which is plain english and informative for the developer. If you want to
> display the 'safe' user messages you have the optional dependency on the
> translator. There is a comparison made with the Validator component, but in
> my opinion that's a different case because the violations always contain
> the message directed at the user and have no plain english message for the
> developer. Apart from that the Validator component contains it's own code
> for replacing {{ }} variables in messages (duplication? not as
> flexible as the translator). Concluding I'd opt for: optional dependency on
> translator component if you want to show 'safe' user messages + message
> keys.
>
> @schmittjoh <https://github.com/schmittjoh> Any things to add?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/4935#issuecomment-9861016>.
>
>
---------------------------------------------------------------------------
by fabpot at 2012-10-29T10:27:37Z
As I said in the discussion about the translations, I'm -1 for the message keys to be consistent with how we manage translations everywhere else in the framework.
---------------------------------------------------------------------------
by stof at 2012-10-29T10:30:50Z
@fabpot When we changed the English translation for the validation errors in 2.1, we had to tag the commit as a BC rbeak as it was changing the source for all other translations. And if you look at the state of the files now, you will see that we are *not* using the English as source anymore in some places as some validation errors have a pluralized translation but the source has not been changed.
So I think using a key is more future-proof.
---------------------------------------------------------------------------
by asm89 at 2012-10-30T19:44:49Z
Any final decision on this? On one hand I have @stof and @schmittjoh +1 on message keys, on the other @fabpot -1. I guess it's your call @fabpot.
Edit: also @vicb seemed to be +1 on message keys earlier on.
---------------------------------------------------------------------------
by drak at 2012-11-01T20:19:00Z
I am also -1, I agree with @fabpot
---------------------------------------------------------------------------
by asm89 at 2012-11-12T09:38:51Z
@fabpot Can you please give a definite answer on this? I personally think @stof and @vicb have good points to do message keys, but with all these different people +1 and -1'ing the PR I'm lost on what it should actually do.
---------------------------------------------------------------------------
by asm89 at 2012-11-14T09:59:06Z
ping @fabpot
---------------------------------------------------------------------------
by asm89 at 2012-11-26T10:01:27Z
ping @fabpot We talked about this in Berlin. Any final thoughts on the PR? :) One idea was to do message keys + opt depend on the translator component if you want to use them, or use your own implementation.
---------------------------------------------------------------------------
by fabpot at 2012-11-26T14:01:37Z
The conclusion is: keep using plain English.
On Mon, Nov 26, 2012 at 11:01 AM, Alexander <notifications@github.com>wrote:
> ping @fabpot <https://github.com/fabpot> We talked about this in Berlin.
> Any final thoughts on the PR? :) One idea was to do message keys + opt
> depend on the translator component if you want to use them, or use your own
> implementation.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/4935#issuecomment-10709997>.
>
>
---------------------------------------------------------------------------
by Inori at 2012-11-26T15:00:22Z
is this final? if not, then +1 for message keys
---------------------------------------------------------------------------
by vicb at 2012-11-27T22:33:47Z
@fabpot I can't understand why we keep discussing this for months as this implementation use *both* keys and plain Englis, ie using keys is optional ( if it was not it would not be an issue according to #6129)
---------------------------------------------------------------------------
by asm89 at 2013-01-02T21:43:46Z
@fabpot @vicb I'll rebase this PR, fix the comments and refactor the message keys to use plain English + {{ }} syntax for the placeholders.
---------------------------------------------------------------------------
by asm89 at 2013-01-07T15:00:58Z
@fabpot If I fix this tonight, will it make the beta?
---------------------------------------------------------------------------
by fabpot at 2013-01-07T15:53:00Z
yes, definitely.
---------------------------------------------------------------------------
by asm89 at 2013-01-07T20:13:38Z
@fabpot I switched the implementation to English messages instead of message keys and fixed the final comments + rebased. Anything you want me to do after this?
Still happy with `getMessageKey()`?
This PR was merged into the master branch.
Commits
-------
6703fb5 added changelog entries
1997e2e fix phpdoc of UrlGeneratorInterface that missed some exceptions and improve language of exception message
f0415ed [Routing] made reference type fully BC and improved phpdoc considerably
7db07d9 [Routing] added tests for generating relative paths and network paths
75f59eb [Routing] add support for path-relative and scheme-relative URL generation
Discussion
----------
[2.2] [Routing] add support for path-relative URL generation
Tests pass: yes
Feature addition: yes
BC break: <del>tiny (see below)</del> NO
deprecations: NO
At the moment the Routing component only supports absolute and domain-relative URLs, e.g.
`http://example.org/user-slug/article-slug/comments` and
`/user-slug/article-slug/comments`.
But there are two link types missing: schema-relative URLs and path-relative URLs.
schema-relative: e.g. `//example.org/user-slug/article-slug/comments`
path-relative: e.g. `comments`.
Both of them would now be possible with this PR. I think it closes a huge gap in the Routing component.
Use cases are pretty common. Schema-relative URLs are for example used when you want to include assets (scripts, images etc) in a secured website with HTTPS. Path-relative URLs are the only option when you want to generate static files (e.g. documentation) that can be downloaded as an HTML archive. Such use-cases are currently not possible with symfony.
The calculation of the relative path based on the request path and target path is hightly unit tested. So it is really equivalent. I found several implemenations on the internet but none of them worked in all cases. Mine is pretty short and works.
I also added an optional parameter to the twig `path` function, so this feature can also be used in twig templates.
Ref: This implements path-relative URLs as suggested in #3908.
<del>[BC BREAK] The signature of UrlGeneratorInterface::generate changed to support scheme-relative and path-relative URLs. The core UrlGenerator is BC and does not break anything, but users who implemented their own UrlGenerator need to be aware of this change. See UrlGenerator::convertReferenceType.</del>
---------------------------------------------------------------------------
by jalliot at 2012-04-16T09:56:56Z
@Tobion For completeness, you should add the option to the `url` and `asset` twig functions/template helpers.
---------------------------------------------------------------------------
by stof at 2012-04-16T10:46:06Z
@jalliot adding the option to ``url`` does not make any sense. The difference between ``path`` and ``url`` is that ``path`` generates a path and ``url`` generates an absolute url (thus including the scheme and the hostname)
---------------------------------------------------------------------------
by Tobion at 2012-04-16T12:27:49Z
@stof I guess jalliot meant we could then generate scheme-relative URLs with `url`. Otherwise this would have no equivalent in twig.
---------------------------------------------------------------------------
by jalliot at 2012-04-16T12:34:08Z
@stof Yep I meant what @Tobion said :)
---------------------------------------------------------------------------
by Tobion at 2012-04-18T11:57:04Z
The $relative parameter I added besides the existing $absolute parameter of the `->generate` method was not clear enough. So I merged those into a different parameter `referenceType`. I adjusted all parts of symfony to use the new signature. And also made the default `UrlGenerator` implementation BC with the old style. So almost nobody will recognize a change. The only BC break would be for somebody who implemented his own `UrlGenerator` and did not call the parent default generator.
Using `referenceType` instead of a simple Boolean is much more flexible. It will for example allow a custom generator to support a new reference type like http://en.wikipedia.org/wiki/CURIE
---------------------------------------------------------------------------
by Tobion at 2012-04-18T13:34:58Z
ping @schmittjoh considering your https://github.com/schmittjoh/JMSI18nRoutingBundle/blob/master/Router/I18nRouter.php would need a tiny change
---------------------------------------------------------------------------
by schmittjoh at 2012-04-18T13:37:39Z
Can you elaborate the necessary change?
---------------------------------------------------------------------------
by Tobion at 2012-04-18T13:51:10Z
This PR changes the signature of `generate` to be able to generate path-relative and scheme-relative URLs. So it needs to be
`public function generate($name, $parameters = array(), $referenceType = self::ABSOLUTE_PATH)` and your implementation would need to change `if ($absolute && $this->hostMap) {` to `if (self::ABSOLUTE_URL === $referenceType && $this->hostMap) {`
I can do a PR if this gets merged.
---------------------------------------------------------------------------
by schmittjoh at 2012-04-18T13:52:14Z
If I understand correctly, the old parameter still works, no?
edit: Ah, ok I see what you mean now.
---------------------------------------------------------------------------
by Tobion at 2012-04-18T13:56:33Z
Yeah the old parameter still works but $absolute would also evaluate to true (a string) in your case for non-absolute URLs, i.e. paths.
---------------------------------------------------------------------------
by Tobion at 2012-04-19T21:09:46Z
ping @fabpot
---------------------------------------------------------------------------
by fabpot at 2012-04-20T04:30:18Z
Let's discuss that feature for 2.2.
---------------------------------------------------------------------------
by Tobion at 2012-04-20T10:40:59Z
What are your objections against it? It's already implemented, it works and it adds support for things that are part of a web standard. The BC break is tiny at the moment (almost nobody is affected) because the core UrlGenerator works as before. But if we waited for 2.2 it will be much harder to make the transition because 2.1 is LTS. So I think is makes sense to add it now. Furthermore it makes it much more future-proof as custom generators can more easiliy add support for other link types like CURIE. At the moment a Boolean for absolute URLs is simply too limited and also somehow inconsistent because $absolute = false stands for an absolute path. You see the awkwardness in this naming.
Btw, I added a note in the changelog. And I will add documentation of this feature in symfony-docs once this is merged.
---------------------------------------------------------------------------
by fabpot at 2012-04-20T12:14:32Z
nobody has ever said that 2.1 would be LTS. Actually, I think we are going to wait for 2.3 for LTS.
---------------------------------------------------------------------------
by Tobion at 2012-04-20T12:27:18Z
Well what I meant is, the longer we wait with this, the harder to apply it.
In 04ac1fdba2 you modified `generate` signature for better extensibility that is not even made use of. I think changing `$abolute` param goes in the same direction and has direct use.
I'd like to know your reason to wait for 2.2. Not enough time to review it, or afraid of breaking something, or marketing for 2.2?
---------------------------------------------------------------------------
by stof at 2012-04-20T16:28:27Z
@Tobion the issue is that merging new features forces to postpone the release so that it is tested by enough devs first to be sure there is no blocking bug in it. Big changes cannot be merged when we are hunting the remaining bugs to be able to release.
---------------------------------------------------------------------------
by schmittjoh at 2012-04-20T16:42:11Z
Considering the changes that have been made to the Form component, and are still being made, I think this is in comparison to that a fairly minor change.
Maybe a clearer guideline on the release process, or the direction would help, and avoid confusion, or wrong expectations on contributors' part.
---------------------------------------------------------------------------
by Tobion at 2012-10-05T13:52:11Z
@fabpot this is ready. So if you agree with it, I would create a documentation PR.
---------------------------------------------------------------------------
by stof at 2012-10-13T16:09:47Z
@fabpot what do you think about this PR ?
---------------------------------------------------------------------------
by Crell at 2012-11-01T16:05:01Z
This feels like it's overloading the generate() method to do double duty: One, make a URl based on a route. Two, make a URI based on a URI snippet. Those are two separate operations. Why not just add a second method that does the second operation and avoid the conditionals? (We're likely to do that in Drupal for our own generator as well.)
---------------------------------------------------------------------------
by Tobion at 2012-11-01T16:38:39Z
@crell: No, you must have misunderstood something. The generate method still only generates a URI based on a route. The returned URI reference can now also be a relative path and a network path. Thats all.
---------------------------------------------------------------------------
by Tobion at 2012-12-13T18:30:28Z
@fabpot this is ready. It is fully BC! I also improved phpdoc considerably.
---------------------------------------------------------------------------
by Tobion at 2012-12-14T20:51:38Z
@fabpot Do you want me to write documentation for it? I would also be interested to write about the new features of the routing component in general. I wanted to do that anyway and it would probably be a good fit for your "new in symfony" articles.
---------------------------------------------------------------------------
by fabpot at 2012-12-14T20:58:16Z
Im' going to review this PR in the next coming days. And to answer your second question, more documentation or better documentation is always a good thing, so go for it.
---------------------------------------------------------------------------
by Tobion at 2013-01-02T21:50:20Z
@fabpot ping. I added changelog entries.
This PR was merged into the master branch.
Commits
-------
586a16e [Validator] Changed DefaultTranslator::getLocale() to always return 'en'
58bfd60 [Validator] Improved the inline documentation of DefaultTranslator
cd662cc [Validator] Added ExceptionInterface, BadMethodCallException and InvalidArgumentException
e00e5ec [Validator] Fixed failing test
cc0df0a [Validator] Changed validator to support pluralized messages by default
56d61eb [Form][Validator] Added BC breaks in unstable code to the CHANGELOG
1e34e91 [Form] Added upgrade instructions to the UPGRADE file
b94a256 [DoctrineBridge] Adapted DoctrineBridge to translator integration in the validator
c96a051 [FrameworkBundle] Adapted FrameworkBundle to translator integration in the validator
92a3b27 [TwigBridge] Adapted TwigBridge to translator integration in the validator
e7eb5b0 [Form] Adapted Form component to translator integration in the validator
46f751c [Validator] Extracted message interpolation logic of ConstraintViolation and used the Translation component for that
Discussion
----------
[Validator] Integrated the Translator in the Validator component
Bug fix: no
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #5844, #6117
Todo: -
License of the code: MIT
Documentation PR: -
This PR allows to replace the default message substitution strategy in the validator (`strtr()`) by passing an implementation of `Symfony\Component\Translation\TranslatorInterface`. The motivation for this are both #5844 and the need to replace the translation strategy in Drupal's integration of the Validator.
In the stand-alone usage of the validator, both the translator and the default translation domain can now be passed to `ValidatorBuilderInterface`:
```php
$validator = Validation::createValidatorBuilder()
->setTranslator(new MyTranslator())
->setTranslationDomain('validators')
->getValidator();
```
References:
* #5844
* #6117
* #6129
* [Add a validation framework to Drupal 8](http://drupal.org/node/1845546)
* [Add the symfony validator component to core despite Symfony potentially releasing BC-breaking updates after 2.3.](http://drupal.org/node/1849564)
---------------------------------------------------------------------------
by Tobion at 2012-11-28T08:53:25Z
no BC break? Looking at ValidatorBuilderInterface there is definitely one.
---------------------------------------------------------------------------
by bschussek at 2012-11-28T08:55:01Z
ValidatorBuilderInterface is not part of the stable API. You are not supposed to implement this interface.
---------------------------------------------------------------------------
by Tobion at 2012-11-28T09:01:07Z
We're not only documenting bc breaks for stable API, otherwise we could remove 90% of the upgrade file since few methods are tagged with API.
An interface that nobody should implement?
---------------------------------------------------------------------------
by bschussek at 2012-11-28T09:30:02Z
The question is what to consider a BC break. Something will always break for someone. Should we consequently mark everything as BC break? I don't think so.
For example, since 2.1, you are supposed to use `Validation::createValidator*()` for creating a validator. Because of that, I won't consider changing the constructor signature of `Validator` a BC break anymore from 2.1 on.
The same for the unstable interfaces. These are currently meant to be used only, that is, type hint against them and call their methods. But we don't guarantee that we won't add methods to them.
---------------------------------------------------------------------------
by Tobion at 2012-11-28T09:38:19Z
I agree that almost any change could be considered a BC break. So we probably need to better define what a BC break is and what not. Otherwise Symfony will stop evolving after 2.3 because from then on Fabien wanted to prevent BC breaks which is almost impossible in a strict definition of bc break.
---------------------------------------------------------------------------
by fabpot at 2012-11-28T11:37:22Z
BC breaks should always be documented, and we guarantee BC only for things tagged with @api. I'm going to update the docs to make things clearer.
---------------------------------------------------------------------------
by bschussek at 2012-11-28T13:09:57Z
@fabpot I documented these changes now in the CHANGELOG: af99ebb1206ac92889b7193ba1ecc12bf2617e85
Are we sure we want to document *all* BC breaks from now on, even in non-@api code? This could rather scare people looking at our changelogs (lots of BC BREAKS there).
---------------------------------------------------------------------------
by fago at 2012-11-28T17:29:58Z
Unfortunately, it turns out the symfony translator interface does not mach the Drupal translation system as well as we initally thought, see http://drupal.org/node/1852106. Given that, this would integrating the validator component into Drupal even harder, because it introduces the dependency on the (unwanted) translation component. :(
---------------------------------------------------------------------------
by stof at 2012-11-28T18:19:36Z
If this does not help Drupal anyway, maybe #5844 is a better way to manage translations for people using the validator outside forms ?
and the Drupal guys would simply follow a similar approach, but based on their own translator instead of the symfony one.
---------------------------------------------------------------------------
by fago at 2012-11-28T18:50:12Z
Yeah. The only problem I see with the approach of #5844 is that *after* validation only the translated messages are available. We'd need to have access to the untranslated messages also.
---------------------------------------------------------------------------
by fago at 2012-11-29T09:49:47Z
As our translation system handles translating pluralized messages differently, the current ExecutionContextInterface::addViolation() method poses a problem also. We need to pass on - both the single and plural - message, as the message gets chosen during translation, see http://api.drupal.org/api/drupal/core!includes!common.inc/function/format_plural/8
So maybe, we could allow adding an already created ConstraintViolation object also? Then, we could implement a "PluralConstraintViolation" class that takes both message templates.
---------------------------------------------------------------------------
by bschussek at 2012-12-03T15:52:36Z
I updated this PR to support pluralized messages by default in the validator. This should solve the problem of the Drupal guys, because their implementation of `TranslatorInterface::transChoice($id, $number, ...)` can now simply split the $id by pipes (`|`) and pass the parts to their own `format_plural($count, $singular, $plural, ...)` function.
For us, it breaks BC because translation catalog sources had to be adapted.
---------------------------------------------------------------------------
by fabpot at 2012-12-03T16:25:52Z
Most of the XLF files are broken (the end is missing now).
IIUC, we now have a hard dependency on the Translation component, which is something we wanted to avoid.
---------------------------------------------------------------------------
by fabpot at 2012-12-03T16:27:56Z
Oops, clicked on the "comment" button too fast.
So, the dependency is hard (you need to install the dep) but light as we only rely on the translation interface from the component (when using the default translator). It looks acceptable to me, especially because we now use Composer to manage dependencies.
---------------------------------------------------------------------------
by bschussek at 2012-12-03T16:54:10Z
@fabpot Thanks for the hint. Going to fix this.
---------------------------------------------------------------------------
by bschussek at 2012-12-04T11:34:43Z
@fabpot Fixed.
---------------------------------------------------------------------------
by bschussek at 2012-12-07T12:40:24Z
Is there anything missing for this PR to be merged?
* 2.1:
[Console] Fix style escaping parsing
[Console] Make style formatter matching less greedy to avoid having to escape when not needed
[Bundle] [FrameworkBundle] fixed indentation in esi.xml services file.
[Component] [Security] fixed PSR-2 coding violation in ClassUtilsTest class.
[Form] Fixed EntityChoiceList when loading objects with negative integer IDs
[TwigBundle] There is no CSS visibility of display, should be visible instead
[Form] corrected source node for a Danish translation
[DependencyInjection] fixed a bug where the strict flag on references were lost (closes#6607)
[HttpFoundation] Check if required shell functions for `FileBinaryMimeTypeGuesser` are not disabled
[CssSelector] added css selector with empty string
[HttpFoundation] Docblock for Request::isXmlHttpRequest() now points to Wikipedia
[DependencyInjection] refactored code to avoid logic duplication
[Form] Deleted references in FormBuilder::getFormConfig() to improve performance
[HttpFoundation] Update docblock for non-working method
Conflicts:
src/Symfony/Bundle/TwigBundle/Resources/views/Exception/trace.html.twig
src/Symfony/Bundle/TwigBundle/Resources/views/Exception/traces.html.twig
* 2.0:
[Bundle] [FrameworkBundle] fixed indentation in esi.xml services file.
[TwigBundle] There is no CSS visibility of display, should be visible instead
[DependencyInjection] fixed a bug where the strict flag on references were lost (closes#6607)
[HttpFoundation] Check if required shell functions for `FileBinaryMimeTypeGuesser` are not disabled
[CssSelector] added css selector with empty string
[HttpFoundation] Docblock for Request::isXmlHttpRequest() now points to Wikipedia
[DependencyInjection] refactored code to avoid logic duplication
Conflicts:
src/Symfony/Bundle/FrameworkBundle/Resources/config/esi.xml
src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
src/Symfony/Component/HttpFoundation/File/MimeType/FileBinaryMimeTypeGuesser.php
* 2.1: (24 commits)
updated license year
Update src/Symfony/Component/HttpFoundation/Response.php
[Form] Fixed inheritance of "error_bubbling" in RepeatedType
[Form] Fixed DateType when used with the intl extension disabled.
[HttpFoundation] fix return types and handling of zero in Response
[HttpFoundation] better fix for non-parseable Expires header date
Fixed missing plural message in portuguese validator
Fix Expires when the header is -1
[DoctrineBridge] Allowing memcache port to be 0 to support memcache unix domain sockets.
[Console] fixed unitialized properties (closes#5935)
[Process] Prevented test from failing when pcntl extension is not enabled.
Revert "[DoctrineBridge] Improved performance of the EntityType when used with the "query_builder" option"
[Form] Fixed failing tests for DateTimeToStringTransformer.
[Locale] Fixed the StubLocaleTest for ICU versions lower than 4.8.
[Bundle] [FrameworkBundle] fixed typo in phpdoc of the SessionListener.
[Form] Fixed test regression introduced in #6440
[Tests] Fix namespaces
Fixed php doc of GenericEvent::__construct
HttpUtils must handle RequestMatcher too
use preferred_choices in favor of preferred_query
...
Conflicts:
src/Symfony/Bridge/Propel1/Form/ChoiceList/ModelChoiceList.php
* 2.0:
updated license year
Update src/Symfony/Component/HttpFoundation/Response.php
[Console] fixed unitialized properties (closes#5935)
[Bundle] [FrameworkBundle] fixed typo in phpdoc of the SessionListener.
bumped Symfony version to 2.0.21-DEV
updated VERSION for 2.0.21
updated CHANGELOG for 2.0.21
Conflicts:
src/Symfony/Bundle/SwiftmailerBundle/LICENSE
src/Symfony/Component/Filesystem/LICENSE
src/Symfony/Component/HttpFoundation/Response.php
src/Symfony/Component/HttpKernel/Kernel.php
This PR was merged into the master branch.
Commits
-------
5e359d3 made the kernel optional in all data collectors
Discussion
----------
made the kernel optional in all data collectors
* 2.1:
fixed typo
[FrameworkBundle] fixed ESI calls
[FrameworkBundle] fixed ESI calls
bumped Symfony version to 2.1.6-DEV
updated VERSION for 2.1.5
updated CHANGELOG for 2.1.5
bumped Symfony version to 2.0.21-DEV
[FrameworkBundle] fixed trusted_proxies configuration for some edge cases
[FrameworkBundle] fixed XSD for the trusted-proxies setting
updated VERSION for 2.0.20
update CONTRIBUTORS for 2.0.20
updated CHANGELOG for 2.0.20
Conflicts:
src/Symfony/Bundle/FrameworkBundle/HttpKernel.php
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
src/Symfony/Component/HttpKernel/Kernel.php
* 2.0:
bumped Symfony version to 2.0.21-DEV
[FrameworkBundle] fixed trusted_proxies configuration for some edge cases
[FrameworkBundle] fixed XSD for the trusted-proxies setting
updated VERSION for 2.0.20
update CONTRIBUTORS for 2.0.20
updated CHANGELOG for 2.0.20
Conflicts:
CONTRIBUTORS.md
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/ConfigurationTest.php
src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Fixtures/xml/full.xml
src/Symfony/Component/HttpKernel/Kernel.php
* 2.1:
[FrameworkBundle] added support for URIs as an argument to HttpKernel::render()
[FrameworkBundle] restricted the type of controllers that can be executed by InternalController
[Process] Allow non-blocking start with PhpProcess
Making it easier to grab the PR template.
[Locale] fixed a test
Fixed failing test
fix double-decoding in the routing system
Conflicts:
src/Symfony/Component/Process/PhpProcess.php
* 2.0:
[FrameworkBundle] added support for URIs as an argument to HttpKernel::render()
[FrameworkBundle] restricted the type of controllers that can be executed by InternalController
Making it easier to grab the PR template.
fix double-decoding in the routing system
Conflicts:
README.md
src/Symfony/Bundle/FrameworkBundle/EventListener/RouterListener.php
src/Symfony/Component/Security/Http/HttpUtils.php
This PR was merged into the 2.0 branch.
Commits
-------
8b2c17f fix double-decoding in the routing system
Discussion
----------
fix double-decoding in the routing system
@fabpot @vicb This should fix it. You know what ;) Don't want to leak more information.
And the good thing, it's no hack nor does it break BC.
* 2.1:
[FrameworkBundle] fixed broken tests
[FrameworkBundle] Fixed logic under test environment.
[Session] Added exception to save method
[Session] Fixed a bug with the TestListener
Added comment
[FrameworkBundle] Added tests for trusted_proxies configuration.
[FrameworkBundle] Added a check on file mime type for CodeHelper::fileExcerpt()
checked for a potentially missing key
[FrameworkBundle] used the new method for trusted proxies
remove realpath call
Conflicts:
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
* 2.0:
Added comment
[FrameworkBundle] Added tests for trusted_proxies configuration.
[FrameworkBundle] Added a check on file mime type for CodeHelper::fileExcerpt()
checked for a potentially missing key
[FrameworkBundle] used the new method for trusted proxies
remove realpath call
Conflicts:
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Configuration.php
This PR was merged into the 2.0 branch.
Commits
-------
f0743b1 Merge pull request #1 from pylebecq/2.0
555e777 [FrameworkBundle] Added tests for trusted_proxies configuration.
a0e2391 [FrameworkBundle] used the new method for trusted proxies
Discussion
----------
[FrameworkBundle] used the new method for trusted proxies
This makes the framework bundle using the new method from the request class.
---------------------------------------------------------------------------
by fabpot at 2012-12-05T10:38:20Z
As this is a sensitive issue, can you add some tests? Thanks.
---------------------------------------------------------------------------
by bamarni at 2012-12-06T13:00:24Z
Well I don't know why it fails on travis, I can't run the full test suite locally because of a segfault but ```phpunit src/Symfony/Bundle/``` marks all the tests as passing.
---------------------------------------------------------------------------
by fabpot at 2012-12-06T13:08:11Z
But it looks like the failing tests come from what you've changed.
---------------------------------------------------------------------------
by bamarni at 2012-12-06T13:29:33Z
Yes, I'm not saying it's not my fault but I can't reproduce this as locally it tells me they pass, I'll try to fix this this evening.
---------------------------------------------------------------------------
by bamarni at 2012-12-06T17:49:28Z
Apparently it fails only when running the whole testsuite, looking at other travis builds I can see this one on 2.0 : https://travis-ci.org/symfony/symfony/jobs/3495511 which fails in a similar way than here (https://travis-ci.org/symfony/symfony/jobs/3530928). Because of a place trying to access an undefined $_SERVER key : ```PHP Notice: Undefined index: SCRIPT_NAME ...``` but I can't find where, and the stack trace references some phpunit classes.
I'd be happy if someone could give me some pointers in here as I don't have any clue about how to fix this..
---------------------------------------------------------------------------
by bamarni at 2012-12-06T18:00:57Z
As a consulsion I'd say I can't run the whole testsuite locally (it fails even when I revert my commit), so there is no reliable way for me to fix this, if anyone is up for continuing this feel free.
---------------------------------------------------------------------------
by fabpot at 2012-12-11T09:47:48Z
@bamarni Can you just update this PR with the code change and no tests at all? I will then finish the PR. Thanks.
---------------------------------------------------------------------------
by bamarni at 2012-12-11T16:58:17Z
@fabpot: thanks for helping me out on this, hope you won't run into the same issue!
This PR was merged into the master branch.
Commits
-------
74a8fcf [FrameworkBundle] Added support for default templates per render tag
Discussion
----------
[FrameworkBundle] Added support for default templates per render tag
This commit allows you to specify default templates per render tag when using hinclude.
E.G:
The following will use the specific default template for the render:
```` {% render "AcmeDemoBundle:Controller:action" with {}, {"standalone" : "js", "default" : "AcmeDemoBundle:Default:content.html.twig"} %}````
or if you don't want to use a template for the default content but just a string, you can do the following
```` {% render "AcmeDemoBundle:Controller:action" with {}, {"standalone" : "js", "default" : "Loading..."} %}````
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Fixes the following tickets: #3356
Todo: -
Documentation
---------------------------------------------------------------------------
by fabpot at 2012-12-14T12:25:40Z
Looks good to me. Can you add a note in the CHANGELOG of the component and send a PR on symfony/symfony-docs about this new feature? Thanks.
---------------------------------------------------------------------------
by pierredup at 2012-12-14T14:30:00Z
@fabpot done, documentation PR symfony/symfony-docs#2021
* 2.1:
[Console] Add support for parsing terminal width/height on localized windows, fixes#5742
[Form] Fixed treatment of countables and traversables in Form::isEmpty()
refactor ControllerNameParser
[Form] Fixed FileType not to throw an exception when bound empty
- Test undefined index #
Maintain array structure
Check if key # is defined in $value
Update src/Symfony/Component/Validator/Resources/translations/validators.pl.xlf
This PR was merged into the master branch.
Commits
-------
d5426f0 [Form] Add tests to prove that label is not rendered when is marked as false
120547c [Form][TwigBridge] Don't set label attributes if is marked as not to be rendered [Form][FrameworkBundle] Add option to disable rendering of label for fields
36e4556 [Form] Option for not displaying a label by setting label to false. [Form] Fixed formatting & translation ..
Discussion
----------
[Form] Added option for not displaying a form-label by setting label to false
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Replaces: #5421
@fabpot @BenjaminBeck: I was just not sure what to do with "table based" forms, so I left `<td></td>` rendered when there is no label, because I'm not sure that we can hide it easily.
---------------------------------------------------------------------------
by XWB at 2012-12-11T09:30:14Z
👍
* 2.1:
fixed CS
fixed CS
[Security] fixed path info encoding (closes#6040, closes#5695)
[HttpFoundation] added some tests for the previous merge and removed dead code (closes#6037)
Improved Cache-Control header when no-cache is sent
removed unneeded comment
Fix to allow null values in labels array
fix date in changelog
removed the Travis icon (as this is not stable enough -- many false positive, closes#6186)
Revert "merged branch gajdaw/finder_splfileinfo_fpassthu (PR #4751)" (closes#6224)
Fixed a typo
Fixed: HeaderBag::parseCacheControl() not parsing quoted zero correctly
[Form] Fix const inside an anonymous function
[Config] Loader::import must return imported data
[DoctrineBridge] Fixed caching in DoctrineType when "choices" or "preferred_choices" is passed
[Form] Fixed the default value of "format" in DateType to DateType::DEFAULT_FORMAT if "widget" is not "single_text"
[HttpFoundation] fixed a small regression
Conflicts:
src/Symfony/Component/HttpFoundation/Tests/Session/Storage/Handler/MongoDbSessionHandlerTest.php
This PR was merged into the master branch.
Commits
-------
7f16c1f [HttpKernel] Add DI extension configs as ressources when possible
Discussion
----------
[HttpKernel] Add DI extension configs as ressources when possible
/cc @rdohms @richardmiller
---------------------------------------------------------------------------
by vicb at 2012-11-30T11:57:48Z
btw @fabpot what about having a base class for `Extension` in the DI ? Would make it easier to re-use it when using standalone components, Di and (the suggested) Config as the greatest part of the class is not HttpKernel specific.
---------------------------------------------------------------------------
by fabpot at 2012-12-06T08:47:28Z
@vicb your suggestion makes sense.
Can you also explain the goal of this PR?
---------------------------------------------------------------------------
by vicb at 2012-12-06T09:01:58Z
The goal of this PR is to avoid having to sfcc when you modify a DI extension configuration. I think @rdohms got trapped.
---------------------------------------------------------------------------
by vicb at 2012-12-06T09:08:08Z
see https://twitter.com/rdohms/status/274059267428978688
---------------------------------------------------------------------------
by stof at 2012-12-06T09:20:54Z
I thought about it several times but never took time to implement it. It is annoying to have to clear the cache when you modify a default value in the Configuration class. So +1
This PR was squashed before being merged into the master branch (closes#6083).
Commits
-------
6236c18 [FrameworkBundle] Added caching to TemplateController
Discussion
----------
[FrameworkBundle] Added caching to TemplateController
Because the main purpose for the `TemplateController` seems to be to render static pages like "disclaimer" and such, it seems useful to allow caching.
imprint:
pattern: /imprint
defaults:
_controller: SymfonyFrameworkBundle:Template:template
template: "::pages/imprint.html.twig"
maxAge: 86400
---------------------------------------------------------------------------
by pierredup at 2012-11-21T20:24:53Z
IMHO I think the caching should be allowed to be set optionally
---------------------------------------------------------------------------
by KingCrunch at 2012-11-21T20:38:54Z
I wrote it this way, because I assume, that it will cover more use-cases, than the other way round, but you are right, that this will change the current behaviour. Would like to hear other opinions, because I don't think one uses this action for anything else than fully-static content (means: The current behaviour doesn't feel very useful to me).
---------------------------------------------------------------------------
by pierredup at 2012-11-21T20:48:19Z
I totally agree, but I would then suggest keep the caching on by default, but have the option to turn it off if necessary
---------------------------------------------------------------------------
by pierredup at 2012-11-21T20:52:01Z
Actually I think to have caching permanently enabled for static content would probably be the best scenario, but I like to think in terms of flexibility and specific user requirements. It would be great to get some opinions from others on this
---------------------------------------------------------------------------
by KingCrunch at 2012-11-23T21:12:45Z
I thought about it and I come to the conclusion, that it is probably a not so good idea to enable caching by default, because ... well, it's not possible to disable it again. I guess something like this
{{ render '@AcmeBundle:ArticleController:latest' with {count: 1} }}
may be not so uncommon as I suggested in the first commit.
---------------------------------------------------------------------------
by fabpot at 2012-12-03T22:18:51Z
Can you make a PR for the docs? (symfony/symfony-docs). Thanks.
This PR was squashed before being merged into the master branch (closes#6173).
Commits
-------
4878ec0 [HttpKernel] [WebProfilerBundle] Better handling of deprecated methods
Discussion
----------
[HttpKernel] [WebProfilerBundle] Better handling of deprecated methods
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes, if you were expecting E_USER_DEPRECATED or E_DEPRECATED to throw an exception
Symfony2 tests pass: yes
Fixes the following tickets: #6139 partly, I'd go through and add the actual trigger_error() calls in another (or possibly one per component) PR
Todo: call trigger_error()
License of the code: MIT
Documentation PR: -
I added the deprecation count with the Exception icon in the Profiler Toolbar, and changed the color of it to be yellow for deprecations and red for exceptions (was yellow for exceptions).
---------------------------------------------------------------------------
by fabpot at 2012-12-03T09:43:09Z
Adding trigger_error calls should be done in one PR to ease the merging. thanks.
This PR was merged into the master branch.
Commits
-------
51223c0 added upgrade instructions
50e6259 adjusted tests
98f3ca8 [Routing] removed tree structure from RouteCollection
Discussion
----------
[Routing] removed tree structure from RouteCollection
BC break: yes (see below)
Deprecations: RouteCollection::getParent(); RouteCollection::getRoot()
tests pass: yes
The reason for this is so quite simple. The RouteCollection has been designed as a tree structure, but it cannot at all be used as one. There is no getter for a sub-collection at all. So you cannot access a sub-collection after you added it to the tree with `addCollection(new RouteCollection())`. In contrast to the form component, e.g. `$form->get('child')->get('grandchild')`.
So you can see the RouteCollection cannot be used as a tree and it should not, as the same can be achieved with a flat array!
Using a flat array removes all the need for recursive traversal and makes the code much faster, much lighter, less memory (big problem in CMS with many routes) and less error-prone.
BC break: there is only a BC break if somebody used the PHP API for defining RouteCollection and also added a Route to a collection after it has been added to another collection.
So
```
$rootCollection = new RouteCollection();
$subCollection = new RouteCollection();
$rootCollection->addCollection($subCollection);
$subCollection->add('foo', new Route('/foo'));
```
must be updated to the following (otherwise the 'foo' Route is not imported to the rootCollection)
```
$rootCollection = new RouteCollection();
$subCollection = new RouteCollection();
$subCollection->add('foo', new Route('/foo'));
$rootCollection->addCollection($subCollection);
```
Also one must call addCollection from the bottom to the top. So the correct sequence is the following (and not the reverse)
```
$childCollection->->addCollection($grandchildCollection);
$rootCollection->addCollection($childCollection);
```
Remeber, this is only needed when using PHP for defining routes and calling methods in a special order. There is no change required when using XML or YAML for definitions. Also, I'm pretty sure that neither the CMF, nor Drupal routing, nor Silex is relying on the tree stuff. So they should also still work.
cc @fabpot @crell @dbu
One more thing: RouteCollection wasn't an appropriate name for a tree anyway as a collection of routes (that it now is) is definitely not a tree.
Yet another point: The XML declaration of routes uses the `<import>` element, which is excatly what the new implementation of addCollection without the need of a tree does. So this is now also more analogous.
---------------------------------------------------------------------------
by Koc at 2012-11-26T17:34:15Z
What benefit of this?
---------------------------------------------------------------------------
by Tobion at 2012-11-26T17:56:53Z
@Koc Why did you not simply wait for the description? ^^
---------------------------------------------------------------------------
by dbu at 2012-11-26T18:33:09Z
i love PR that remove more code than they add whithout removing functionality.
---------------------------------------------------------------------------
by Crell at 2012-11-26T18:49:52Z
There's an issue somewhere in Drupal where we're trying to use addCollection() as a shorthand for iterating over one collection and calling add() on the other for each item. We can't do that, however, because the subcollections are not flattened properly when reading back and our current dumper can't cope with that. So this change would not harm Drupal at all, and would mean I don't have fix a bug in our dumper. :-) I cannot speak for any other projects, of course.
---------------------------------------------------------------------------
by Tobion at 2012-11-27T19:06:34Z
Ok, this is ready.
* 2.1: (29 commits)
[DependencyInjection] fixed composer.json
[Validator] Fix typos in validators.ru.xlf
Edited some minor grammar and style errors in russian validation file
Updated Bulgarian translation
[Form] improve error message with a "hasser" hint for PropertyAccessDeniedException
[Form] Updated checks for the ICU version from 4.5+ to 4.7+ due to test failures with ICU 4.6
[Form] simplified a test from previous merge
Update src/Symfony/Component/Form/Extension/Core/Type/FileType.php
fixed CS
Xliff with other node than source or target are ignored
small fix of #5984 when the container param is not set
Filesystem Component mirror symlinked directory fix
[Process][Tests] fixed chainedCommandsOutput tests
fixed CS
Use better default ports in urlRedirectAction
Add tests for urlRedirectAction
info about session namespace
fix upgrade info about locale
Update src/Symfony/Component/DomCrawler/Tests/FormTest.php
Update src/Symfony/Component/DomCrawler/Form.php
...
* 2.0:
[DependencyInjection] fixed composer.json
[Form] Updated checks for the ICU version from 4.5+ to 4.7+ due to test failures with ICU 4.6
fixed CS
small fix of #5984 when the container param is not set
fixed CS
Use better default ports in urlRedirectAction
Add tests for urlRedirectAction
Update src/Symfony/Component/DomCrawler/Tests/FormTest.php
Update src/Symfony/Component/DomCrawler/Form.php
[Security] remove escape charters from username provided by Digest DigestAuthenticationListener
[Security] added test extra for digest authentication
fixed CS
[Security] Fixed digest authentication
[Security] Fixed digest authentication
[SecurityBundle] Convert Http method to uppercase in the config
Use Norm Data instead of Data
Conflicts:
src/Symfony/Bridge/Doctrine/Form/EventListener/MergeCollectionListener.php
src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php
src/Symfony/Component/DependencyInjection/composer.json
this can happen when the config for the router is unset, but this method
does not need to depend on routing. reading an unset config would raise an exception.
This PR was merged into the master branch.
Commits
-------
9f1cd84 moved most static assets directly into the templates
Discussion
----------
moved most static assets directly into the templates
This has been done for several reasons:
* for consistency with the way we already manage the WDT and the
profiler icons;
* it makes the Exception independent from the location of the assets
(and from the asset() function)
* this is the second step to make the WebProfiler useable outside the
full-stack framework
see dbcd171dd3
---------------------------------------------------------------------------
by stof at 2012-11-13T17:52:39Z
the images blue_picto_more and blue_picto_less are also used in DoctrineBundle (and probably in JMSDebuggingBundle but I haven't checked). Could you submit a PR inlining the images too before removing them ?
---------------------------------------------------------------------------
by fabpot at 2012-11-13T18:02:18Z
Any image useful for other bundles should not be deleted. So, if DoctrineBundle is using the blue_pico_* images, I'm going to revert their deletion.
---------------------------------------------------------------------------
by schmittjoh at 2012-11-13T18:07:02Z
I'm using them in two bundles, JMSDebuggingBundle and JMSJobQueueBundle (same for the exception related templates).
---------------------------------------------------------------------------
by dlsniper at 2012-11-13T22:22:29Z
Wouldn't it be better to have the encoded icons in a separate CSS file as different classes/background images?
---------------------------------------------------------------------------
by fabpot at 2012-11-14T11:28:24Z
ok, I've reverted the removal of the images that might be useful in third-party bundles.
This has been done for several reasons:
* for consistency with the way we already manage the WDT and the
profiler icons;
* it makes the Exception independent from the location of the assets
(and from the asset() function)
* this is the second step to make the WebProfiler useable outside the
full-stack framework
see dbcd171dd3
* 2.1: (24 commits)
forced Travis to use source to workaround their not-up-to-date Composer on PHP 5.3.3
[Routing] removed irrelevant string cast in Route
Fixed typo
Make YamlFileLoader and XmlFileLoader file loading extensible
[HttpKernel] fix typo
Fixed singularization of "prices"
[Form] Removed an exception that prevented valid formats from being passed, e.g. "h" for the hour, "L" for the month etc.
[HttpKernel] fixed Client when using StreamedResponses (closes#5370)
fixed PDO session handler for Oracle (closes#5829)
[HttpFoundation] fixed PDO session handler for Oracle (closes#5829)
[Locale] removed a check that is done too early (and it is done twice anyways)
Update src/Symfony/Component/Validator/Resources/translations/validators.fa.xlf
Adding new localized strings for farsi validation.
[HttpFoundation] moved the HTTP protocol check from StreamedResponse to Response (closes#5937)
[Form] Fixed forms not to be marked invalid if their children are already marked invalid
[Form] Excluded some tests in NumberToLocalizedStringTransformerTest which fail on ICU 4.4, but work on ICU 4.8
added missing tests from previous merge
[Form] Fixed NumberToLocalizedStringTransformer to accept both comma and dot as decimal separator, if possible
Fix export-ignore on Windows
Show correct class name InputArgument in error message
...
Conflicts:
.travis.yml
src/Symfony/Component/Form/Extension/Core/DataTransformer/NumberToLocalizedStringTransformer.php
The goal is to make things more decoupled and more reusable across
different bundles.
There will be a PR for the distribution bundle too to simplify the code
based on this PR.
This PR was merged into the master branch.
Commits
-------
56fe8d1 duplicated the code helper code to the Twig bundle
Discussion
----------
moved code helper code to the Twig bundle
These helpers are very specific and are only used in TwigBundle for the
profiler and the exception templates.
---------------------------------------------------------------------------
by schmittjoh at 2012-11-12T09:12:56Z
Is there a reason for this BC break other than a cosmetical tweak?
If not strictly necessary, I'd like to see these kind of changes being scheduled for 3.0.
---------------------------------------------------------------------------
by fabpot at 2012-11-12T09:29:35Z
Of course, I don't want to make this change without a reason and indeed, I forgot to mention the why of this change. Let me explain.
I've been working on the integration of the Symfony web profiler into other OSS that use HttpKernel like Silex and Drupal for quite some time now. That's why I developed the new namespace feature in Twig, that's why I've refactored the web profiler to only use Twig and not the templating component. One of the last step is this PR, which reduces the number of dependencies to be able to use the WebProfiler bundle.
So, this change is not cosmetic, but one more step towards the goal of making the web profiler more reusable.
---------------------------------------------------------------------------
by schmittjoh at 2012-11-12T13:22:28Z
I see, makes sense. How about duplicating the code for now?
After looking through the history, it doesn't seem like it changes often
(the last real code change was a year ago). So, it should not be much more
maintenance effort, and we could keep BC here.
On Mon, Nov 12, 2012 at 10:29 AM, Fabien Potencier <notifications@github.com
> wrote:
> Of course, I don't want to make this change without a reason and indeed, I
> forgot to mention the why of this change. Let me explain.
>
> I've been working on the integration of the Symfony web profiler into
> other OSS that use HttpKernel like Silex and Drupal for quite some time
> now. That's why I developed the new namespace feature in Twig, that's why
> I've refactored the web profiler to only use Twig and not the templating
> component. One of the last step is this PR, which reduces the number of
> dependencies to be able to use the WebProfiler bundle.
>
> So, this change is not cosmetic, but one more step towards the goal of
> making the web profiler more reusable.
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/5986#issuecomment-10281201>.
>
>
---------------------------------------------------------------------------
by fabpot at 2012-11-12T13:32:33Z
Of course, that's a possibility. But what for? I doubt that people are using this code. Are you using this code somewhere?
---------------------------------------------------------------------------
by schmittjoh at 2012-11-12T13:37:24Z
Yes, I believe that I'm using it both in JMSDebuggingBundle, and
JMSJobQueueBundle as both are rendering exception stack traces at some
point.
On Mon, Nov 12, 2012 at 2:32 PM, Fabien Potencier
<notifications@github.com>wrote:
> Of course, that's a possibility. But what for? I doubt that people are
> using this code. Are you using this code somewhere?
>
> —
> Reply to this email directly or view it on GitHub<https://github.com/symfony/symfony/pull/5986#issuecomment-10287353>.
>
>
---------------------------------------------------------------------------
by fabpot at 2012-11-12T14:11:50Z
ok, fair enough. The code is now in the framework bundle as well.
The code has been duplicated and not moved for BC reasons.
This code has been duplicated in the Twig bundle to be able to decouple
the web profiler and the exception templates.
