Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[TwigBridge][Validator] Add the Twig constraint and its validator#58805
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
| // Another syntax error example (unclosed variable) | ||
| ['Hello {{ name', 'Unexpected token "end of template" ("end of print statement" expected) at line 1.', 1], | ||
| // Unknown filter error | ||
| ['Hello {{ name | unknown_filter }}', 'Unknown "unknown_filter" filter at line 1.', 1], |
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.
Here, with the default environment, all the following would be invalid template, even if they are on most of the Symfony docs page.
{%form_themeformresources %}{{name |trans }}{%trans_default_domaindomain %}{{object|serialize(format ='json',context = []) }}{{text|humanize }}{{name |trans }}So ... what does "Twig valid" mean ?
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.
Resolved by injecting the environment, as suggested by@derrabus#58805 (comment)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
What is the use case for this ? |
@stof the constraint is intended to validate the Twig syntax of user-customized templates (e.g., emails, reports, widgets) to prevent syntax-related runtime errors. |
The low-deps test failure appears to be caused by your changes. |
Uh oh!
There was an error while loading.Please reload this page.
| // Another syntax error example (unclosed variable) | ||
| ['Hello {{ name', 'Unexpected token "end of template" ("end of print statement" expected) at line 1.', 1], | ||
| // Unknown filter error | ||
| ['Hello {{ name | unknown_filter }}', 'Unknown "unknown_filter" filter at line 1.', 1], |
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 is a very defined syntax.
Here this is not the syntax that is validated, but the runtime Twig configuration.
And i still think it should not be exposed like this.
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 the thrown exception indicates a syntax error.
Seehttps://github.com/twigphp/Twig/blob/3.x/src/Error/SyntaxError.php
Uh oh!
There was an error while loading.Please reload this page.
| $value = (string) $value; | ||
| $prevErrorHandler = set_error_handler(static function ($level, $message, $file, $line) use (&$prevErrorHandler) { |
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.
A deprecated function is not valid ?
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.
If you don't want deprecated Twig syntax to be used in your templates, why not? But we should probably make this behavior configurable.
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.
As Twig is not aligned with Symfony release cycle, this seems a bit fragile to me, but why not with a flag, indeed.
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.
Good point. I've added theskipDeprecations option.
| ['Hello {{ name }}'], | ||
| ['{% if condition %}Yes{% else %}No{% endif %}'], | ||
| ['{# Comment #}'], | ||
| ['Hello {{ "world" | upper }}'], |
fabpotNov 10, 2024 • edited by OskarStark
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by OskarStark
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.
| ['Hello {{ "world" |upper }}'], | |
| ['Hello {{ "world"|upper }}'], |
And everywhere else where you're using the| operator.
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.
Done
f116b44 toaab17ecCompareUh oh!
There was an error while loading.Please reload this page.
| @@ -1,6 +1,11 @@ | |||
| CHANGELOG | |||
| ========= | |||
| 7.2 | |||
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.
| 7.2 | |
| 7.3 |
| 7.2 | ||
| --- | ||
| * Added support for the new `twig` validator (from Twig 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.
| *Added support for thenew`twig` validator (from Twig Bridge) | |
| *Add support for the`twig` validator |
src/Symfony/Bridge/Twig/CHANGELOG.md Outdated
| @@ -5,6 +5,7 @@ CHANGELOG | |||
| --- | |||
| * Deprecate passing a tag to the constructor of `FormThemeNode` | |||
| * Add the `Twig` constraint for validating Twig content | |||
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.
| * Add the`Twig` constraint for validating Twigcontent | |
| * Add the`Twig` constraint for validating Twigtemplates |
src/Symfony/Bridge/Twig/CHANGELOG.md Outdated
| @@ -5,6 +5,7 @@ CHANGELOG | |||
| --- | |||
| * Deprecate passing a tag to the constructor of `FormThemeNode` | |||
| * Add the `Twig` constraint for validating Twig content | |||
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 moved to a new 7.3 section.
| #[HasNamedArguments] | ||
| public function __construct( | ||
| public string $message = 'This value is not valid Twig.', |
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.
| public string$message ='This value is not valid Twig.', | |
| public string$message ='This value is notavalid Twig template.', |
Thank you@sfmok. |
6ae4c85 intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
…fmok)This PR was merged into the 7.3 branch.Discussion----------[Validator] Add docs for bridge twig validator#20836This pull request adds documentation for the new Twig Validator constraint introduced in the Twig Bridge.Changes:- Added a page to document the Twig validator constraint, including:- Installation instructions for the symfony/twig-bridge package.- Usage of the constraint in Symfony applications.- Detailed options available, including `message` and `skipDeprecations`.- Updated `index.rst` to include the new page under the "**Topics**" section for the **Twig Bridge.**Related Issue:[Issue#20836 - Add docs for Twig validator](#20836)Related PR:[Symfony PR #58805](symfony/symfony#58805) – Introduced the Twig validator constraint in the Twig Bridge.Commits-------ac9e20c Add docs for bridge twig validator#20836
The default error message needs to be added in the translation files, so that it gets translated in the project. |
…bbuh)This PR was merged into the 6.4 branch.Discussion----------[Validator] add translations for the Twig constraint| Q | A| ------------- | ---| Branch? | 6.4| Bug fix? | no| New feature? | no| Deprecations? | no| Issues || License | MITadd missing translation file entries for#58805Commits-------f12ba2e add translations for the Twig constraint
Uh oh!
There was an error while loading.Please reload this page.
Add a new constraint for validating Twig content (inspired from Yaml constraint)