Commits
-------
8404b35 Updated indonesian translations for trans-unit 47 and 48
Discussion
----------
Updated indonesian translations for trans-unit 47 and 48
Commits
-------
ee0fe7a Added guessers for Size and SizeLength constraints
Discussion
----------
Added guessers for Size and SizeLength constraints
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
---------------------------------------------------------------------------
by jalliot at 2011/09/30 13:40:37 -0700
BTW, I've noticed that some constraints currently don't have guessers in 2.0:
* ``False`` (which could be guessed as a checkbox)
* ``True`` (which could be guessed as a required checkbox)
* ``Choice`` (which could be guessed as a choice type with medium confidence and with the choice list being guessed as the list provided for the constraint)
Are there any reasons why this is not implemented in 2.0 or should I try to make a PR for it?
There is also the ``Collection`` case but I guess it would be too difficult for this one...
Commits
-------
f9b2be9 Added french translation for SizeLength and UserPassword constraints
Discussion
----------
Added french translation for SizeLength and UserPassword constraints
---------------------------------------------------------------------------
by fabpot at 2011/09/30 13:45:46 -0700
47 is already taken for "This value should be the user current password". Should be 48.
---------------------------------------------------------------------------
by jalliot at 2011/09/30 13:59:01 -0700
@fabpot Fixed and added translation for 47 as well.
Commits
-------
0e00e3f [DoctrineBundle] CS
0c4b793 [DoctrineBundle] Fixed performances issues on "On-demand" proxy file generation
e866a67 [DoctrineBundle] Tries to auto-generate the missing proxy files on the autoloader
Discussion
----------
[DoctrineBundle] Tries to auto-generate the missing proxy files on the autoloaded
See:
https://github.com/symfony/symfony/issues/1965https://github.com/symfony/symfony/issues/1535
This fix is not really clean and there's maybe a factorizing work to do on it, but this work and avoid me spending my day deleting session cookies each time I clear cache.
---------------------------------------------------------------------------
by stloyd at 2011/08/23 10:37:28 -0700
You should follow Symfony2 CS (http://symfony.com/doc/current/contributing/code/standards.html).
---------------------------------------------------------------------------
by ruudk at 2011/09/26 02:50:13 -0700
+1
---------------------------------------------------------------------------
by fabpot at 2011/09/27 07:01:51 -0700
It looks like a bug fix, so this PR should be closed and a new one based on the 2.0 branch should be open. @beberlei: are you fine with this patch?
---------------------------------------------------------------------------
by beberlei at 2011/09/29 04:24:22 -0700
What is this for? I dont understand the bug and the solution here screams cache slam.
---------------------------------------------------------------------------
by beberlei at 2011/09/29 04:34:02 -0700
Ok i get the problem but the solution is still a monsterous hack. Can we find a real solution to the problem? There has to be one.
---------------------------------------------------------------------------
by Gregwar at 2011/09/29 04:34:25 -0700
@beberlei, when an user is authenticated for instance, there can be proxies serialized in session.
Si if you clear the cache in dev environment you'll get an error because the matching proxy classes won't exist and you'll be forced to clear your cookies and reauth, which can be annoying
---------------------------------------------------------------------------
by Gregwar at 2011/09/29 04:38:45 -0700
@beberlei, yes, I agree that we should do something more elegant, but the problem is that when PHP "meet" the proxy class we can't really know what was the "original" class it is supposed to extend
And as @schmittjoh said, this will only be executed very rarely
---------------------------------------------------------------------------
by beberlei at 2011/09/29 05:18:35 -0700
You agree you want something more suitable and still want this to be merged?
To ease the immediate pain wr could allow this however only in debug mode. A real solution here is maybe to move the proxy files out of the env folders. Rhey dont depend ont the env after all.
---------------------------------------------------------------------------
by stloyd at 2011/09/29 05:21:33 -0700
Proxy is not depending on env, but generation of proxy is... So this solution will be hard IMO, or even unacceptable...
---------------------------------------------------------------------------
by Gregwar at 2011/09/29 05:25:39 -0700
@beberlei what I meant is that I agree that's dirty but I don't think of anything better to solve this...
---------------------------------------------------------------------------
by fabpot at 2011/09/29 06:23:03 -0700
Even if the current patch is not the best solution, we should probably apply it to fix the problem and think about a better solution afterwards. Does it sound good for everybody?
Commits
-------
3223c5a Removed now useless test
21cf0ac Backported new behaviour from PR #2148 and removed check for interface at run-time
8b240d4 Implementation of kernel.event_subscriber tag for services.
Discussion
----------
Added missing kernel.event_subscriber tag (closes#2012)
This PR adds a ``kernel.event_subscriber`` tag which allows to register services as event subscribers in the same way ``kernel.event_listener`` allows to register them as event listeners.
The service is still lazy loaded and the DIC does not need to be recompiled for every modification in the service's code.
There is one important thing to remember:
If the service is created by a factory, the class parameter **MUST** reflect the real class of the service, although it is not needed at the moment for the DIC. For that issue, we could either forbid services created by factories or add a note to the documentation.
This PR closes#2012.
---------------------------------------------------------------------------
by jalliot at 2011/08/24 06:42:18 -0700
I'm not sure the test is good enough so feel free to add some more.
---------------------------------------------------------------------------
by jalliot at 2011/08/25 03:46:20 -0700
I re-implemented the check for EventSubscriberInterface in ContainerAwareEventDispatcher because I think the overhead is minimum and it allows to use this method even without the tag (at run-time).
I also added some tests for RegisterKernelListenersPass.
---------------------------------------------------------------------------
by stof at 2011/09/04 02:42:00 -0700
@jalliot Your branch conflicts with the current master. could you rebase it ?
---------------------------------------------------------------------------
by jalliot at 2011/09/04 02:57:03 -0700
Rebased
---------------------------------------------------------------------------
by jalliot at 2011/09/13 02:19:46 -0700
@fabpot What do you think about this PR? At the moment, the subscribers are not really usable in Symfony2 because of the lack of this tag.
---------------------------------------------------------------------------
by fabpot at 2011/09/13 04:17:46 -0700
I don't like subscribers. There are other PRs on adding more support for them, but the reality is that they are complex for no added benefit. I'm wondering if it wouldn't be better to just remove them altogether.
---------------------------------------------------------------------------
by jalliot at 2011/09/13 04:38:20 -0700
@fabpot Well I prefer listeners too but I think that if Symfony2 does support subscribers (which it does at the moment), it should do it properly and completely, thus allowing to register subscriber services like here or to register several methods for one same event like in #2148.
But I guess that if you merged those 2 PRs (well actually this one would have to be modified first if #2148 is merged but I'll do it then), many use cases would be covered and people should stop asking for more support :) (except maybe for removing the static modifier but this would be wrong IMO and prevent entirely this PR).
---------------------------------------------------------------------------
by fabpot at 2011/09/28 11:47:10 -0700
@jalliot: #2148 has been merged. Can you update this PR accordingly? thanks.
---------------------------------------------------------------------------
by jalliot at 2011/09/28 12:00:44 -0700
Sure thing. Will do it as well as removing the check for the interface tonight or tomorrow :)
---------------------------------------------------------------------------
by jalliot at 2011/09/29 08:53:17 -0700
@fabpot Check for interface removed and #2148 merged. Also rebased on latest master.
---------------------------------------------------------------------------
by fabpot at 2011/09/29 09:09:11 -0700
Tests do not pass.
---------------------------------------------------------------------------
by jalliot at 2011/09/29 09:18:48 -0700
@fabpot Fixed
Commits
-------
731b28b [composer] add missing deps for FrameworkBundle
9c8f100 [composer] change ext/intl to the new ext-intl syntax
d535afe [composer] fix monolog-bridge composer.json, add more inter-component deps
9ade639 [composer] add composer.json
Discussion
----------
Composer
This PR adds a composer.json file for [composer](https://github.com/composer/composer) ([more info](packagist.org/about-composer)).
For discussion you can also go into #composer-dev on freenode and argue with naderman, seldaek and everzet.
---------------------------------------------------------------------------
by naderman at 2011/09/26 15:51:51 -0700
You haven't entered any keywords, they might come in handy when searching for packages on packagist.
But really this is just a +1 ;-)
---------------------------------------------------------------------------
by stof at 2011/09/26 16:12:21 -0700
See my comments on your previous (non-rebased) commit: f1c0242b5a
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:04:36 -0700
Following dependencies do not have a composer.json yet: Twig, Doctrine (orm, dbal, common), swiftmailer.
Also missing from the standard edition: assetic, twig-extensions, jsm-metadata, SensioFrameworkExtraBundle, JMSSecurityExtraBundle, SensioDistributionBundle, SensioGeneratorBundle, AsseticBundle.
The point is, those can be added later on. Having the components composerized is already a leap forward. Also, doctrine depends on some symfony components, we've got to start somewhere.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 00:36:41 -0700
Also, just for information, the plan is to have `symfony/framework-bundle` be the "framework", with all dependencies to doctrine etc, though we should really only have strict requirements in there, and then in symfony-standard we ship a composer.json that requires the framework-bundle, doctrine-orm and things like that that are not essential to core. Otherwise people don't have a choice about what they use anymore.
Just a comment btw, the json is invalid, all / should be escaped. However json_decode is nice enough to parse those without complaining, browsers do too, even Crockford's json2.js does, so I'm not sure if we should privilege readability over strictness, since it seems nobody really cares about this escaping.
---------------------------------------------------------------------------
by igorw at 2011/09/27 00:41:39 -0700
So, I've implemented all of @stof's suggestions, except (for reasons stated above):
* doctrine to DoctrineBundle
* swiftmailer to SwiftmailerBundle
* twig to TwigBundle
* doctrine-common to Validator
* FrameworkBundle (what exactly does it depend on?)
---------------------------------------------------------------------------
by stof at 2011/09/27 00:52:31 -0700
@igorw at least HttpKernel, Routing, Templating, EventDispatcher, Doctrine Common (annotations cannot be disabled), Translator, Form (optional), Validator (optional), Console (optional). See the service definitions to see the others
@Seldaek FrameworkBundle does not depend on Doctrine, except for Common
---------------------------------------------------------------------------
by beberlei at 2011/09/27 03:15:34 -0700
What does the symfony/ or ext/ prefix control in composer?
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 03:33:52 -0700
symfony/ is just the (mandatory) vendor namespace. Also ext/ has been renamed to ext- now, so it's not in any vendor, and should avoid potential issues.
---------------------------------------------------------------------------
by beberlei at 2011/09/27 05:07:03 -0700
@Seldaek Mandatory? So every package name is "vendor/package"? I like that because previously i thought package names are not namespaced, and thus clashes could occur between different communities easily.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 05:16:20 -0700
@beberlei: Mandatory. As of yesterday http://packagist.org/ will tell you you have an invalid package name if there's no slash in it. See 1306d1ca82 (diff-3)
* 2.0:
[Validator] added support for grapheme_strlen when mbstring is not installed but intl is installed
removed separator of choice widget when the separator is null
Commits
-------
060d75d [Console] fixed CS and simplified code
a5ff635 [Console] added support for interactive shell without readline extension
Discussion
----------
[2.1] [Console] added support for interactive shell without readline extension
---------------------------------------------------------------------------
by lmcd at 2011/06/30 15:12:56 -0700
Very useful! Pre-installed PHP on OS X ships without readline.
---------------------------------------------------------------------------
by stof at 2011/06/30 15:24:47 -0700
@lmcd You can already use the console, just not the shell.
---------------------------------------------------------------------------
by lmcd at 2011/06/30 15:27:00 -0700
@stof Yep I know, but it would be nice to use the shell :)
---------------------------------------------------------------------------
by hason at 2011/07/01 01:54:34 -0700
@stof Interactive generators will not work on Windows and Mac OS X without these simple changes. I think this is bad news.
---------------------------------------------------------------------------
by Seldaek at 2011/07/01 01:58:59 -0700
Yup, it's definitely a good addition.
---------------------------------------------------------------------------
by Seldaek at 2011/07/01 01:59:26 -0700
@fabpot: I'm not sure why this is tagged 2.1, but IMO this should go in now.
---------------------------------------------------------------------------
by stof at 2011/07/01 01:59:44 -0700
@hason This is not true. The shell is only used when running the console with the ``-s`` option. A command can be interactive without using the shell (I'm on Windows)
---------------------------------------------------------------------------
by lmcd at 2011/07/01 02:08:17 -0700
@stof Yea true, the interactive generators are fine without readline, however I don't think this dependancy should prevent people using the shell on Win/Mac.
---------------------------------------------------------------------------
by hason at 2011/07/01 02:28:10 -0700
@stof This is good news :) I was hasty.
---------------------------------------------------------------------------
by lmcd at 2011/07/04 02:03:36 -0700
I'm feeling inclined to remove readline entirely for 2.1 and roll our own solution to support things like this in the shell: http://lmcd.me/v/sf-autocomplete-2.mov
---------------------------------------------------------------------------
by stof at 2011/09/04 04:55:37 -0700
what is the status of this PR ?
Commits
-------
5146a1f [EventDispatcher] Added possibility for subscribers to subscribe several times for same event
Discussion
----------
[EventDispatcher] Added possibility for subscribers to subscribe several times for same event
[EventDispatcher] Added possibility for subscribers to subscribe several times for same event
closes#2146
And it is of course fully BC :)
---------------------------------------------------------------------------
by jalliot at 2011/09/09 17:34:07 -0700
If merged, #2021 will have to reflect the change too
Commits
-------
dd20f01 Fixed assets:install to use a relative path instead of an absolute
Discussion
----------
[2.1] Fixed "assets:install" to create relative instead of absolute symlinks
This is a fairly simple fix so that the symlinks are relative to the resources rather than an absolute path that breaks from machine-to-machine or upon deployment.
We were trying to figure out why styles were messed up for other contributors, but then found that the paths were hard-coded for my machine :)
---------------------------------------------------------------------------
by ericclemmons at 2011/06/04 09:44:11 -0700
Any other thoughts/updates on this?
---------------------------------------------------------------------------
by fabpot at 2011/06/04 22:31:39 -0700
We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two.
---------------------------------------------------------------------------
by ericclemmons at 2011/06/05 09:55:00 -0700
Sorry, I didn't think that we would be an issue since the Bundle "Best Practices" states to not include other bundles as dependencies.
If absolute links are a must, then the next alternative is for collaborators to add "/web/bundles" to .gitignore and each person run "assets:install" upon installation/update.
I was personally hoping there were a way to have this versioned for easier deployment.
On Jun 5, 2011, at 12:31 AM, fabpot<reply@reply.github.com> wrote:
> We have such a feature in symfony1 and IIRC it does not work very well. One problem is when you use symlink for some bundles. Then, you should not use a relative symlink as there is a common path between the two.
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1173#issuecomment-1303600
---------------------------------------------------------------------------
by henrikbjorn at 2011/06/27 04:56:58 -0700
``` php
<?php
// ...
->addOption('relative', null, InputOption::VALUE_NONE, 'The --symlink option will generate relative paths')
// ...
```
and just default to absolute paths ?
---------------------------------------------------------------------------
by ericclemmons at 2011/06/27 08:37:50 -0700
I'm very supportive of that compromise. Up to @fabpot if I should add this back in, since relative paths were apparently problematic with symfony1.
---------------------------------------------------------------------------
by sbusch at 2011/07/15 08:46:01 -0700
+1
I'm developing on Mac and the files are mounted on a Linux box which serves the project. The paths are not the same on those two systems. If I accidentally install assets on my Mac the absolute paths won't work on the Linux webserver.
Other scenario: one teammate could add those symlinks by accident to the git repository, which breaks all other installations.
Relative symlinks could help a lot in these cases.
---------------------------------------------------------------------------
by ericclemmons at 2011/07/15 08:47:53 -0700
@sbusch Your issues are the same as mine, which prompted this ticket :)
Until this gets @fabpot's blessing, it's best to simply add `web/bundles` to your `.gitignore` file and tell your users to always run `assets:install --symlink` each time they pull down code & something breaks ;)
---------------------------------------------------------------------------
by sbusch at 2011/07/15 08:58:33 -0700
The handling (calculation) of relative symlinks IMO fits better to the `symlink()` method of `\Symfony\Component\HttpKernel\Util\Filesystem`. Possible method signature:
symfony/src/Symfony/Component/HttpKernel/Util/Filesystem.php:
```php
<?php
// ...
/**
* Creates a symbolic link or copy a directory.
*
* @param string $originDir The origin directory path
* @param string $targetDir The symbolic link name
* @param Boolean $copyOnWindows Whether to copy files if on Windows
* @param Boolean $makeRelative Whether to try to create a relative link
*/
public function symlink($originDir, $targetDir, $copyOnWindows = false, $makeRelative = false)
{
```
And what about changing the `--symlink` option to optionally have a value, instead of adding a new depending option? E.g. `--symlink[=absolute|relative]`, with "absolute" as default:
symfony/src/Symfony/Bundle/FrameworkBundle/Command/AssetsInstallCommand.php:
```php
<?php
// ...
class AssetsInstallCommand extends ContainerAwareCommand
{
/**
* @see Command
*/
protected function configure()
{
$this
->setDefinition(array(
new InputArgument('target', InputArgument::REQUIRED, 'The target directory (usually "web")'),
))
->addOption('symlink', null, InputOption::VALUE_OPTIONAL, 'Symlinks the assets instead of copying it. Allowed values: "absolute" (default) and "relative".', 'absolute')
->setHelp(<<<EOT
The <info>assets:install</info> command installs bundle assets into a given
directory (e.g. the web directory).
<info>./app/console assets:install web [--symlink]</info>
A "bundles" directory will be created inside the target directory, and the
"Resources/public" directory of each bundle will be copied into it.
To create a symlink to each bundle instead of copying its assets, use the
<info>--symlink</info> option. Use <info>--symlink=relative</info> for relative symlinks.
EOT
)
->setName('assets:install')
;
}
/**
* @see Command
*
* @throws \InvalidArgumentException When the target directory does not exist
*/
protected function execute(InputInterface $input, OutputInterface $output)
{
if (!is_dir($input->getArgument('target'))) {
throw new \InvalidArgumentException(sprintf('The target directory "%s" does not exist.', $input->getArgument('target')));
}
if ($input->hasOption('symlink'))
{
if (!function_exists('symlink')) {
throw new \InvalidArgumentException('The symlink() function is not available on your system. You need to install the assets without the --symlink option.');
}
if (!in_array($input->getOption('symlink'), array('absolute', 'relative'))) {
throw new \InvalidArgumentException(sprintf('Invalid value "%s" for option "symlink"', $input->getOption('symlink')));
}
}
$filesystem = $this->getContainer()->get('filesystem');
// Create the bundles directory otherwise symlink will fail.
$filesystem->mkdir($input->getArgument('target').'/bundles/', 0777);
foreach ($this->getContainer()->get('kernel')->getBundles() as $bundle) {
$originDir = $bundle->getPath().'/Resources/public';
if (is_dir($originDir)) {
$targetDir = $input->getArgument('target').'/bundles/'.preg_replace('/bundle$/', '', strtolower($bundle->getName()));
$output->writeln(sprintf('Installing assets for <comment>%s</comment> into <comment>%s</comment>', $bundle->getNamespace(), $targetDir));
$filesystem->remove($targetDir);
if ($input->hasOption('symlink')) {
$filesystem->symlink($originDir, $targetDir, false, $input->getOption('symlink') == 'relative');
} else {
$filesystem->mkdir($targetDir, 0777);
$filesystem->mirror($originDir, $targetDir);
}
}
}
}
```
---------------------------------------------------------------------------
by sbusch at 2011/07/15 09:04:46 -0700
@ericclemmons: yes, that's our current workaround. I started with manually converting absolute links to relative ones, but that quickly got very annoying ;-)
After that I tried to implement the generation of relative links by myself (where the proposals of my previous comment come from) until I found your PR.
---------------------------------------------------------------------------
by henrikbjorn at 2011/07/18 00:20:38 -0700
@sbush if it defaults to something how would you turn it off ?
---------------------------------------------------------------------------
by stof at 2011/07/18 00:26:16 -0700
@henrikbjorn a default value for the option is used when using ``--symlink`` without the value. If you don't use the option at all, it is disabled.
---------------------------------------------------------------------------
by stof at 2011/07/18 11:58:29 -0700
In fact no. The default value seems to be also used when the option is not set at all. @fabpot is this intended ?
---------------------------------------------------------------------------
by Seldaek at 2011/07/19 05:18:29 -0700
Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process.
---------------------------------------------------------------------------
by ericclemmons at 2011/07/19 06:15:34 -0700
Nobody is even entertaining --relative?
On Jul 19, 2011, at 7:18 AM, Seldaek<reply@reply.github.com> wrote:
> Symlinks on windows, although technically possible, don't quite work with PHP on most setups. Also git doesn't seem to support them either on windows (not sure why not). For those reasons, and although I'm sure this doesn't apply to every project, I would recommend you just have everyone run `assets:install [--symlink]` on their local machine, and make that command run on the server as part of your deployment process.
>
> --
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/1173#issuecomment-1606463
---------------------------------------------------------------------------
by Gregwar at 2011/08/10 08:56:27 -0700
I agree with the idea of proposing a --relative option, I'm currently working on a Samba mounted filesystem and I'm forced to create manually symlinks to get things working since the paths are not the same
Commits
-------
908a7a3 [HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies set with a default '/' path, unless it was explicitly specified
Discussion
----------
[HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies
[HttpFoundation] Fix bug in clearCookie/removeCookie not clearing cookies set with a default '/' path, unless it was explicitly specified
---------------------------------------------------------------------------
by Seldaek at 2011/08/02 10:31:44 -0700
The reason is that Cookie::__construct defaults to '/' btw, so if you don't specify it, and then call clearCookie without specifying again, the paths don't match.
---------------------------------------------------------------------------
by Koc at 2011/08/07 00:06:13 -0700
I think that correctrly use base path. Is it possible?
For example we have 2 apps
* site.com/app1/index.php
* site.com/app2/index.php
and app2 will remove cookies of app1
---------------------------------------------------------------------------
by Seldaek at 2011/08/07 02:58:10 -0700
IMO if people want that they should specify the path manually, by default cookies are always set for the entire host and I think it should stay like that.
---------------------------------------------------------------------------
by Koc at 2011/08/07 04:26:47 -0700
It is hard to specify path manually everywhere when set/remove cookies.
---------------------------------------------------------------------------
by Seldaek at 2011/09/27 07:01:43 -0700
@fabpot: ping? You said this was ok, but it was never merged.
Commits
-------
f7bf7b5 fixed condition
181332b added a Controller:getUser() shortcut to recover the current user
Discussion
----------
[2.1] added a Controller:getUser() shortcut to recover the current user
Commits
-------
c4a0f79 Updates according to suggestions.
6aec789 Added tests.
54454ba Added generic filtering to ParameterBag.
Discussion
----------
Added generic filtering to ParameterBag.
Adds filtering convenience using PHP's filter_var() e.g.
$request->get->filter($key, '', false, FITLER_SANITIZE_STRING);
See http://php.net/manual/en/filter.filters.php for capabilities.
---------------------------------------------------------------------------
by GromNaN at 2011/09/25 15:41:50 -0700
What is the use case ?
---------------------------------------------------------------------------
by drak at 2011/09/25 15:52:19 -0700
Input variable validation/sanitization. ParameterBag has a few built in like `getAlnum()` for example. This method offer's PHP's full filtering and sanitization suite.
---------------------------------------------------------------------------
by fabpot at 2011/09/27 00:56:41 -0700
Can you add some unit tests for this new feature?
---------------------------------------------------------------------------
by drak at 2011/09/27 00:58:56 -0700
Sure thing.
---------------------------------------------------------------------------
by drak at 2011/09/27 01:07:03 -0700
Before I make the commit, is the method name ok for you or would you prefer it is called `getFiltered()`?
---------------------------------------------------------------------------
by fabpot at 2011/09/27 01:13:46 -0700
`filter` sounds good to me.
---------------------------------------------------------------------------
by drak at 2011/09/27 02:37:01 -0700
I've added some tests.
---------------------------------------------------------------------------
by stloyd at 2011/09/27 02:42:42 -0700
@drak IMO you must check that user don't use unknown filter and/or flags for filter.
---------------------------------------------------------------------------
by drak at 2011/09/27 02:48:38 -0700
@stloyd - I'm not sure that's practical at all, this is a wrapper for a built-in PHP function and I don't understand why we would need validate arguments for a PHP function - it's the coder's job to use the API correctly - none of the inputs to this function are coming from a web request. It would also mean that the API would need to keep track of any upstream changes to constants in the PHP engine (which are just integers after all). It's really just not practical.
---------------------------------------------------------------------------
by stealth35 at 2011/09/27 05:16:50 -0700
@drak it's could be cool to use `filter_id` ✌️
if (is_string) {
$filter = filter_id($filter);
}
---------------------------------------------------------------------------
by drak at 2011/09/27 07:05:42 -0700
@stealth35 regarding this
if (is_string) {
$filter = filter_id($filter);
}
I believe strongly in the use of IDEs when coding and autocomplete nicely provides when you type `FILTER_`. Additionally, `filter_id()` only works on filters, but not for the flags, so I'm not entirely sure how useful it would be overall compared to using a good IDE (which you need when working with complex frameworks anyhow, imo :)
---------------------------------------------------------------------------
by drak at 2011/09/27 07:30:10 -0700
Ok check it now.
Commits
-------
d675c28 [FrameworkBundle] Use Router instead of RouterInterface
ae7ae8d [FrameworkBundle] Moved router_listener from web to router.xml since it depends on the router
35a9023 [FrameworkBundle] Added isEnabled to Router commands, fixes#1467536d979 [Console] Added Command::isEnabled method that defines whether to add the command or not
Discussion
----------
[2.1] [Console] Added Command::isEnabled method
This addresses #1467.
The idea is to allow commands to evaluate whether they can run or not, since they are automatically registered.
- It's useful for the two router:* commands since they're optional (router can be disabled), but part of the FrameworkBundle that is not really optional.
- It could be useful for third party code as well.
- It's BC.
- aa95bb0d395810b29a3e654673e130736d9d1080 should address the issue in #1467, while the other commits just make sure the command is not registered at all if the router isn't standard.
One issue remains though:
- A few other services like twig helpers get the `ròuter` injected, this means that if there is really **no** router service defined, there is still an error. I'm not sure how to fix those beyond adding `on-invalid="null"` but I'm not sure if that's desirable. I guess we could argue that the router is a big candidate for replacement/suppression, and as such it should be truly optional, but if we do it I don't know where it'll lead. I don't want to end up in a situation where half the dependencies are optional to support every possible combination. @fabpot wdyt?
---------------------------------------------------------------------------
by kriswallsmith at 2011/06/28 16:19:46 -0700
I'd rather see us not register a command instead of register and then disable it. Can we do the same thing you've done here in the bundle's registerCommands() method?
---------------------------------------------------------------------------
by Seldaek at 2011/06/28 16:51:36 -0700
Note that it's never really registered. During the registration it's checked and skipped if not enabled.
However, doing it as you suggest means overriding/copy-pasting all the code from the core Bundle class, which I don't like so much. It also means adding code specific to those two commands in a somewhat unrelated place, which I also don't like.
I'm not saying the current solution is perfect, but from the alternatives I considered, it's the best I have found.
---------------------------------------------------------------------------
by stof at 2011/09/04 04:58:04 -0700
@Seldaek your branch conflicts with master. could you rebase it ?
@fabpot what do you think about this PR ?
---------------------------------------------------------------------------
by Seldaek at 2011/09/04 08:39:05 -0700
Rebased
Builds upon aead4a9836180cabae4d47fe27c634dcd79ac8f2, which prematurely removed request scoping from the assets templating helper in all cases. The helper need only be request-scoped if one or more request-scoped packages (e.g. PathPackages) are injected into it. This change makes it possible to utilize the assets helper outside of a request (e.g. during a console script).
To ensure that the assets helper is not assigned a request scope, all asset base URL's must be defined for all packages (default and any named) and both protocols: HTTP and SSL. The included test config fixtures concisely accomplish this by specifying a single HTTPS URL as the base URL for our default and named package, since FrameworkExtension's Configuration conveniently registers this URL for both protocols.
No other helpers have request scope and the assets helper's parameters don't appear to depend on the request in any way, so this appears to be unnecessary. As-is, request scope here prevents use of the assets helper from a console command that may need to internally render a template.
Adds filtering convenience using PHP's filter_var() e.g.
`$request->get->filter($key, '', false, FITLER_SANITIZE_STRING);`
See http://php.net/manual/en/filter.filters.php for capabilities.
* 2.0:
bumped Symfony version to 2.0.3-DEV
updated VERSION for 2.0.2
update CONTRIBUTORS for 2.0.2
updated CHANGELOG for 2.0.2
updated vendors for 2.0.2
merged branch helmer/target_path (PR #2228)
