Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Form][OptionsResolver] Show deprecated options definition on debug:form command#27667
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
b252112 toe2e05f9Comparelinaori commentedJun 21, 2018
Will it be possible to use this command to findall form types that have a deprecated option, and see which of those options are deprecated per type? That would make fixing them a walk in the park! |
yceruto commentedJun 21, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@iltar sure, I'll add an option Status: Needs Work |
69cf93f to150c14dCompareyceruto commentedJun 24, 2018
Added
Status: Needs Review (Travis failures aren't related) |
a42dfa2 to64f444bCompareyceruto commentedJul 7, 2018
Any update about this? @ogizanagi you're working on this command in the past, so will be great to have your opinion on this! thanks. :) |
ogizanagi left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Makes sense. Thanks@yceruto for working on this!
| /** | ||
| * @return string|\Closure | ||
| * | ||
| * @throws NoConfigurationException on no configured normalizer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
on no configured deprecation
| return$definition; | ||
| } | ||
| protectedfunctionfilterTypesByDeprecated(array$types):array |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
To be consistent with current code, should this rather live in the command and pass deprecated types & options to the descriptor as$options, so the descriptors' responsibility sticks to handling output?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I agree, but in that case should we to movecollectOptions(),getParentOptionsResolver(), etc, too?
ogizanagiJul 15, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I guess it's okay as we aim to describe OptionResolver and ResolvedFormTypeInterface instances. What is a bit weird here to me is injecting the form registry as an option.
But no strong opinion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Okay, done.
| for ($i =0;$i <$count; ++$i) { | ||
| $cells =array(); | ||
| foreach (array_keys($headers)as$group) { | ||
| if (isset($options[$group][$i])) { |
ogizanagiJul 15, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Could be simplified a bit I guess:
diff --git a/src/Symfony/Component/Form/Console/Descriptor/TextDescriptor.php b/src/Symfony/Component/Form/Console/Descriptor/TextDescriptor.phpindex 1e63aa0c68..3702d2f6b1 100644--- a/src/Symfony/Component/Form/Console/Descriptor/TextDescriptor.php+++ b/src/Symfony/Component/Form/Console/Descriptor/TextDescriptor.php@@ -130,17 +130,12 @@ class TextDescriptor extends Descriptor for ($i = 0; $i < $count; ++$i) { $cells = array(); foreach (array_keys($headers) as $group) {- if (isset($options[$group][$i])) {- $option = $options[$group][$i];-- if (\is_string($option) && \in_array($option, $this->requiredOptions, true)) {- $option .= ' <info>(required)</info>';- }-- $cells[] = $option;- } else {- $cells[] = null;+ $option = $options[$group][$i] ?? null;+ if (\is_string($option) && \in_array($option, $this->requiredOptions, true)) {+ $option .= ' <info>(required)</info>'; }++ $cells[] = $option; } $tableRows[] = $cells; }
src/Symfony/Component/Form/Tests/Fixtures/Descriptor/types_with_deprecated_options.json OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
64f444b tof02ee12Compareyceruto commentedJul 15, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks for the review@ogizanagi! I've fixed most comments. (AppVeyor & Travis failures are unrelated) |
676e79d tocde4c4bCompareb2cf002 to856578cCompareyceruto commentedJul 30, 2018
Rebased and ready. |
fabpot left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
with minor comments
| ->setDefinition(array( | ||
| newInputArgument('class', InputArgument::OPTIONAL,'The form type class'), | ||
| newInputArgument('option', InputArgument::OPTIONAL,'The form type option'), | ||
| newInputOption('show-deprecated',null, InputOption::VALUE_NONE,'Used to show deprecated options in form types'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Display deprecated options in form types
| <info>php %command.full_name% ChoiceType choice_value</info> | ||
| Use the <info>--show-deprecated</info> option to display form types with deprecated options or the deprecated options of the given form type: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should be cut at around 80 chars
856578c to87c209dCompareyceruto commentedSep 4, 2018
@fabpot comments addressed, thanks! (AppVeyor failure is unrelated) |
fabpot commentedSep 4, 2018
Thank you@yceruto. |
…tion on debug:form command (yceruto)This PR was merged into the 4.2-dev branch.Discussion----------[Form][OptionsResolver] Show deprecated options definition on debug:form command| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | -| License | MIT| Doc PR | -Next move after#27277.It will look like this:Use `--show-deprecated` option to show form types with deprecated options (example):Use `--show-deprecated` option to show deprecated options of the given form type (example):Deprecated option (example):Commits-------87c209d Show deprecated options definition on debug:form command
Uh oh!
There was an error while loading.Please reload this page.
Next move after#27277.
It will look like this:
Use
--show-deprecatedoption to show form types with deprecated options (example):Use
--show-deprecatedoption to show deprecated options of the given form type (example):Deprecated option (example):
