Commit Graph

953 Commits

Author SHA1 Message Date
Fabien Potencier
5314836d3c merged branch Herzult/feature/collection_size_validator (PR #4149)
Commits
-------

3a5e84f [Validator] Add CollectionSize constraint

Discussion
----------

[Validator] Add CollectionSize constraint

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

I will also send a PR to the documentation as soon as this one is accepted.

---------------------------------------------------------------------------

by bschussek at 2012-04-29T08:24:28Z

-1

I dislike the rising amount of very specific constraints in the core. Can't we add this to Size?

---------------------------------------------------------------------------

by vicb at 2012-04-29T09:01:39Z

@bschussek #3918 implements what you propose but then the messages are not valid any more:

```php
<?php
    public $minMessage = 'This value should be {{ limit }} or more';
    public $maxMessage = 'This value should be {{ limit }} or less';
    public $invalidMessage = 'This value should be a valid number';
```

I can imagine 2 solutions:

- adding some more message,
- rename the `Size` constraint to `Range` and create a new `Size` constraint for arrays / countables.

What do you think ?

---------------------------------------------------------------------------

by bschussek at 2012-04-29T09:27:53Z

I'd prefer the second solution and merge `Size` with `SizeLength` as well.

---------------------------------------------------------------------------

by vicb at 2012-04-29T09:34:50Z

@bschussek It would make sense. @makasim @Herzult any one of you would like to contribute this (i.e. rename the current Size to Range and create a new Size supporting arrays / countables / strings) ?

---------------------------------------------------------------------------

by Herzult at 2012-04-29T14:31:12Z

Yep, I'm on it.

---------------------------------------------------------------------------

by stof at 2012-04-29T15:22:44Z

@Herzult could you take the other comment into account and merge SizeLength into you Size ?

---------------------------------------------------------------------------

by vicb at 2012-04-29T15:33:05Z

The guessers should also be modified (it might also affect the ODM which is in an other repo, if so it would be good to sync the changes).

---------------------------------------------------------------------------

by Herzult at 2012-04-29T16:38:19Z

@stof the problem merging SizeLength into Size is that they don't have the same required options & messages.

---------------------------------------------------------------------------

by Herzult at 2012-04-29T16:47:40Z

And what about renaming Range to Interval and SizeLength to IntervalLength?

---------------------------------------------------------------------------

by stof at 2012-04-29T16:54:38Z

Well, SizeLength is about matching the length of a string currently. Nothing related to intervals

---------------------------------------------------------------------------

by Herzult at 2012-04-29T17:29:40Z

Here are the current names:

 * **Size** for collection (countable) size
 * **Range** for numbers
 * **SizeLength** for strings

Merging **SizeLength** into **Size** is maybe not appropriate because collections and strings are different things. It'll be hard to find messages that fit both collections and strings. Maybe we had better to find a better name for both. What do you think?

About the ValidatorTypeGuesser, I'll update it as soon as we know ow to name the constraints.

---------------------------------------------------------------------------

by vicb at 2012-04-29T17:43:01Z

Size is a good name for both strings and "collections", could we have two sets of strings and select according to the type ?

---------------------------------------------------------------------------

by Herzult at 2012-04-29T22:39:55Z

I tried to merge them together, what do you think?

---------------------------------------------------------------------------

by vicb at 2012-04-30T06:52:37Z

I think your changes are great, may be @bschussek has more feedback. The ValidatorTypeGuesser and the translation are yet to be updated.

---------------------------------------------------------------------------

by hhamon at 2012-05-01T12:32:28Z

Am I missing something or `SizeLength` for strings is a duplicate for `MinLength` and `MaxLength` constraints?

---------------------------------------------------------------------------

by Herzult at 2012-05-02T13:29:36Z

Yep, that's true. But the only link between this PR and the SizeLength constraint is that I merged it to the one I introduced.

---------------------------------------------------------------------------

by Herzult at 2012-05-07T07:48:01Z

@bschussek what do you think?

---------------------------------------------------------------------------

by vicb at 2012-05-10T19:51:26Z

@Herzult this PR looks good to me, could you update the changelog and update guides, try to factorize the code and squash the commits ? Thanks.

---------------------------------------------------------------------------

by travisbot at 2012-05-11T15:42:35Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1306112) (merged 8d8e6443 into 4ac3bddb).

