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
This PR was merged into the 2.1 branch.
Commits
-------
2155719 [Component] [Security] fixed PSR-2 coding violation in ClassUtilsTest class.
Discussion
----------
[2.1] [Component] [Security] fixed PSR-2 coding violation in ClassUtilsTest class
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Fixes the following tickets: -
Todo: -
License of the code: MIT
This PR was merged into the 2.0 branch.
Commits
-------
113271c [Bundle] [FrameworkBundle] fixed indentation in esi.xml services file.
Discussion
----------
[2.x] [Bundle] [FrameworkBundle] fixed indentation in esi.xml services file.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Fixes the following tickets: -
Todo: -
License of the code: MIT
This PR was merged into the 2.1 branch.
Commits
-------
eb93e66 [Console] Fix style escaping parsing
8ca1b80 [Console] Make style formatter matching less greedy to avoid having to escape when not needed
Discussion
----------
[Console] Fix output formatting issues
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT
This was implemented in order to satisfy Drupal's requirements for a
singular and a plural message whenever a message is passed to their
implementation of transChoice().
This PR was merged into the master branch.
Commits
-------
4d7c895 Fix bug where backspacing to an empty string and using the arrow keys would fail. Added test to prevent in future
cd1def3 Fix bad unit test with undefined offset (spotted by @stloyd)
7f149ae [Console] Split tests for `DialogHelper` that tests `ask()` method
e6574de [Console] Fix `stty` reset when using `DialogHelper#ask()` with autocomplete functionality
Discussion
----------
[Console] Fix `DialogHelper#ask()` with autocomplete functionality
Bug fix: yes
Feature addition: no
BC break: no
Symfony2 tests pass: yes
This PR fixes failing test after: 9d94fc7 as well as correctly resets `stty` to prevent _strange_ data showing-up in console:
---------------------------------------------------------------------------
by fabpot at 2013-01-07T17:27:40Z
ping @lmcd
---------------------------------------------------------------------------
by lmcd at 2013-01-08T03:09:51Z
I'm using -1 as a neutral position when incrementing/decrementing the offset using arrow keys. If it started as zero, then it'd be incremented to 1 on down arrow and we'd miss the first (zero-index) match.
An easier fix for this is to replace all `if ($numMatches > 0)` with `if ($numMatches > 0 && -1 !== $ofs)`
Also, this:
if ($i === 0) {
$ofs = -1;
$matches = $autocomplete;
$numMatches = count($matches);
}
// Pop the last character off the end of our string
$ret = substr($ret, 0, $i);
$numMatches = 0;
Should be this:
if ($i === 0) {
$ofs = -1;
$matches = $autocomplete;
$numMatches = count($matches);
}
else {
$numMatches = 0;
}
// Pop the last character off the end of our string
$ret = substr($ret, 0, $i);
Edit: put these parts into a new pull request to avoid confusion https://github.com/symfony/symfony/pull/6614
---------------------------------------------------------------------------
by lmcd at 2013-01-08T03:11:20Z
Good catch on the stty issue 👍
---------------------------------------------------------------------------
by stloyd at 2013-01-08T12:14:01Z
@fabpot @lmcd I have "merged" fixes from: #6614 and removed those `env`s when used with `stty`.
---------------------------------------------------------------------------
by lmcd at 2013-01-08T12:16:05Z
@stloyd Awesome :)
This PR was merged into the 2.1 branch.
Commits
-------
55aa012 [Form] Fixed EntityChoiceList when loading objects with negative integer IDs
Discussion
----------
[Form] Fixed EntityChoiceList when loading objects with negative integer IDs
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #6610
Todo: -
License of the code: MIT
Documentation PR: -
---------------------------------------------------------------------------
by stof at 2013-01-08T10:21:57Z
wouldn't you need to do the opposite replacement when loading the result ? And shouldn't it be done in the propel bridge too ?
---------------------------------------------------------------------------
by bschussek at 2013-01-08T10:25:29Z
@stof No, indices aren't used for loading the result, just values. Yes this should be done for the propel bridge, but that bridge is missing even the tests for the ID-as-identifier usage. Do you want to create a PR?
This PR was merged into the 2.0 branch.
Commits
-------
8da2b41 [TwigBundle] There is no CSS visibility of display, should be visible instead
Discussion
----------
[TwigBundle] There is no CSS visibility of display, should be visible instead
This PR was merged into the 2.1 branch.
Commits
-------
ae3d454 [Form] corrected source node for a Danish translation
Discussion
----------
[Form] corrected source node for a Danish translation
This PR was merged into the master branch.
Commits
-------
5be0042 better regexp, more test cases, added comments about each credit card
cc278af [Validator] Fix `CardSchemeValidator` double violation when value is non-numeric. Making scheme option accept strings in addition to arrays.
Discussion
----------
[Validator] Improve regexp for Credit Cards and some more tests
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets:
Todo: Ensure these regexps are proper (credit card validation is always a pain)
License of the code: MIT
Documentation PR:
Regarding Cases excluded from new Regular Expressions:
- Credit card lengths should be respected, these regexp cover lengths in http://en.wikipedia.org/wiki/Bank_card_number
- Visa length can only be 16 and 13 (older ones)
- Diners Cards starting by 5 come from a joint venture between Diners Club and MasterCard, and should be processed like a MasterCard (according to http://www.regular-expressions.info/creditcard.html).
- There seems to be JCB cards starting by 2131 and 1800, I could find them is some places, also found these numbers being tested in Credit Card generators, but some people don't cover them. I don't know their story either
Any comments will be much appreciated!
---------------------------------------------------------------------------
by fabpot at 2013-01-06T19:33:27Z
Thanks for working on this. It would be very valuable if you can add information about these regexes as comments (with links to relevant sources -- like what you've done in the PR description). Thanks.
---------------------------------------------------------------------------
by ricardclau at 2013-01-06T21:01:52Z
Always glad to be able to contribute a little bit
@fabpot you mean @link / @see PHPDoc inside CardSchemeValidator.php? Or further comments in this discussion before adding them?
---------------------------------------------------------------------------
by fabpot at 2013-01-06T21:16:48Z
The more information we can add in the class, the better it is.
---------------------------------------------------------------------------
by ricardclau at 2013-01-07T20:56:05Z
I've added comments and included code from #6603 as I've said there. If you need something else, please let me know, once this is merged, #6603 can also be closed
---------------------------------------------------------------------------
by fabpot at 2013-01-07T21:41:40Z
Can you keep the commit from #6603 to keep ownership?
---------------------------------------------------------------------------
by ricardclau at 2013-01-07T21:44:16Z
I actually have thought about that... let me try my git skills :)
---------------------------------------------------------------------------
by ricardclau at 2013-01-07T21:59:16Z
There you go!
This PR was merged into the 2.0 branch.
Commits
-------
1d362b8 [DependencyInjection] fixed a bug where the strict flag on references were lost (closes#6607)
Discussion
----------
[DependencyInjection] fixed a bug where the strict flag on references were lost (closes#6607)
This PR was merged into the master branch.
Commits
-------
bcc5552 [Form] Protected methods in FormConfigBuilder and FormBuilder from being called when it is turned into a FormConfigInterface instance
fee1bf5 [Form] Introduced base ExceptionInterface
Discussion
----------
[Form] Protected methods in FormConfigBuilder and FormBuilder
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
When a form is created, a `FormConfigInterface` instance is passed to the constructor of `Form` and then accessible via `getConfig()`. For performance reasons, `FormBuilder` also implements `FormConfigInterface` and is passed here directly instead of copying its values to a new `FormConfig` instance, although protected from further modifications.
In addition to the already existing guard clauses in the setters, this PR adds guard clauses to the getters of `FormBuilder` to prevent misuse of the "config" object giving unexpected results.
Additionally, this PR introduces first improvements of the form's `Exception\` namespace as described in #6549.
---------------------------------------------------------------------------
by mvrhov at 2013-01-05T18:19:04Z
@bschussek I don't have the time to review this one, but please do not over optimize. We do a lot of weird things. e.g. I have the following code in one of my models.
```php
public function validateEmail(ExecutionContextInterface $context)
{
if ((null !== $this->product) && (in_array($this->product->getType(), array(ProductTypeEnum::Software, ProductTypeEnum::Subscription)))) {
/** @var $form Form */
$form = $context->getRoot();
if (($form instanceof Form) && ('foo' == $form->getConfig()->getOption('app_name'))) {
$context->validate($this, 'email', 'Email');
}
}
}
```
---------------------------------------------------------------------------
by bschussek at 2013-01-05T18:46:17Z
@mvrhov Absolutely no problem. Most people will use methods from `FormConfigInterface` on the result of `getConfig()`, because that's the type hinted return value, and that's the way it should be. I don't even know how many people are aware that `getConfig()` returns the builder object.
This PR is for rare cases where people write forward-incompatible hacks by accessing `FormBuilder` methods like `get()`, `getParent()` or the like on `getConfig()` (I don't know why anyone would do this, but you never know...)
---------------------------------------------------------------------------
by vicb at 2013-01-07T11:08:34Z
> Backwards compatibility break: no
Really ?
---------------------------------------------------------------------------
by vicb at 2013-01-07T12:26:22Z
Changing `FormException` from a class to an interface is a BC break (`new FormException()`)
---------------------------------------------------------------------------
by bschussek at 2013-01-07T12:41:20Z
@vicb Fair enough ;) I adapted the CHANGELOG and UPGRADE-2.2.
---------------------------------------------------------------------------
by fabpot at 2013-01-07T15:50:03Z
This PR break the tests.
---------------------------------------------------------------------------
by bschussek at 2013-01-07T15:50:34Z
I just fixed them and they run on my computer. I'm not sure why Travis fails again.
---------------------------------------------------------------------------
by bschussek at 2013-01-07T15:56:41Z
Travis merged the wrong commit. It merged 42c8987f350650a42d8c968d32cde50fc5d548c0 - see the merge commit here 766f7fe958ea69a302bed6298e55a3001a51e2b8 - when it should have merged cd3f4f9dfe43a526c23f30e16354ff11489b9db1.
This PR was merged into the 2.0 branch.
Commits
-------
3195122 [HttpFoundation] Check if required shell functions for `FileBinaryMimeTypeGuesser` are not disabled
Discussion
----------
[HttpFoundation] Check if required shell functions are not disabled
Rebased to `2.0` & squashed version of #6526.
This PR was merged into the master branch.
Commits
-------
0cf9fcb [HttpFoundation] Deprecated count() and \Countable
Discussion
----------
[HttpFoundation] Deprecated count() and \Countable
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #6408
Todo: -
License of the code: MIT
Documentation PR: -
The docblock is done in #6423 so will be merged up.
---------------------------------------------------------------------------
by stloyd at 2013-01-07T11:19:20Z
@fabpot ping
---------------------------------------------------------------------------
by drak at 2013-01-07T11:22:54Z
This should be OK for merge as the solution was agreed with @fabpot before making the PR.
---------------------------------------------------------------------------
by fabpot at 2013-01-07T11:24:50Z
@drak Can you add an entry in the CHANGELOG stating that this is a BC break?
---------------------------------------------------------------------------
by drak at 2013-01-07T15:31:21Z
@fabpot - There doesnt seem to be a CHANGELOG-2.2 in the master branch... where should I place the information?
---------------------------------------------------------------------------
by Tobion at 2013-01-07T15:46:25Z
@drak just in the CHANGELOG under the 2.2. header
---------------------------------------------------------------------------
by fabpot at 2013-01-07T15:48:43Z
Here: https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/CHANGELOG.md
---------------------------------------------------------------------------
by drak at 2013-01-07T15:48:45Z
@Tobion - thanks, I forgot it's in the package root, not the repo root. :)
---------------------------------------------------------------------------
by drak at 2013-01-07T15:52:37Z
@fabpot - updated changelog, squashed and rebased.
This PR was merged into the 2.1 branch.
Commits
-------
8513777 [HttpFoundation] Update docblock for non-working method
Discussion
----------
[HttpFoundation] Update docblock for non-working method
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Rerences the following tickets: #6408
Todo: -
License of the code: MIT
Documentation PR: -
---------------------------------------------------------------------------
by piotrpasich at 2012-12-19T21:32:24Z
I think that we shouldn't comment it as not working but add new methods called countFlashes and countFlashesTypes. CountFlashesTypes should return count($this->flashBag) and countFlashes should count all items in every array inside flashbag.
---------------------------------------------------------------------------
by drak at 2012-12-20T17:43:38Z
ping @fabpot
This PR was merged into the 2.1 branch.
Commits
-------
33e9d00 [Form] Deleted references in FormBuilder::getFormConfig() to improve performance
Discussion
----------
[Form] Deleted references in FormBuilder::getFormConfig() to improve performance
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #6558
Todo: -
License of the code: MIT
Documentation PR: -
---------------------------------------------------------------------------
by vicb at 2013-01-07T11:09:24Z
> Backwards compatibility break: no
Really ?
---------------------------------------------------------------------------
by vicb at 2013-01-07T12:27:37Z
Adding a public method is a BC break.
---------------------------------------------------------------------------
by bschussek at 2013-01-07T12:42:14Z
The method is inherited from the parent class, so the change should not affect BC.
---------------------------------------------------------------------------
by vicb at 2013-01-07T13:27:21Z
my bad.
