-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Form] Add debug:form command #23694
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/Symfony/Bundle/FrameworkBundle/Command/FormDebugCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/FormDebugCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/FormDebugCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/FormDebugCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/FormDebugCommand.php
Outdated
Show resolved
Hide resolved
src/Symfony/Bundle/FrameworkBundle/Command/FormDebugCommand.php
Outdated
Show resolved
Hide resolved
I'm thinking in more options to avoid type the FQCN, so what about to search the form type inside registered types or installed bundles? $ bin/console debug:form EasyAdminFormType if there is more than one registered with the same suffix, we could display the first and print at the bottom a comment with another matches? |
d68b634
to
a4938b6
Compare
Failure on deps=high is normal. |
At first, I'd suggest to add the list of "preferred" namespaces to use as an optional constructor argument of the command.
If there is more than one match let's show a message saying the argument is ambiguous. |
@ogizanagi Sorry, I didn't see your last comment before submit these changes. |
Last update: We use the namespace from all known form types as base to search the form type. We show a comment when there is more than one type found. This mean, if there is at least one form type registered, its namespace will be used to find other ones not registered. |
a02cd92
to
1437e85
Compare
Not a blocker to me (and don't work on it right now, we need more opinions), but other debug commands are using |
The |
1871684
to
68d49b7
Compare
@ogizanagi I thought on this also at the beginning, but then I gave up, because this information can be obtained by using the command Even though, if any else votes for this, I'll be glad to implement it. |
@ogizanagi I had left this to the end, but now that you mention it, this suggestion #23694 (comment) could be the proper class to use descriptors. |
I also think that listing them all when no arg is passed would be nice, getting the same with About descriptors, I think that not using them is fine. The ones which describe component specific data are in the framework (container, router, ..), using them would prevent having the command in the form component. |
@chalasr : We could add our own in the component itself (or even just the |
d738b92
to
d67dc00
Compare
d67dc00
to
69d5260
Compare
Ready for me (just squashing commits). |
69d5260
to
4f040d7
Compare
why are the class "internal"? isn't their design clean already? |
All other descriptor classes were always made internal and it simplifies maintenance. I don't think we ever had requests to open these since they exist. Regarding the |
src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
Show resolved
Hide resolved
@stof I'm no sure :/ as It was the modus operandi and the consensus for other commands #23624, #23801, #23694 (comment) |
@ogizanagi should I move the |
@yceruto I would let it as is. If one asks for extensibility with a legit use case, we can move it easily, removing the internal flag. |
Same opinion as @chalasr |
Thanks @yceruto. |
This PR was merged into the 3.4 branch. Discussion ---------- [Form] Add debug:form command | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #23688 | License | MIT | Doc PR | -  A short class name (e.g. `DateType`) can be passed as `class` argument too (the command will try to resolve its FQCN if it's in known form type namespaces). Commits ------- 4f040d7 Add debug:form command
@yceruto Tests are failing on master, could you have a look? https://travis-ci.org/symfony/symfony/jobs/270338064 |
Sure, I'll take a look soon. |
Done in #24059 |
Thanks @ogizanagi! I'm really busy right now. |
…nagi) This PR was merged into the 3.4 branch. Discussion ---------- [Form] Add ambiguous & exception debug:form tests | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no <!-- don't forget updating src/**/CHANGELOG.md files --> | BC breaks? | no | Deprecations? | no <!-- don't forget updating UPGRADE-*.md files --> | Tests pass? | yes | Fixed tickets | complete #23694 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | N/A Just completing tests a bit for this new feature. cc @yceruto Commits ------- 35f9c0b [Form] Add ambiguous & exception debug:form tests
A short class name (e.g.
DateType
) can be passed asclass
argument too (the command will try to resolve its FQCN if it's in known form type namespaces).