---------------------------------------------------------------------------

by vicb at 2012-05-11T21:42:21Z

* could #4259 be helpful ?
* please squash the commits.
* please create a PR / issue on [symfony-docs](https://github.com/symfony/symfony-docs)

thanks for the updates.

---------------------------------------------------------------------------

by travisbot at 2012-05-13T18:38:18Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1321123) (merged eeda9044 into 4ac3bddb).

---------------------------------------------------------------------------

by travisbot at 2012-05-13T18:45:12Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1321146) (merged 491ca19a into 8b54eb56).

---------------------------------------------------------------------------

by travisbot at 2012-05-14T11:29:39Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1326110) (merged 44865024 into 8b54eb56).

---------------------------------------------------------------------------

by vicb at 2012-05-14T11:49:37Z

@Herzult what about plural translations ?

---------------------------------------------------------------------------

by travisbot at 2012-05-14T16:52:37Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1328677) (merged 93480f95 into 46ffbd52).

---------------------------------------------------------------------------

by travisbot at 2012-05-14T17:03:13Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1328705) (merged 326c3b81 into 46ffbd52).

---------------------------------------------------------------------------

by vicb at 2012-05-14T20:19:18Z

thanks for the updates, this PR looks fine to me. @bschussek ?

---------------------------------------------------------------------------

by vicb at 2012-05-16T06:45:51Z

@Herzult can you squash your commits ?

---------------------------------------------------------------------------

by travisbot at 2012-05-16T11:20:44Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1344811) (merged 3a5e84f4 into 58b6ef23).
2012-05-16 14:22:22 +02:00
Antoine Hérault
3a5e84f4a7 [Validator] Add CollectionSize constraint
[Validator] Rename constraint Size to Range

[Validator] Rename constraint CollectionSize to Size

[Validator] Merge the SizeLength into the Size constraint

[Validator] Update messages in Size constraint for consistancy

[Validator] Add english and french translation for Size messages

[Validator] Tweak expected types for exceptions in SizeValidator

[Validator] Fix CS in SizeValidator

[Validator] Update the ValidatorTypeGuesser

[Validator] Tweak SizeValidator

[Validator] Update CHANGELOG

[Validator] Complete previous CHANGELOG updates

[Form] Update validator type guesser

[Validator] Pluralize collection size english messages

[Validator] Pluralize Size french messages
2012-05-16 13:15:05 +02:00
Fabien Potencier
41621e42e9 fixed phpdoc @param alignment 2012-05-15 22:19:31 +02:00
Fabien Potencier
f27f5969b6 fixed 2.0 merge 2012-05-15 22:15:08 +02:00
Fabien Potencier
03d4b0264f merged 2.0 2012-05-15 18:49:53 +02:00
Fabien Potencier
bd07b8919d merged branch bschussek/options (PR #3968)
Commits
-------

95727ff [OptionsResolver] Updated PHP requirements to 5.3.3
1c5f6c7 [OptionsResolver] Fixed issues mentioned in the PR comments
d60626e [OptionsResolver] Fixed clear() and remove() method in Options class
2b46975 [OptionsResolver] Fixed Options::replace() method
16f7d20 [OptionsResolver] Improved implementation and clarity of the Options class
6ce68b1 [OptionsResolver] Removed reference to non-existing property
9c76750 [OptionsResolver] Fixed doc and block nesting
876fd9b [OptionsResolver] Implemented fluid interface
95454f5 [OptionsResolver] Fixed typos
256b708 [OptionsParser] Renamed OptionsParser to OptionsResolver
04522ca [OptionsParser] Added method replaceDefaults()
b9d053e [Form] Moved Options classes to new OptionsParser component

Discussion
----------

Extracted OptionsResolver component out of Form

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=options)

This PR refactors the options-related code of the Form component into a separate component. See the README file for usage examples.

---------------------------------------------------------------------------

by schmittjoh at 2012-04-17T18:11:03Z

To me it seems like we have some redundancy with the Config/Definition component. I'm wondering if these two can/should be merged somehow?

---------------------------------------------------------------------------

