-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Add completion for debug:twig #43846
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
@fabpot disabled IDE and tried to clean my code. |
You can ignore fabbot :) |
OK 🤣 |
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.
Thank you for the PR!
I've added a couple comments, seems like you missed some things while applying some of Fabien's comments :) If this is fixed, it should be ready to go imho
@wouterj thanks for code review. I missed that we have two methods for execute() displayGeneralText() and displayGeneralJson() |
$names = []; | ||
$paths = $this->getLoaderPaths(); | ||
|
||
foreach ($paths as $namespace => $paths) { | ||
$names[] = $namespace; | ||
} | ||
|
||
return $names; |
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.
This can be replaced with array_keys($this->getLoaderPaths())
(and can be inlined on line 118 directly)
|
||
private function getAvailableFormatOptions(): array | ||
{ | ||
return ['text', 'json']; |
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.
can be inlined as well
@wouterj thanks optimized code :) |
Thank you @StaffNowa. |
Add completion for debug:twig