[3.0] Clean Form, Validator, DowCrawler and some more #16075

nicolas-grekas · 2015-10-02T13:53:49Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

Tobion · 2015-10-02T14:19:19Z

src/Symfony/Component/Form/FormFactory.php

@@ -61,36 +61,12 @@ public function createForProperty($class, $property, $data = null, array $option
     */
    public function createBuilder($type = 'Symfony\Component\Form\Extension\Core\Type\FormType', $data = null, array $options = array())
    {
-        $name = null;
-        $typeName = null;
-
        if ($type instanceof ResolvedFormTypeInterface) {


getBlockPrefix must be added to ResolvedFormTypeInterface

I'm not sure we actually need to keep support for ResolvedFormTypeInterface in the factory. it is not supported according to FormFactoryInterface

I think there is a bug in the current implementation. createBuilder has logic to handle ResolvedFormTypeInterface, but it then calls createNamedBuilder which will throw an exception as it requires a string as the type.

Yeah, it must be removed. It was deprecated in 2.8: https://github.com/symfony/symfony/blob/2.8/src/Symfony/Component/Form/FormFactory.php#L114
So it must be removed from this method as well.

https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/FormFactoryInterface.php must be updated as well as it still says string|FormTypeInterface $type

Updated, can you please review again?

Tobion · 2015-10-02T14:34:56Z

src/Symfony/Component/Form/FormFactoryInterface.php

-     * @param string|FormTypeInterface $type    The type of the form
-     * @param mixed                    $data    The initial data
-     * @param array                    $options The options
+     * @param string $type    The type of the form


there are more of it in the interface

harikt · 2015-10-02T15:11:10Z

src/Symfony/Component/Form/Extension/Validator/EventListener/ValidationListener.php

-    /**
-     * @param ValidatorInterface       $validator
-     * @param ViolationMapperInterface $violationMapper
-     */


May I know what is the reason of removing the docblock comments ?

they provide no value at all, this is just an exact repetition of the method signature

ok, thanks. I wonder whether this have any impact on the sami / phpdoc .

Tobion · 2015-10-02T15:14:20Z

Please also remove max_length from https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/FormFactoryInterface.php#L96

nicolas-grekas · 2015-10-02T15:16:09Z

Removed

Tobion · 2015-10-02T15:27:01Z

The choice_list option must be removed still I think.

nicolas-grekas · 2015-10-02T15:56:03Z

@Tobion I agree with you, but this is more tricky and could be left for an other PR.
I added a few more removals btw.

fabpot · 2015-10-02T16:33:57Z

The choice_list removal should be done as its own PR.

Tobion · 2015-10-02T16:37:57Z

src/Symfony/Component/DomCrawler/Crawler.php

@@ -846,136 +846,6 @@ public static function xpathLiteral($s)
    }

    /**
-     * @deprecated Using the SplObjectStorage API on the Crawler is deprecated as of 2.8 and will be removed in 3.0.
-     */
-    public function attach($object, $data = null)


removing this without actually removing extends \SplObjectStorage makes no sense

Tobion · 2015-10-02T16:40:10Z

Apart from the changes in Crawler 👍

nicolas-grekas · 2015-10-02T16:57:51Z

Crawler updated

nicolas-grekas · 2015-10-02T17:08:17Z

src/Symfony/Component/Debug/ExceptionHandler.php

@@ -38,12 +38,6 @@ class ExceptionHandler

    public function __construct($debug = true, $charset = null, $fileLinkFormat = null)
    {
-        if (false !== strpos($charset, '%')) {
-            // Swap $charset and $fileLinkFormat for BC reasons


fabpot · 2015-10-02T17:34:13Z

Thank you @nicolas-grekas.

…(nicolas-grekas) This PR was merged into the 3.0-dev branch. Discussion ---------- [3.0] Clean Form, Validator, DowCrawler and some more | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | - | License | MIT | Doc PR | - Commits ------- abca2d6 [3.0] Clean Form, Validator, DowCrawler and some more

This PR was merged into the 2.8 branch. Discussion ---------- Add a few additional tests for the Crawler | Q | A | ------------- | --- | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a While looking at the update of the Crawler in #16075, I spotted a few mistakes. But these were cases not covered by the testsuite. This is adding tests for these cases in the 2.8 branch (they could be added in 2.3 eventually though). Commits ------- 528d3bd Add a few additional tests for the Crawler

This PR was merged into the 3.0-dev branch. Discussion ---------- Fix the crawler refactoring | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | n/a | License | MIT | Doc PR | n/a This fixes a few mistakes I spotted in #16075 for the DomCrawler component. Regression tests are added separately in #16093 to be included in older branches too. Commits ------- d128735 Fix the crawler refactoring

renatomefi · 2015-12-16T13:16:35Z

Hello @nicolas-grekas
I see you removed the support for createForm using objects:
abca2d6#diff-5c1348d69be32426ff20446c9e812365R64

How can I create forms which comes from a service for instance?

stof · 2015-12-16T13:30:28Z

@renatomefidf defining form types as service never required passing the instance to createForm. If you are passing an instance here, it means you are not using your form type as a service as far as the Form component is concerned.
When the form type is registered as a service, using it only involves using its name (see how you use the Symfony core types: they are defined as services)

renatomefi · 2015-12-16T13:34:52Z

Thank you @stof
Its actually not a problem I'm facing, but a question in here:
http://stackoverflow.com/questions/33451498/symfony-3-define-form-as-a-service

Thanks again for helping and making it clearer!

stof · 2015-12-16T13:40:02Z

OK, I answered on SO itself

PaddyLock · 2016-02-12T16:25:01Z

@stof I am trying to do the same as @renatomefidf But I can't get it to work. Should I be using the form name defined in the form class within getBlockPrefix(), or the forms service name or the name defined in the service definition within
tags: - {name: 'form.my_name'}

I have tried with

$this->createForm('form.my_name', $entity);
$this->createForm('my_form_service_name', $entity);
$this->createForm(MyFormType::class, $entity);

The last one uses the form class but not the form as a service, which is what I need.

Thanks

xabbuh · 2016-02-12T16:49:40Z

@PaddyLock The tag name to add a form type service is form.type. But please use one of the support channels if you need further helper. Thank you!

carsonbot added the Status: Needs Review label Oct 2, 2015

nicolas-grekas force-pushed the master-clean branch 2 times, most recently from fa56ecd to da438fa Compare October 2, 2015 14:06

Tobion reviewed Oct 2, 2015
View reviewed changes

nicolas-grekas force-pushed the master-clean branch 2 times, most recently from f2d71b8 to 9d47437 Compare October 2, 2015 14:29

Tobion reviewed Oct 2, 2015
View reviewed changes

nicolas-grekas force-pushed the master-clean branch from 9d47437 to f1fecb4 Compare October 2, 2015 14:36

harikt reviewed Oct 2, 2015
View reviewed changes

nicolas-grekas force-pushed the master-clean branch from f1fecb4 to 14e6761 Compare October 2, 2015 15:15

nicolas-grekas force-pushed the master-clean branch from 14e6761 to f3478f9 Compare October 2, 2015 15:53

nicolas-grekas changed the title ~~[3.0] Cleaning Form and Validator a bit more~~ [3.0] Cleaning Form, Validator and some more Oct 2, 2015

Tobion reviewed Oct 2, 2015
View reviewed changes

nicolas-grekas force-pushed the master-clean branch from f3478f9 to 4ea2af5 Compare October 2, 2015 16:57

[3.0] Clean Form, Validator, DowCrawler and some more

abca2d6

nicolas-grekas force-pushed the master-clean branch from 4ea2af5 to abca2d6 Compare October 2, 2015 17:06

nicolas-grekas changed the title ~~[3.0] Cleaning Form, Validator and some more~~ [3.0] Clean Form, Validator, DowCrawler and some more Oct 2, 2015

nicolas-grekas reviewed Oct 2, 2015
View reviewed changes

fabpot merged commit abca2d6 into symfony:master Oct 2, 2015

This was referenced Oct 2, 2015

Add a few additional tests for the Crawler #16093

Merged

Fix the crawler refactoring #16094

Merged

fabpot mentioned this pull request Nov 16, 2015

Release v3.0.0-BETA1 #16560

Merged

xabbuh mentioned this pull request Nov 27, 2015

Deprecated choice_list option in ChoiceType? #16716

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[3.0] Clean Form, Validator, DowCrawler and some more #16075

[3.0] Clean Form, Validator, DowCrawler and some more #16075

nicolas-grekas commented Oct 2, 2015

Tobion Oct 2, 2015

Tobion Oct 2, 2015

Tobion Oct 2, 2015

Tobion Oct 2, 2015

Tobion Oct 2, 2015

nicolas-grekas Oct 2, 2015

Tobion Oct 2, 2015

nicolas-grekas Oct 2, 2015

harikt Oct 2, 2015

nicolas-grekas Oct 2, 2015

harikt Oct 2, 2015

Tobion commented Oct 2, 2015

nicolas-grekas commented Oct 2, 2015

Tobion commented Oct 2, 2015

nicolas-grekas commented Oct 2, 2015

fabpot commented Oct 2, 2015

Tobion Oct 2, 2015

Tobion commented Oct 2, 2015

nicolas-grekas commented Oct 2, 2015

nicolas-grekas Oct 2, 2015

fabpot commented Oct 2, 2015

renatomefi commented Dec 16, 2015

stof commented Dec 16, 2015

renatomefi commented Dec 16, 2015

stof commented Dec 16, 2015

PaddyLock commented Feb 12, 2016

xabbuh commented Feb 12, 2016

[3.0] Clean Form, Validator, DowCrawler and some more #16075

[3.0] Clean Form, Validator, DowCrawler and some more #16075

Conversation

nicolas-grekas commented Oct 2, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tobion commented Oct 2, 2015

nicolas-grekas commented Oct 2, 2015

Tobion commented Oct 2, 2015

nicolas-grekas commented Oct 2, 2015

fabpot commented Oct 2, 2015

Choose a reason for hiding this comment

Tobion commented Oct 2, 2015

nicolas-grekas commented Oct 2, 2015

Choose a reason for hiding this comment

fabpot commented Oct 2, 2015

renatomefi commented Dec 16, 2015

stof commented Dec 16, 2015

renatomefi commented Dec 16, 2015

stof commented Dec 16, 2015

PaddyLock commented Feb 12, 2016

xabbuh commented Feb 12, 2016