Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 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
weaverryan commentedDec 26, 2017
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 toacc04c2Comparenicolas-grekas commentedDec 26, 2017 • 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.
Implementation now ready with the full list of Twig functions/filters handled by the bridge. |
xabbuh 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.
Theform_enctype() function is missing. What about the filters from theCodeExtension?
| '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 andyaml_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 andtranschoice are filters
acc04c2 to97438c8Comparenicolas-grekas commentedDec 27, 2017 • 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.
@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 to3bc8bdeCompare| 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 toac891acComparefabpot commentedDec 30, 2017
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
Uh oh!
There was an error while loading.Please reload this page.
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.