Skip to content

[ExpressionLanguage] Use script to generate regex #58342

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

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

HypeMC
Copy link
Contributor

@HypeMC HypeMC commented Sep 20, 2024

Q A
Branch? 7.2
Bug fix? maybe?
New feature? no
Deprecations? no
Issues -
License MIT

Fixes/uses the generate_operator_regex.php script for the regex added in #58052

@xabbuh
Copy link
Member

xabbuh commented Sep 20, 2024

Does it mean that we should add a test that would fail without these changes?

@HypeMC HypeMC force-pushed the fix-generate-operator-regex branch from abe4d2a to 3fc4b43 Compare September 20, 2024 20:32
@HypeMC
Copy link
Contributor Author

HypeMC commented Sep 20, 2024

Does it mean that we should add a test that would fail without these changes?

@xabbuh https://github.com/symfony/symfony/compare/abe4d2a51f664b40b2a74ce9be2ba0cb2aef715e..3fc4b432961f7931ba325b81373f955a9fbff0a1

self::assertStringContainsString(
$output,
file_get_contents((new \ReflectionClass(Lexer::class))->getFileName()),
\sprintf('You need to use "%s" to generate the operator regex.', $script),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
\sprintf('You need to use "%s" to generate the operator regex.', $script),
\sprintf('You need to run "%s" to generate the operator regex.', $script),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot Done

@xabbuh
Copy link
Member

xabbuh commented Sep 21, 2024

What I mean is that the operator regex must be used at one point to parse operators. If that regex wasn’t correct before, we need to find an expression that was not parsed properly and use that to build a test.

@HypeMC HypeMC force-pushed the fix-generate-operator-regex branch from 3fc4b43 to 3fc2f3b Compare September 21, 2024 13:13
@HypeMC
Copy link
Contributor Author

HypeMC commented Sep 21, 2024

What I mean is that the operator regex must be used at one point to parse operators. If that regex wasn’t correct before, we need to find an expression that was not parsed properly and use that to build a test.

@xabbuh It's not that the regex wasn't correct in the sense that it didn't work. There are tests that cover the functionality, and they passed. It's just that the regex generated by the script has a slightly different order and uses preg_quote, which escapes some additional characters.

@nicolas-grekas
Copy link
Member

Thank you @HypeMC.

@nicolas-grekas nicolas-grekas merged commit 8ad277a into symfony:7.2 Sep 28, 2024
9 of 10 checks passed
@HypeMC HypeMC deleted the fix-generate-operator-regex branch September 28, 2024 10:06
nicolas-grekas added a commit that referenced this pull request Oct 1, 2024
…rator (HypeMC)

This PR was merged into the 7.2 branch.

Discussion
----------

[ExpressionLanguage] Add support for logical `xor` operator

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | -
| License       | MIT

Contains #58342

Adds support for the logical `xor` operator.

Commits
-------

6e0a613 [ExpressionLanguage] Add support for logical `xor` operator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants