Commits
-------
d49e306 [DomCrawler] Fixed handling of relative query strings as links
a4451b4 [DomCrawler] Added tests for links starting with ?foo
Discussion
----------
[DomCrawler] Fixed handling of relative query strings as links
The link object concatenates links containing only a query string to the current URI. This shouldn't happen if the current URI already contains a query string.
This PR fixes the incorrect behavior (tests included).
Commits
-------
7783a05 Removed unused code from DateType Additional tests for ChoiceType and DateType based code
cdd39ac Added ability to set "empty_value" for `DateTimeType`, `DateType` and `TimeType` Additional tests covering added code
af4a7d7 More tests and more compatible code, with some suggestions from @helmer
527b738 Test covered version of fix for issue #1336
Discussion
----------
[Form] Added ability to set "empty_value" for choice list
Hey,
This PR is similar to #1336, but this one is fully test covered and have few change in behavior:
- if choice field is not set as non-required, `empty_value` is not added automaticly,
- also `empty_value` is not set if field have option `multiple` or `expanded`,
- `empty_value` for `DateType` and `TimeType` can be set "global" or per field, i.e.:
```
$builder->add('date', 'choice', array('required' => false, 'empty_value' => array('day' => 'Choose day')));
```
- `DateType` and `TimeType` code was cleaned a bit,
- added missing option to set up choice list as required when using PHP templates
---------------------------------------------------------------------------
by stloyd at 2011/06/20 04:55:45 -0700
@fabpot I'm just not sure is that change with removing "auto-adding" of `empty_value` is good (probably BC)
---------------------------------------------------------------------------
by lenar at 2011/06/20 05:24:02 -0700
Now this is a really nice way to hijack work done by others. Really encourages newcomers. Gratz!
---------------------------------------------------------------------------
by fabpot at 2011/06/20 05:57:40 -0700
@lenar: if the code in this PR is yours (at least partly), I'm not going to merge it. @stloyd, can you clarify this issue with @lenar? Thanks.
---------------------------------------------------------------------------
by lenar at 2011/06/20 06:21:11 -0700
It's @helmer's mostly, not mine, but the issue stays.
---------------------------------------------------------------------------
by fabpot at 2011/06/20 06:26:15 -0700
No matter who the code belongs to, Git allows us to keep track of all contributors. So, we need to do our best to not loose any code ownership.
---------------------------------------------------------------------------
by helmer at 2011/06/20 06:58:03 -0700
I do not care much for ownership, just that this kind of cooperation (or lack thereof) is kind of exhausting. Closed#1336.
---------------------------------------------------------------------------
by stloyd at 2011/06/20 07:47:53 -0700
@fabpot, @lenar: This PR is inspired by #1336, made by @helmer, but after looking at his code and talking with him, we cant (IMO) get an consensus. So I wrote this PR as an another way to fix issue described in #1336.
__Summary__: I don't think this one is better than fix at #1336, it's more like another approach to fix that issue.
---------------------------------------------------------------------------
by helmer at 2011/06/20 08:15:59 -0700
@stloyd: I actually think your variant is better, so good job there, thanks.
It just ain't nice to:
1) Comment on my changes being useless due to lack of tests
2) Writing brand new testsuite from your perspective that "proves" my approach is "wrong" (while ignoring my answers, why I did something precisely like I did, which I did in sync with @fabpot comments on his first attempt to improve the issue)
3) Saying my PR is broken because your new tests against it fail
4) Changing functionality to "fix" something that was not really broken
Other than that, I wanted to contribute a few lines to improve something relatively simple, and it ended up in a huge mess with more lost hours than I planned to spend on it.
On the bright side, we ended up with something good (:
---------------------------------------------------------------------------
by stloyd at 2011/06/20 08:32:30 -0700
@helmer: 1) & 2) Sorry for that "bad language", but you get me wrong a bit. Tests was written for code in master (there was no problem to change them to work with your POV). 3) Same as before, you can adopt tests easily, but never mind. Maybe later we could cooperate better ;-)
About 4) I mentioned it in description of this PR and that was point I was disagreeing with you (also about how "default" options are adopted in fields) :-)
Commits
-------
5d46e63 [Form] Add the FormHelper configuration
a43fad4 [Form] Improve unit tests for rendering
1cb2129 [FrameworkBundle][Form] Adding a cache to FormHelper::lookupTemplate()
f39ce67 [Form][FrameworkBundle] PHP theming
Discussion
----------
[2.1] RFC [Form] Php theming
This PR implements theming support for the php engine.
It works similarly as the twig theming with themes being folders and blocks being individual files.
There are probably a few things to tune before this can get merged:
### Theme naming
The current format is "\<Bundle\>:\<Controller\>" i.e. "FrameworkBundle:Form".
Is this ok or could you imagine something better ?
### Div and Table theme folders
Currently "FrameworkBundle\\Resources\\views\\Form" and "FrameworkBundle\\Resources\\views\\FormTable"
Is this ok or anything better ?
### Form helper configuration
I am not sure if the configuration is at the best possible location:
```
framework:
templating:
form:
resources: [themeA, themeB]
```
Any better idea ?
There is a [thread on the ml](http://groups.google.com/group/symfony-devs/browse_thread/thread/9b3f131fe116b511)
Commits
-------
7af003b [HttpFoundation] Allow stringable objects and numbers in response body + added tests
8126fb7 [HttpFoundation] Ensure response body is string, fixes#1378
Discussion
----------
[HttpFoundation] Ensure response body is string, fixes#1378
The issue in #1378 is clearly just a misuse of the class due but the error it triggers is confusing. This hopes to fix that.
---------------------------------------------------------------------------
by Seldaek at 2011/06/21 04:08:40 -0700
Added tests and support for __toString + numbers (kinda pointless but well..)
This change removes the need for the {_locale} hack.
Now, all paths in the Security component can be:
* An absolute path (/login)
* An absolute URL (http://symfony.com/login)
* A route name (login)
So, if you want to use a path that includes a global parameter (like _locale),
use a route instead of a path.
Commits
-------
2c1108c [Form] Revert the ability to override anything else than the text of the label while rendering a row
da467a6 [Form] Fix the exception message when no block is found while rendering
8670995 [Form] Optimize rendering when the block to render is known
41e07c9 [Form] Optimize rendering
ee5d975 [Form] Remove a test which is no more relevant (after recent FileType refactoring)
f729c6b [Form] Add the ability to override label & widget options when rendering a row
e09ae3f [Form][FrameworkBundle] Make FormHelper::renderSection() recursively callable, introduce FormHelper::renderBlock()
e43fb98 [Form][TwigBridge] Make FormExtension::render() recursively callable to ease theming
Discussion
----------
[Form] Some refactoring of the rendering
# First two commits
## FormExtension::render() can now be called recursively.
The main use case is theming support in for collections. Let's consider that you have a collection of `CustomType`, the type hierarchy while rendering the proto would be `field < form < custom < prototype`. Before this change any theme applied to your custom type (i.e. a `custom_row` block) would not have been taken into account while rendering the prototype because of the structure of the `prototype_row` block:
```html
{% block prototype_row %}
{% spaceless %}
<script type="text/html" id="{{ proto_id }}">{{ block('field_row') }}</script>
{% endspaceless %}
{% endblock prototype_row %}
```
which skip the `custom_row` block rendering to fallback to the `field_row` block rendering.
With this PR `prototype_row` recursively calls `FormExtension::render()`
```html
{% block prototype_row %}
{% spaceless %}
<script type="text/html" id="{{ proto_id }}">{{ form_row(form) }}</script>
{% endspaceless %}
{% endblock prototype_row %}
```
this has for effect to render the block for the parent type (i.e. `custom_row`)
## FormHelper
The `FormHelper` has been updated to more closely match the `FormExtension` architecture and the templates have been modified accordingly. `echo $view['form']->renderBlock(<block name>)` is the php equivalent of `{{ block(<block name>) }}`.
The attributes are now rendered using a template rather than by the `FormHelper::attributes()` method.
Several templates have been fixed.
# Third commit
The `$varStack` property was used to forward options to the label and the widget when rendering a row. The implementation was not working as expected. The proposed way to override label and widget options is to pass these options in the `label` and `widget` keys while callinf `render_row`.
That would be:
`{{ form_row(form.field, {"attr": {<row attributes>}, "label" : {"label": <text>, "attr": {<label attr>}}, "widget" : { "attr" : {<widget attributes}} } }}`
So there is now the ability to set attributes for the row (`<div>` or `<tr>`).
This has been discussed on [the mailing list](http://groups.google.com/group/symfony-devs/browse_thread/thread/17754128ba480545). **I would like to find a compromise with @Seldaek before this gets merged**
The `$varStack` property is now only used when recursively calling `FormExtension::render()`
# Notes
I have preferred to submit several commits in order to ease review and to keep some history.
---------------------------------------------------------------------------
by stof at 2011/06/20 05:20:56 -0700
@vicb On a side note, do you think it would be possible to support form theming in PHP templates too ? Currently, the only way to customize the rendering of forms when using PHP templates is to overwrite the FrameworkBundle's templates, and this impacts all forms. This makes the PHP rendering far less powerful than the Twig one.
I don't know the Form rendering and the PHPEngine well enough to know if it is feasible for 2.1 or not.
---------------------------------------------------------------------------
by vicb at 2011/06/20 05:35:11 -0700
@stof I hope to make it possible but I need a little bit more thinking to find the best possible solution which should not look like a hack.
---------------------------------------------------------------------------
by vicb at 2011/06/21 01:13:10 -0700
This should not be merged yet, it might have some issue with the variable stack. I am working on it.
---------------------------------------------------------------------------
by vicb at 2011/06/21 01:41:11 -0700
Sorted out the issue, it was linked to some local _optimization_, the code of this PR is ok.
---------------------------------------------------------------------------
by vicb at 2011/06/21 02:01:24 -0700
I have pushed a [POC of php theming based on this PR](https://github.com/vicb/symfony/commits/form%2Fphp-theme) to my repo - it is lacking a configuration and cache layer.
I have open [a thread on the ml](http://groups.google.com/group/symfony-devs/browse_thread/thread/9b3f131fe116b511) to discuss this.
---------------------------------------------------------------------------
by vicb at 2011/06/21 23:40:21 -0700
@fabpot fixed in the last commit.
Commits
-------
159fc0e [Validator] Added symbols to IDNs validation
c827faf [Validator] Add support for IDNs and custom TLDs
Discussion
----------
[Validator] Add support for IDNs and custom TLDs
Minor changes to allow for IDNs and [custom TLDs](http://news.softpedia.com/news/ICANN-Approves-New-Custom-Generic-Top-Level-Domains-Like-google-bank-206977.shtml). This is the only sane way to support everything in a timeless manner.
---------------------------------------------------------------------------
by stealth35 at 2011/06/20 04:32:09 -0700
maybe it should be check the host with idn_to_ascii (if function exists, maybe it's should recreate un punycode en/decoder in the stub)
---------------------------------------------------------------------------
by mvrhov at 2011/06/20 04:40:10 -0700
/me :faceslap.
Haven't seen the link in PR
---------------------------------------------------------------------------
by Seldaek at 2011/06/20 04:40:40 -0700
@mvrhov: Yup, that's what pushed me to reconsider adding this.
@stealth35: I'm not sure if this is needed. I don't want this to be too strict, with another validator or with an extra option I think we can make a check that the domain actually exists, or do a GET / on it or something, but this just checks validity of the syntax.
---------------------------------------------------------------------------
by stealth35 at 2011/06/20 04:48:05 -0700
I understand :)
what about funny IDN like : [☎.com] (http://xn--y3h.com/) ?
---------------------------------------------------------------------------
by Seldaek at 2011/06/20 04:53:19 -0700
@stealth35: Fixed
---------------------------------------------------------------------------
by stealth35 at 2011/06/20 04:56:18 -0700
it's seem great,for acceptable chars [RFC] (http://www.faqs.org/rfcs/rfc3490.html) said (with UseSTD3ASCIIRules option) :
(a) Verify the absence of non-LDH ASCII code points; that is, the
absence of 0..2C, 2E..2F, 3A..40, 5B..60, and 7B..7F.
Commits
-------
72d0ebe9 [WebProfilerBundle] Added the support of the the logging context in the template
410b3e0 [HttpKernel] Added the context in the LoggerInterface
Discussion
----------
context in the LoggerInterface
This adds the context in the LoggerInterface. The change is totally BC for people using the logger. However this affects people implementing the interface.
Note that this require Seldaek/monolog#33 for the implementation
---------------------------------------------------------------------------
by Seldaek at 2011/06/17 04:24:18 -0700
@fabpot: just ping me when you are merging this one, so I can merge in monolog and we avoid out-of-sync issues.
---------------------------------------------------------------------------
by stof at 2011/06/17 04:49:05 -0700
@Seldaek you can merge in Monolog when you want. Monolog is BC so merging it before the PR in Symfony2 does not break things.
---------------------------------------------------------------------------
by Seldaek at 2011/06/17 05:08:34 -0700
Ah right, I thought the interfaces wouldn't match, but PHP allows extra args it seems so I'll merge right now.
---------------------------------------------------------------------------
by stof at 2011/06/17 05:32:58 -0700
PHP allows extra *optionnal* args and it is the case here :)
---------------------------------------------------------------------------
by Seldaek at 2011/06/17 05:35:00 -0700
Well yes otherwise you break the interface. Anyway it's merged so @fabpot, anytime :)
Commits
-------
edf4b87 Add missing "tearDown" functions, and some missing variable declaration (this saves for me almost 20MB when run all tests) Force AsseticBundle tests to use TestCase Fix test for DoctrineBundle to use TestCase
2b0c352 Increase code coverage for: YamlParser, Validators, PhpEngine + Helpers, HttpFoundation
b88a0a0 Remove tabs
99f9337 Additional tests for PhpEngine + Helpers More tests for UrlValidator
450ed85 Additional tests for DateTimeValidator, EmailValidator and UrlValidator
Discussion
----------
[Tests] Cleanup + make code coverage more happy
Hey,
this PR is a bit bigger than usually ;-) few infos what's inside:
- Fix `DoctrineBundle` test to use `TestCase`
- Mark tests as "incomplete" instead of commenting them out
- Increase code coverage for: `Validators`, `PhpEngine` + `Helpers`, `HttpFoundation` (`Session`, `Response` etc.)
- And my favourite ;-) added missing variables definition (also removed non-used) and `tearDown()` function (if needed) to tests which allowed me saved __~15MB__ when running all tests
---------------------------------------------------------------------------
by stloyd at 2011/06/16 05:58:21 -0700
@fabpot & @marcw It was rebased and cleanup up (I split up `AsseticBundle` symfony/AsseticBundle#1 change to new repo), and added few new tests.
Commits
-------
6436da2 added quote
7dc4e24 [Tests][Locale] locale supported INTL_ICU_VERSION, use ReflectionExtension to find ICU version
Discussion
----------
[Tests][Locale] locale supported INTL_ICU_VERSION, use ReflectionExtension to find ICU version
---------------------------------------------------------------------------
by igorw at 2011/06/16 03:44:25 -0700
Nice.
`INTL_ICU_VERSION` is PHP 5.4 only, right?
Also, it may be possible to use:
$output = \ReflectionExtension::export('intl', true);
That way we can get rid of the output buffering.
---------------------------------------------------------------------------
by stloyd at 2011/06/16 03:49:22 -0700
@igorw It will be added in PHP 5.3.7 to.
---------------------------------------------------------------------------
by stealth35 at 2011/06/16 04:04:09 -0700
@igorw no export don't return the info table
Commits
-------
72c074a [Session] Used \Locale::setDefault() when the locale is setted
Discussion
----------
[Session] Used \Locale::setDefault() when the locale is setted
For `DateType` in form component (by example), `\Locale::getDefault()` is used to displayed the name of months.
If `\Locale` class is not used when the locale is setted in the session, the name of months is not in a good language.
This PR solves this problem.
---------------------------------------------------------------------------
by pborreli at 2011/05/29 09:13:44 -0700
what if user doesn't have intl extension ?
---------------------------------------------------------------------------
by stof at 2011/05/29 09:24:04 -0700
You should wrap the calls to ``\Locale::setDefault`` in a ``class_exist`` check to avoid issue when using the stub implementation (for which calling ``setDefault`` is forbidden).
---------------------------------------------------------------------------
by francisbesset at 2011/05/29 09:26:40 -0700
@pborreli: Symfony have a fake Locale class and this class is used only if the server haven't intl enabled.
---------------------------------------------------------------------------
by stof at 2011/05/29 09:33:16 -0700
@francisbesset Yeah, but ``setDefault`` throw a ``BadMethodCall`` exception.
and so the check has to use ``extension_loaded`` instead of ``class_exists``.
---------------------------------------------------------------------------
by fabpot at 2011/06/13 10:12:15 -0700
Ticket #1121 is related to this PR.
---------------------------------------------------------------------------
by fabpot at 2011/06/15 06:18:28 -0700
I have just tried another implementation where the locale is passed as an argument to the built-in types and some data transformers (via a `LocaleAwareInterface` interface). That works fine as forms are immutable now, but the solution is obviously more "complex" as we need to pass the locale to many different classes. Also, using `Locale::setDefault()` has an advantage over my method: you can change the locale whenever you want within a PHP process (which can be useful even if this is an edge case). Last, but not the least, if make sense to update the PHP Locale to the user locale.
So, to sum up, this patch is probably the best solution (easy and flexible enough).
