Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[ExpressionLanguage] Added expression language syntax validator#35849

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

Merged

Conversation

Andrej-in-ua
Copy link
Contributor

@Andrej-in-uaAndrej-in-ua commentedFeb 24, 2020•
edited
Loading

QA
Branch?master
Bug fix?no
New feature?yes
Deprecations?no
Tickets#35700
LicenseMIT
Doc PRN/A

Proposal implementation#35700

The current solution is a compromise between support complexity and cleanliness.

I tried different solutions to the issue. A beautiful solution was obtained only with full duplication of the parser code. That is unacceptable because parser complexity is quite high.

The main problem in this solution is that nodes instances are created which are then not used. I do not think that linter can be a bottleneck and will greatly affect performance. If this is corrected, the parser code becomes a bunch of if's.

JFI: I did not added parsing without variable names, because this breaks caching and potential location for vulnerabilities.

@Koc
Copy link
Contributor

Koc commentedFeb 24, 2020

ref#16323

Andrej-in-ua reacted with thumbs up emoji

@nicolas-grekasnicolas-grekas added this to thenext milestoneFeb 26, 2020
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

lint()?

Andrej-in-ua reacted with thumbs up emoji
@Andrej-in-uaAndrej-in-uaforce-pushed theexpression-language-validator branch 2 times, most recently from7a7556d to42e6a4eCompareFebruary 28, 2020 13:55
@Andrej-in-uaAndrej-in-ua marked this pull request as ready for reviewFebruary 28, 2020 15:01
@nicolas-grekas
Copy link
Member

Friendly ping@Andrej-in-ua

@Andrej-in-ua
Copy link
ContributorAuthor

Sorry for the delay, a slight change in priorities at work :(
Tomorrow or the day after tomorrow I will push changes.
Thanks!

@Andrej-in-uaAndrej-in-uaforce-pushed theexpression-language-validator branch from1ea6e34 to8c88fb3CompareApril 1, 2020 15:18
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

I think this is good, just some minor comments on my side.
Please also add a line in the CHANGELOG of the component.

Andrej-in-ua reacted with thumbs up emoji
@fabpotfabpotforce-pushed theexpression-language-validator branch from4e126ab toa5cd965CompareMay 5, 2020 05:59
@fabpot
Copy link
Member

Thank you@Andrej-in-ua.

Andrej-in-ua reacted with hooray emoji

@fabpotfabpot merged commit3d30ff7 intosymfony:masterMay 5, 2020
@Andrej-in-uaAndrej-in-ua deleted the expression-language-validator branchMay 5, 2020 13:19
@fabpotfabpot mentioned this pull requestMay 5, 2020
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.

5 participants
@Andrej-in-ua@Koc@nicolas-grekas@fabpot@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp