Skip to content
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

Modify Painless grammar to support right brackets as statement delimiters #29566

Merged
merged 3 commits into from
Apr 18, 2018

Conversation

jdconrad
Copy link
Contributor

This allows the grammar to determine when and what delimiters statements will use by splitting up the statements into regular statements and delimited statements, those that do not require a delimiter versus those that do. This allows consumers of the statements to determine what delimiters the statements will use so that in certain cases semicolons are not necessary like when there's a closing right bracket. A little bit of leniency for a little a bit of convenience.

This change removes the need for semicolon insertion in the lexer, simplifying the existing lexer quite a bit. It also ensures that there isn't a need to track semicolons being inserted into places that aren't necessary such as array initializers.

The reason for the large number of lines is due to grammar regeneration. The generated code can be ignored.

@jdconrad jdconrad added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v7.0.0 v6.3.0 labels Apr 17, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Neat! I like that you didn't have to change any tests. The change makes sense to me.

@jdconrad
Copy link
Contributor Author

@nik9000 Thanks for the review! Will commit once I can get the CI to pass.

@jdconrad
Copy link
Contributor Author

@elasticmachine Please test this.

@jdconrad jdconrad merged commit da9a689 into elastic:master Apr 18, 2018
jdconrad added a commit that referenced this pull request Apr 18, 2018
This allows the grammar to determine when and what delimiters statements will use by
splitting up the statements into regular statements and delimited statements, those that do
not require a delimiter versus those that do. This allows consumers of the statements to
determine what delimiters the statements will use so that in certain cases semicolons are
not necessary like when there's a closing right bracket.

This change removes the need for semicolon insertion in the lexer, simplifying the existing
lexer quite a bit. It also ensures that there isn't a need to track semicolons being inserted
into places that aren't necessary such as array initializers.
@jdconrad jdconrad deleted the semicolon branch August 28, 2018 16:32
jdconrad added a commit that referenced this pull request Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
jdconrad added a commit that referenced this pull request Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
jdconrad added a commit that referenced this pull request Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
jdconrad added a commit that referenced this pull request Aug 28, 2018
Trailers (statements following something like an if statement) that don't use brackets currently require a semicolon even if they're the last statement. This is a regression caused by (#29566) and noted by (#33193). This change fixes the regression and adds a test for the broken case.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement v6.3.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants