Commits
-------
03fee4f Fix permissions
431460f [Form] Remove choice or choice_list requirement as the following conditions already check enough and this condition prevents empty select forms (populated by ajax for example)
Discussion
----------
[Form] Choice fix
[Form] Remove choice or choice_list requirement as the following conditions already check enough and this condition prevents empty select forms (populated by ajax for example)
---------------------------------------------------------------------------
by stloyd at 2011/07/05 06:26:36 -0700
You should revert permission changes.
---------------------------------------------------------------------------
by fabpot at 2011/07/05 06:28:14 -0700
Why not replacing `if (!$options['choices'] && !$options['choice_list']) {` by `if (!isset($options['choices']) && !isset($options['choice_list'])) { `?
---------------------------------------------------------------------------
by beberlei at 2011/07/05 06:35:50 -0700
gnaa permission changes, i cant seem to configure my machine such that it does not do it, i have to do this on a per repository basis, very annoying.
@fabpot isset() is already guaranteed because these two options are in the defaults.
---------------------------------------------------------------------------
by beberlei at 2011/07/05 06:39:43 -0700
Fixed the permissions
---------------------------------------------------------------------------
by stof at 2011/07/05 06:48:37 -0700
@beberlei Can't you fix it in the global git config ?
---------------------------------------------------------------------------
by webda2l at 2011/07/05 09:48:58 -0700
I met the same problem this afternoon and vote for the isset solution. Better than nothing and work for me.
https://github.com/symfony/symfony/pull/1539
---------------------------------------------------------------------------
by stof at 2011/07/05 09:50:09 -0700
@webda2l why is a check that always return true better than nothing ? It adds overhead without adding any value in the code.
Commits
-------
3917ed7 Revert "* DateType, DateTimeType, TimeType: - a bit changed readability"
c85b815 Fixed few issues with Date and Time:
Discussion
----------
[Form] Fixed few issues with Date and Time
Fixed few issues with Date and Time:
* TimeType:
- seconds are no longer populated if "with_seconds" = false
- "widget = text" is now properly rendered (closes#1480)
* DateTimeToStringTransformer:
- fixed using not default "format" (probably fix#1183)
* DateType, DateTimeType, TimeType:
- fixed "input = datetime" and test covered
Commits
-------
164aea4 [Security] Add tests for the channel listener
d51cbc0 [Security] Remove useless attribute in basic authentication listener & test it
91e6dc9 [Security] Add tests for the anonymous authentication listener
3c2affb [Security] Update access listener constructor's prototype and add tests
81afd77 [Security] Add tests for the firewall map
aa6ae33 [Security] Remove useless attribute & var in firewall
Discussion
----------
Test security
---------------------------------------------------------------------------
by lsmith77 at 2011/06/29 13:41:07 -0700
@schmittjoh is probably the person to review this change ..
Commits
-------
418d6a0 [Routing] Fix syntax error when dumping routes with single quotes in the requirements or pattern
2b5e22d [Routing] Fix ApacheDumper when a space appears in a default value
6c7f484 [Routing] Fix dumper so it doesn't print trailing whitespace
761724a [Routing] Adjust urlescaping rules, fixes#752
Discussion
----------
[Router] Bunch o' Fixes
The first commit changes the escaping rule to fix issues I had previously, and #752 as well, here's from the full commit message:
Only + and % are now encoded in generated routes, since they are the only characters that, if not encoded, could cause problems/conflicts when decoded. Namely + turns into a space, and % followed by numbers could do funky things.
The matcher decodes everything which works since nothing will have %NN without being escaped, and + are escaped as well.
Second commit is just a test fix for the first
Third and fourth are simply dumper escaping issues, nothing to argue about.
Note that all changes have had test cases added, and I spent a few hours torturing/testing all this stuff with both Apache and PHP dumpers, in many browsers, and with URLs as wonky as `/%01%02%03%04%05%06%07%08%0E%0F%10%11%12%13%14%15%16%17%18%19%1A%1B%1C%1D%1E%1F%20!%22$%25&%27%28%29*+,-./0123456789:;%3C=%3E@ABCDEFGHIJKLMNOPQRSTUVWXYZ%5B%5D%5E_%60abcdefghijklmnopqrstuvwxyz%7B|%7D~/baz` which essentially represent the 1-255 char range minus ? and #.
The only issues I really encountered after all the patches were applied is that Apache refuses to match `%22` (= `"`) and `*` in a url. I guess it's just because they're not allowed chars in windows paths, but | and < > works fine though. Anyway this works with the PHP dumper, and it didn't work either without my patches so it's not like I broke it, I'm just saying for the record.
Commits
-------
1cc1027 added @Annotation to UniqueEntity
ee22c5d added a note to update file
efcb435 updated to doctrine changes
Discussion
----------
updated to doctrine changes
---------------------------------------------------------------------------
by excelwebzone at 2011/06/30 06:29:23 -0700
Should also be implemented to the Route class and to all SensioFrameworkExtraBundle annotation classes
* TimeType:
- seconds are no longer populated if "with_seconds" = false
- "widget = text" is now properly rendered (closes#1480)
* DateTimeToStringTransformer:
- fixed using not default "format" (probably fix#1183)
* DateType, DateTimeType, TimeType:
- fixed "input = datetime" and test covered
- a bit changed readability
Only + and % are now encoded in generated routes, since they are the only characters that, if not encoded, could cause problems/conflicts when decoded. Namely + turns into a space, and % followed by numbers could do funky things.
The matcher decodes everything which works since nothing will have %NN without being escaped, and + are escaped as well.
Commits
-------
f4c7333 Fix populating seconds when option "with_seconds" is set to false
Discussion
----------
[Form][DateTimeType] Fix invalid data when "with_seconds" = false
Fix populating seconds when option `with_seconds` is set to `false`.
Commits
-------
4e3406d Sync with master and clean up
ad5d2c1 Added to `TimeType` extension possibility to render form as `single_text` (similar to DateType option) (issue #1205) Adjusted `DateTimeType` to allow usage of this new feature
Discussion
----------
[Form][TimeType] Added possibility to render form as "single_text"
Added to `TimeType` extension possibility to render form as `single_text` (similar to `DateType` option) (issue #1205)
Adjusted `DateTimeType` to allow usage of this new feature
---------------------------------------------------------------------------
by ouardisoft at 2011/06/17 03:41:18 -0700
+1
---------------------------------------------------------------------------
by stloyd at 2011/06/21 01:05:51 -0700
@fabpot Any decision about this one ? I'm asking because I also have similar fix for #1323 but it requires this one ;-)
---------------------------------------------------------------------------
by fabpot at 2011/06/22 23:32:08 -0700
@stloyd: Can you rebase to master?
---------------------------------------------------------------------------
by stloyd at 2011/06/23 05:03:44 -0700
@fabpot Done.
Commits
-------
7db0b95 [HttpKernel] Removed unnecessary strtoupper
0891c57 [HttpKernel] Added test
1350645 [HttpKernel] Uppercased a few http methods
05c9906 [HttpKernel] Suppress response content for 304 responses out of the cache
Discussion
----------
HttpCache changes for 304 responses
Fixes#1413
Commits
-------
2d29a82 New test for Process, testing stdout and stderr at different stream sizes
Discussion
----------
Make run() fully non-blocking and fix potential other problems
Multiple changes:
1) make writing to process non-blocking too - otherwise there might be increased possibility for buffer deadlock
given big enough input data. Also now it's guaranteed that all stdin data will be written.
2) get rid of fgets() - fgets() isn't really good function to use in case of non-blocking sockets. Data loss possible.
---------------------------------------------------------------------------
by fabpot at 2011/06/22 07:11:55 -0700
Does it make https://github.com/symfony/symfony/pull/1365 obsolete?
---------------------------------------------------------------------------
by lenar at 2011/06/22 14:08:14 -0700
@fabpot: After reading, I really don't know. Let's hope. But ...
I now improved Process tests a bit to test stdout, stderr with different stream sizes and different
behaviours of child processes. Added it to non-blocking-process branch, commit 2d29a82412.
In my case, nothing fails, but maybe this helps other people. Or Windows people - I myself cannot test on Windows.
---------------------------------------------------------------------------
by fabpot at 2011/06/22 22:59:55 -0700
These tests pass on my Linux box but fail on my Mac.
---------------------------------------------------------------------------
by fabpot at 2011/06/22 23:05:14 -0700
Actually, on the Mac, the tests behave correctly but the exit code is `-1` instead of `0`.
---------------------------------------------------------------------------
by lenar at 2011/06/23 01:23:51 -0700
Could you check if the $this->status['running'] (after call to proc_get_status()) is true in the case you get -1.
On my linux I got it -1 couple of times. 99% of time it doesn't happen. I theorized it's because sometimes the child
process isn't finished enough and finally I got confirmation too that in case of -1 the process is still running (stats['running'] === true).
But it's really almost unreproducible on my Linux. So if you have this value every time it might be easier for you to find solution.
What comes into my mind:
1) maybe we should poll, let's say if process is still running we usleep(1000) and the try proc_get_status() again until not running. Maybe up to a 1 sec.
2) maybe, if the process is still running we can trust the return value subsequently given by proc_close()?
Or maybe there's some other problem on Mac.
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)