by kriswallsmith at 2012-04-17T18:14:44Z

I would also suggest merging this into the Config component. Its current name too closely resembles Python's optparser lib, which could create confusion.

---------------------------------------------------------------------------

by bschussek at 2012-04-17T18:18:49Z

Merge conflict artifacts are fixed now.

@schmittjoh Do we? Isn't the idea of the Config component to read complex configuration from different configuration providers? (YAML, XML, Annotations etc.)

The idea of this parser is to be highly performant and to be usable in simple classes. If this can be achieved with the Config component, I'm happy to learn more.

---------------------------------------------------------------------------

by schmittjoh at 2012-04-17T18:27:08Z

The config component is basically a super intelligent version of array_merge and the like.

About performance, I haven't really done any tests to say something about the impact. I think it's safe to say that it would be at least slower than your implementation in its current form due to the additional indirection. However, we could probably add a caching layer.

---------------------------------------------------------------------------

by bschussek at 2012-04-17T18:31:22Z

Have you checked the README I wrote? Are you sure the Config component is intended for the same purpose and not *way* too complex in this case?

---------------------------------------------------------------------------

by stof at 2012-04-17T18:51:14Z

You also forgot to update the ``replace`` section of the root composer.json file.

And regarding doing such thing with the Config Definition stuff, it would be more difficult: it builds the tree of values with their defaults, and then merges stuff coming from different sources. The form component however receives defaults from different places (which also define the allowed keys at the same time) and then receives user options only once. And it needs to handle easily default values which depend from other values. So I think both implementations are useful for different needs (however, we could argue about making it a subnamespace in the Config component, but this would add yet another different stuff in it)

---------------------------------------------------------------------------

by jalliot at 2012-04-17T18:58:03Z

@bschussek You need to add this component to the main composer.json too.

---------------------------------------------------------------------------

by lsmith77 at 2012-04-18T06:54:17Z

doesn't this overlap a bit with the ``TreeBuilder`` in the Config component?

---------------------------------------------------------------------------

by lsmith77 at 2012-04-18T06:59:12Z

ah just saw @stof's comment .. i think the biggest argument against TreeBuilder is that it was designed for a very specific purpose and performance wasn't one of them. where as Form needs something that performs fast. so yeah i do see different use cases, but i don't think this means we should have a new component.

furthermore while i haven't read the code in details i am surprised it doesn't make use of http://php.net/manual/en/function.array-replace-recursive.php to merge defaults into a user supplied options array.

---------------------------------------------------------------------------

by bschussek at 2012-04-18T08:10:49Z

@stof, @jalliot: Fixed.

> furthermore while i haven't read the code in details i am surprised it doesn't make use of http://php.net/manual/en/function.array-replace-recursive.php to merge defaults into a user supplied options array.

@lsmith77: Because that's not what this component does. The key feature of this component is to resolve default values of options that depend on the *concrete* values of other options. I invite you to read the README.

Is it a good idea to merge this into Config? I think that both components address different audiences and different purposes. The idea of this one is to initialize classes with simple, run-time provided arrays. The idea of Config is to load and validate complex configurations from storage providers, such as the filesystem.

---------------------------------------------------------------------------

by bschussek at 2012-04-18T08:18:48Z

Note: Not all relevant code of this component is shown in the diff. The (crucial) Options and LazyOption classes have only been moved out of Form.

---------------------------------------------------------------------------

by lsmith77 at 2012-04-18T08:20:02Z

> Is it a good idea to merge this into Config? I think that both components address different audiences and different purposes. The idea of this one is to initialize classes with simple, run-time provided arrays. The idea of Config is to load and validate complex configuration values from the filesystem (typically).

decoupled is all fine, but to me this feels a bit too granular. but i am just expressing a gut feeling here

---------------------------------------------------------------------------

by jalliot at 2012-04-18T08:34:03Z

I think too it should be included in the config component (maybe in a subnamespace). Indeed the behaviour is too different to be merged into the current component but its purpose is similar and is all about *configuration* (hence the name of the component). Otherwise we could also split the current Config component into smaller components as it seems to me there are already parts of it that are totally unrelated to each other.

---------------------------------------------------------------------------

by bschussek at 2012-04-18T11:30:55Z

@jalliot Can you go into detail which parts that are and what changes you suggest?

@kriswallsmith Any other naming suggestion?

---------------------------------------------------------------------------

by jalliot at 2012-04-18T11:34:35Z

@bschussek I don't know the current component well enough but that's the impression I had last time I looked at its code but I may be wrong.

---------------------------------------------------------------------------

by stof at 2012-04-18T19:30:43Z

@bschussek the Definition subnamespace of the Config component is standalone. It is not directly related to the Loader part

---------------------------------------------------------------------------

by bschussek at 2012-04-19T09:32:48Z

@stof So what do you recommend?

I think this is also a question of marketing. Is the Definition subnamespace intended to be used totally separately of the loaders? What are the use cases? If there are good use cases, it makes sense to me to extract the Definition part into a separate component. Otherwise not.

It is also a question of marketing, because the purpose of a component should be communicable in simple words (quoting @fabpot). The purpose of Config is (copied from the README):

> Config provides the infrastructure for loading configurations from different data sources and optionally monitoring these data sources for changes. There are additional tools for validating, normalizing and handling of defaults that can optionally be used to convert from different formats to arrays.

I think this purpose is completely different than that of OptionsParser.

---------------------------------------------------------------------------

by stof at 2012-04-19T11:39:50Z

The current description itself shows the current state: what is advocated as the main goal of the component (and was the original part) is the loader stuff. But the Definition part (mentioned as "additional tools") is bigger in term of LOC

---------------------------------------------------------------------------

by bschussek at 2012-04-19T11:55:17Z

@stof: Yes, this is a fact, but what's your opinion? How do we proceed with this PR?

---------------------------------------------------------------------------

by stof at 2012-04-19T12:21:44Z

Well, my opinion is that the current Config component may deserve to be split into 2 components (as someone may need only part of it). But this would be a huge BC break. @fabpot what do you think ?

---------------------------------------------------------------------------

by bschussek at 2012-04-23T10:14:57Z

@fabpot Can we merge this?

---------------------------------------------------------------------------

by fabpot at 2012-05-10T06:45:20Z

@bschussek I'm +1 for this PR but as mentioned by @kriswallsmith, we must find another name as `OptionsParser` immediately make me think of something related to the CLI.

---------------------------------------------------------------------------

by stof at 2012-05-10T06:47:45Z

However, after thinking about it again, I would vote for keeping it in its own component instead of adding yet another independant part in Config, to avoid forcing Form users to get the whole Config component

---------------------------------------------------------------------------

by bschussek at 2012-05-10T09:09:36Z

I'm having difficulties finding a better name. The main difference to CLI option parsers is that these actualy *parse* a string, while this class only receives an array of options (does not do any parsing). Otherwise both have the same purpose.

A couple of other suggestions:

* OptionsLoader (likely confused with our filesystem loaders)
* OptionsResolver
* OptionsMerger
* OptionsMatcher (not accurate)
* OptionsBuilder (likely confused with the builder pattern)
* OptionsJoiner
* OptionsBag (likely confused with the session bags)
* OptionsConfig (likely confused with Config)
* OptionsDefinition (likely confused with Config\Definition)
* OptionsSpec
* OptionsCombiner
* OptionsInitializer
* OptionsComposer

The difficulty is to find a name that best reflects its purpose:

```
$parser->setDefaults(...);
$parser->setRequired(...);
$parser->setOptional(...);
$parser->setAllowedValues(...)
$parser->parse($userOptions);
```

The only of the above examples that makes sense to me here is OptionsResolver -> resolve($userOptions).

Ideas?

---------------------------------------------------------------------------

by stof at 2012-05-10T09:56:54Z

OptionsResolver seems a better name than OptionsParser

---------------------------------------------------------------------------

by luxifer at 2012-05-10T09:59:45Z

Agree with @stof

---------------------------------------------------------------------------

by r1pp3rj4ck at 2012-05-10T10:03:53Z

I don't really like the plural in the name, but OptionsResolver seems better than OptionsParser. OptionResolver maybe?

---------------------------------------------------------------------------

by sstok at 2012-05-10T10:10:14Z

@r1pp3rj4ck Options makes more sense as they can be nested/deeper, and thus are multiple.

Agree with @stof also.

---------------------------------------------------------------------------

by r1pp3rj4ck at 2012-05-10T10:13:01Z

@sstok well, we have multiple events too and the name is EventDispatcher, not EventsDispatcher. Actually none of the component names are plural.

---------------------------------------------------------------------------

by newicz at 2012-05-10T10:33:50Z

OptionsResolver - I find it suggesting that there is some kind of problem to be resolved and there's not,
maybe OptionsDefiner but it isn't good aswell this is a tough one
2012-05-15 10:14:33 +02:00
jaugustin
9215c4478f [Form] fix failing tests for remove call on an objectCollection 2012-05-14 20:50:10 +02:00
Bernhard Schussek
076a104e86 [Form] Created failing test for PropertyPath modifying collections while iterating them 2012-05-14 20:50:04 +02:00
Bernhard Schussek
256b7081a4 [OptionsParser] Renamed OptionsParser to OptionsResolver 2012-05-14 19:35:07 +02:00
Bernhard Schussek
b9d053edb2 [Form] Moved Options classes to new OptionsParser component 2012-05-14 19:35:07 +02:00
Romain Geissler
47605f63e3 [Form][DataMapper] Do not update form to data when form is read only 2012-05-14 17:35:21 +02:00
William DURAND
ceb5ce6e5e [Form] fixed tests 2012-05-12 19:09:49 +02:00
William DURAND
df36afb123 [Form] Added tests 2012-05-12 18:44:54 +02:00
William DURAND
6d5ad3b289 [Form] Added right HTML types to Datetime/Date/Time types if single_text is true 2012-05-12 18:44:53 +02:00
Fabien Potencier
62594291e4 [Form] fixed wrong class path (closes #4239) 2012-05-10 12:15:10 +02:00
Fabien Potencier
3719c70870 updated minimum PHP version to 5.3.3
5.3.3 has some interesting fixes and this is the version used by
Redhat 6 and Debian 6
2012-05-07 10:29:11 +02:00
Fabien Potencier
5bed5f3c2c merged branch willdurand/fix-components (PR #4155)
Commits
-------

c195957 [Components] Tests/Autoloading fixes

Discussion
----------

Fix components

See #4141

----
This PR:

* configures each component to use composer to manage "dev" dependencies instead of env variables;
* adds phpunit configuration file on Filesystem component;
* fixes READMEs.

It's mergeable without any problems, but I would recommend to wait a fix in Composer in order to use `self.version` in `require`/`require-dev` sections.

Note: I kept `suggest` sections because it makes sense but this PR doesn't aim to provide useful explanations for each entry. It could be another PR, not that one.

---------------------------------------------------------------------------

by willdurand at 2012-04-30T20:43:13Z

@fabpot I reviewed each component, one by one. Now `phpunit` always works, even if tests are skipped. A simple `composer install --dev` allows to run the complete test suite. Each commit is well separated from the others. I guess, everything is ok now.

---------------------------------------------------------------------------

by Tobion at 2012-04-30T20:47:00Z

Please squash, as it makes no sense to have the same commit for each component.

---------------------------------------------------------------------------

by fabpot at 2012-05-01T14:26:11Z

Can you squash your commits before I merge? Thanks.

---------------------------------------------------------------------------

by willdurand at 2012-05-01T14:29:38Z

done

---------------------------------------------------------------------------

by fabpot at 2012-05-01T15:48:25Z

It does not seem that the commits are squashed.

---------------------------------------------------------------------------

by willdurand at 2012-05-01T15:54:08Z

done
2012-05-01 17:59:34 +02:00
William DURAND
c1959571ac [Components] Tests/Autoloading fixes
* Switched to Composer to manage "dev" dependencies
* Fixed READMEs
* Excluded vendor in phpunit.xml.dist files
* Fixed message in bootstrap.php files
* Added autoloader for the component itself
2012-05-01 17:51:41 +02:00
Fabien Potencier
26f933e7bd fixed CS 2012-05-01 15:23:48 +02:00
Fabien Potencier
9fbf8555f0 Revert "merged branch Seldaek/master (PR #4133)"
This reverts commit 00e7a94a8c, reversing
changes made to a01dec00f4.
2012-04-27 19:55:40 +02:00
Fabien Potencier
091ede2dd1 merged branch bschussek/issue3994 (PR #4046)
Commits
-------

246c885 [Form] Fixed: Default value of 'error_bubbling' is now determined by the 'single_control' option
d3bb4d0 [Form] Renamed option 'primitive' to 'single_control'
167e64f [Form] Fixed: Field attributes are not rendered in the label anymore. Label attributes are now passed in "label_attr"
68018a1 [Form] Dropped useless test that is guaranteed by OptionsParser tests and that needs to be adapted very often
649752c [Form] Fixed: CSRF token was not displayed on empty complex forms
c623fcf [Form] Fixed: CSRF protection did not run if token was missing
eb75ab1 [Form] Fixed results of the FieldType+FormType merge.

Discussion
----------

[Form] Fixed errors introduced in the FieldType+FormType merge

Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #3994, #4000, #2294, #4118
Todo: -

![Travis Build Status](https://secure.travis-ci.org/bschussek/symfony.png?branch=issue3994)

---------------------------------------------------------------------------

by Tobion at 2012-04-22T15:39:20Z

`primitive` is a pretty abstract option name. It depends on the person what he considers primitive. Maybe more explicit naming or better documentation what it means.

---------------------------------------------------------------------------

by bschussek at 2012-04-22T15:47:29Z

Better suggestions?

The distinction here is between primitive and complex forms, where primitive forms are such forms that can be represented by a single HTML tag. This obviously needs to be documented.

---------------------------------------------------------------------------

by Tobion at 2012-04-22T15:49:45Z

Maybe `single_widget` or something like that.

---------------------------------------------------------------------------

by vicb at 2012-04-23T13:09:43Z

@Tobion @bschussek would `elementary` be better than `primitive` ?

---------------------------------------------------------------------------

by vicb at 2012-04-23T13:17:04Z

and `compound \ composite` better than `complex` ?

---------------------------------------------------------------------------

by bschussek at 2012-04-23T14:08:33Z

@vicb I fail to see how elementary/compound is easier to understand than primitive/complex. Maybe single_widget, but what's the opposite of this case? multi_widget?

---------------------------------------------------------------------------

by vicb at 2012-04-23T14:15:09Z

Actually I am fine with anything... as long as it is documented.

---------------------------------------------------------------------------

by bschussek at 2012-04-23T14:22:31Z

Still I think that this unveals a more profound naming problem. How do we (also in the documentation) name forms with children (formerly "forms") and forms without children (formerly "fields")?

Should we refer to them as

* forms and fields?
* complex and primitive forms?
* ...

We must first answer this question before we can find an intuitive option name. If the documentation always switches between different terminologies, neither will it be understandable nor will this option be easy to remember.

---------------------------------------------------------------------------

by vicb at 2012-04-23T15:10:32Z

> Still I think that this unveals a more profound naming problem. How do we (also in the documentation) name forms with children (formerly "forms") and forms without children (formerly "fields")?

To make it clear, I would rather say forms that **can have** children and forms that **can not have** children (i.e. Empty collections have no children but they can have and this is reason why you have to introduce those options, right ? - that could be a good example for the doc).

It will probably be better to refer to "complex" / "primitive" forms in the doc (and use the "form" / "field" terms to explain them).

Note: I think @Tobion concern is that "primitive" / "complex" could be pejorative terms (this is why I have proposed "elementary" / "compound").

---------------------------------------------------------------------------

by Tobion at 2012-04-23T16:00:54Z

1. primitive/complex is subjective (and could be pejorative too)
2. elementary/compound is more explicit so probably better than primitive/complex
3. I dislike this option in general. Does it make sense to change this option from a user perspective? I guess it's always the same as long as the widget structure stays the same. So it should be resolved at a higher level dynamically from the widget structure and not exposed to any configuration.
4. In documentation I would use the terms forms and fields. Because all people with HTML knowledge will understand that fields cannot have sub-fields whereas forms can. But since this distinction is not findable in code, it should be mentioned that all these are implemented as a form hierarchy.

---------------------------------------------------------------------------

by mvrhov at 2012-04-23T16:02:00Z

how about simple and complex?

---------------------------------------------------------------------------

by bschussek at 2012-04-23T16:06:33Z

@Tobion It does not make sense to change this option from the user perspective, still the overloading type has to propagate to FormType whether it is a form or a field, so that the default behaviour is correct.

A second option how to implement this is to add a method `isField` to FormTypeInterface that can be overloaded and receives the options. I don't really like to introduce new methods here unless absolutely required.

What about renaming the option "primitive" to "is_field"? The blocks in the template would then be named "form_widget_field" and "form_widget_form".

---------------------------------------------------------------------------

by tristanbes at 2012-04-25T14:01:06Z

Oh, I should've seen this before, i thought I was doing something wrong. (empty collections gets an input field bug)

Please big :UP: on this. When will it be merged ? @bschussek

---------------------------------------------------------------------------

by Tobion at 2012-04-25T15:30:28Z

+1 for "is_field" and "form_widget_field" but I would rather use "form_widget_compound" instead of "form_widget_form" which is quite strange.

---------------------------------------------------------------------------

by bschussek at 2012-04-26T16:34:04Z

@Tobion "simple" and "compound" then?

---------------------------------------------------------------------------

by Tobion at 2012-04-26T16:49:58Z

no "field" and "compound"

---------------------------------------------------------------------------

by bschussek at 2012-04-26T17:17:02Z

I don't like "field" for a simple reason: Consider the "date" type. We are typically speaking of the "date" field there. But technically, the "date" field is a compound field. So?

---------------------------------------------------------------------------

by Tobion at 2012-04-26T21:17:37Z

I don't understand the open question. You proposed "is_field" and "form_widget_field" yourself. So calling the template block "form_widget_field" is a comprehensible consequence of "is_field". I wouldn't call the date type with multiple inputs a field.

---------------------------------------------------------------------------

by tristanbes at 2012-04-26T21:52:39Z

We should take a decision cause right here i got all my forms that are broken because of the empty collection rendering as input field :-).

I guess we are many in that situation.

---------------------------------------------------------------------------

by bschussek at 2012-04-27T08:28:16Z

I renamed "primitive" to "single_control" now to match with the HTML specification which names all input elements (input, select etc.) "controls". The opposite is now "compound".

Meanwhile, I added a fix for #4118.

@fabpot This is ready for merge now.

---------------------------------------------------------------------------

by Tobion at 2012-04-27T10:22:49Z

Hm, I know naming things is hard and sometimes not really important. But since users need to know which block to override, it is essential to make it clear. I think there is still one issue.
The block is named `form_widget_single_control` in order, as you said, to abstract away if it's an input, select etc. But in fact it can only render `input` and nothing else. So this is misleading.
So you could also simply name it `form_widget_input`.
Apart from that I agree with everything.
2012-04-27 19:13:34 +02:00
Jordi Boggiano
00c4267726 Update branch aliases 2012-04-27 12:47:50 +02:00
Bernhard Schussek
246c8852c8 [Form] Fixed: Default value of 'error_bubbling' is now determined by the 'single_control' option 2012-04-27 10:24:06 +02:00
Bernhard Schussek
d3bb4d085c [Form] Renamed option 'primitive' to 'single_control' 2012-04-27 10:18:25 +02:00
Bernhard Schussek
167e64f799 [Form] Fixed: Field attributes are not rendered in the label anymore. Label attributes are now passed in "label_attr" 2012-04-27 09:48:34 +02:00
Bernhard Schussek
68018a10da [Form] Dropped useless test that is guaranteed by OptionsParser tests and that needs to be adapted very often 2012-04-27 09:47:17 +02:00
Bernhard Schussek
649752c947 [Form] Fixed: CSRF token was not displayed on empty complex forms 2012-04-27 09:47:16 +02:00
Bernhard Schussek
c623fcf4d4 [Form] Fixed: CSRF protection did not run if token was missing 2012-04-27 09:47:16 +02:00
Bernhard Schussek
eb75ab1b74 [Form] Fixed results of the FieldType+FormType merge. 2012-04-27 09:47:16 +02:00
Fabien Potencier
7048172e3f [Form] added CHANGELOG 2012-04-26 22:38:50 +02:00
Christian Raue
f287f0804d removed superfluous public modifier from interface methods 2012-04-24 19:15:04 +02:00
Christian Raue
ee57c04b24 added missing dot in FormType as follow-up to #3922 2012-04-24 01:06:23 +02:00
Bernhard Schussek
d9e142bd62 [Form] Restored and deprecated method guessMinLength in FormTypeGuesser 2012-04-23 16:02:44 +02:00
julien.galenski
f7200e479c [Form] added method guessPattern to FormTypeGuesserInterface
rephrase changelog
2012-04-23 12:28:18 +02:00
Fabien Potencier
2786f21822 fixed CS 2012-04-20 09:05:48 +02:00
Fabien Potencier
086b73506d merged branch pvanliefland/ticket_1692 (PR #3945)
Commits
-------

8bdff01 [DoctrineBridge][Form] added collection guess for array Doctrine type and array constraint type

Discussion
----------

[Form] [DoctrineBridge] Better field type guessing for array doctrine type and array validator type

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1692
Todo: -

---------------------------------------------------------------------------

by bschussek at 2012-04-18T08:45:17Z

Could you please add an entry to the CHANGELOG and squash your commits into one?

---------------------------------------------------------------------------

by pvanliefland at 2012-04-18T17:20:39Z

Done
2012-04-19 13:13:47 +02:00
Bernhard Schussek
ccd6bbc0a1 [Form] Removed extra CSRF field on collection prototype 2012-04-19 11:00:26 +02:00
pierre
8bdff01f17 [DoctrineBridge][Form] added collection guess for array Doctrine type and array constraint type
Fixes #1692
2012-04-18 19:15:40 +02:00
Bernhard Schussek
6e4ed9e177 [Form] Fixed regression: bind(null) was not converted to an empty string anymore 2012-04-17 17:29:15 +02:00
Bernhard Schussek
fcb2227ac9 [Form] Deprecated FieldType, which has been merged into FormType 2012-04-17 16:44:39 +02:00
Bernhard Schussek
bfa7ef2d9b [Form] Removed obsolete exceptions 2012-04-17 16:44:38 +02:00
Bernhard Schussek
2a49449862 [Form] Simplified CSRF mechanism and removed "csrf" type
CSRF fields are now only added when the view is built. For this reason we already know if
the form is the root form and avoid to create unnecessary CSRF fields for nested fields.
2012-04-17 16:44:38 +02:00
Joseph Bielawski
bfc1aafff6 [Tests] Use proper assertions 2012-04-14 10:05:05 +02:00
Christophe Coevoet
538819a8bf [Form] Fixed the tests 2012-04-13 19:11:38 +02:00
Christophe Coevoet
9e956a8ecd [Form] Cleaned the FormValidatorInterface deprecation 2012-04-13 18:57:23 +02:00
Bernhard Schussek
6df7a7223e [Form] Deprecated FormValidatorInterface and moved implementations to event listeners 2012-04-13 16:42:01 +02:00
Fabien Potencier
cec8fed31c merged branch willdurand/add-all-method-to-form-builder (PR #3886)
Commits
-------

be2456b [Form] [Tests] Used assertCount()
4120f13 [Form] Added all() method to the FormBuilder class

Discussion
----------

[Form] Added all() method to the FormBuilder class

In order to perform some introspection on a FormBuilder instance,
we need to be able to get its children. Almost everything is accessible
in this class, but the children are not.

This PR adds a all() method to respect the current API (get(), remove(),
...).

---------------------------------------------------------------------------

by bschussek at 2012-04-12T09:54:04Z

👍
2012-04-13 06:56:27 +02:00
William DURAND
be2456b19e [Form] [Tests] Used assertCount() 2012-04-11 20:45:41 +02:00
William DURAND
4120f1392f [Form] Added all() method to the FormBuilder class
In order to perform some introspection on a FormBuilder instance,
we need to be able to get its children. Almost everything is accessible
in this class, but the children are not.

This PR adds a all() method to respect the current API (get(), remove(),
...).
2012-04-11 19:12:00 +02:00
William DURAND
6f56dfc0d6 [Form] Fixed DateType default options 2012-04-11 18:56:33 +02:00