-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[TwigBundle/Brige] catch missing requirements to throw meaningful exceptions #25601
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
Thanks for this Nicolas. Obviously, this is a one-off solution, but it IS one of the most common situations. So... are you thinking w could merg this and start with this alone? Or do you intend for this to turn into something bigger? |
dbd2d2e
to
acc04c2
Compare
Implementation now ready with the full list of Twig functions/filters handled by the bridge. |
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.
The form_enctype()
function is missing. What about the filters from the CodeExtension
?
'workflow_transitions' => 'workflow', | ||
'workflow_has_marked_place' => 'workflow', | ||
'workflow_marked_places' => 'workflow', | ||
'yaml_encode' => 'yaml', |
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.
yaml_encode
and yaml_dump
are filters
'logout_url' => 'security-http', | ||
'logout_path' => 'security-http', | ||
'is_granted' => 'security-core', | ||
'trans' => 'translation', |
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.
trans
and transchoice
are filters
acc04c2
to
97438c8
Compare
@xabbuh thanks, comments addressed.
deprecated in 2.3, removed in 3.0
http-kernel/foundation/routing are always installed with Flex, no need for special care |
97438c8
to
3bc8bde
Compare
return false; | ||
} | ||
|
||
throw new SyntaxError(sprintf('Did you forget to run "composer require symfony/%s"? Unknown filter "%s".', $name, self::$filterComponents[$name])); |
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.
Unknown filter "%s".
this must be last in the message because the twig file/line is appended later on
([...] Unknown filter "%s" in foo.html.twig on line 123)
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 would add a comment in the code directly.
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.
comment added in the code
3bc8bde
to
ac891ac
Compare
Thank you @nicolas-grekas. |
…ningful exceptions (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- [TwigBundle/Brige] catch missing requirements to throw meaningful exceptions | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #25256, symfony/webpack-encore#173 | License | MIT | Doc PR | - It is possible to register a handler for undefined twig functions/filters. IMHO, this is the hook point we should leverage to throw meaningful exception messages. This works well on its own. We now just need to list the functions/filters with appropriate messages. There is one case we could enhance: at warmup time, Twig exceptions are swallowed, thus not visible. Shouldn't we make these visible instead? ping @weaverryan since this is related to two issues of yours. Commits ------- ac891ac [TwigBundle/Brige] catch missing requirements to throw meaningful exceptions
It is possible to register a handler for undefined twig functions/filters.
IMHO, this is the hook point we should leverage to throw meaningful exception messages.
This works well on its own. We now just need to list the functions/filters with appropriate messages.
There is one case we could enhance: at warmup time, Twig exceptions are swallowed, thus not visible.
Shouldn't we make these visible instead?
ping @weaverryan since this is related to two issues of yours.