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

Update painless antlr grammar for fields API $-syntax #125818

Merged
merged 4 commits into from
Feb 17, 2022

Conversation

stu-elastic
Copy link
Contributor

@stu-elastic stu-elastic commented Feb 16, 2022

$('fieldname', <default>) was added in elasticsearch/80518.

This change updates the ANTLR grammar so that the syntax check
passes.

painless_lexer.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessLexer.g4
painless_parser.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessParser.g4

Generated by running npm run build:antlr4ts from packages/kbn-monaco

Related to #84695

$('fieldname', <default>) was added in elasticsearch/elastic#80518.

This updated the ANTLR grammar so that the syntax check
passes.

painless_lexer.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessLexer.g4
painless_parser.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessParser.g4

Generated by running `npm run build:antlr4ts` from `packages/kbn-monaco`

Related to elastic#84695
@stu-elastic stu-elastic added the painless painless label Feb 16, 2022
@stu-elastic stu-elastic requested a review from a team as a code owner February 16, 2022 16:18
@stu-elastic
Copy link
Contributor Author

Before:
image

After:
image

@alisonelizabeth alisonelizabeth added Feature:Painless Lab Dev tool for learning Painless Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more labels Feb 16, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/platform-deployment-management (Team:Deployment Management)

@alisonelizabeth
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @stu-elastic! I think I found a regression, and left a couple comments about it. Let me know what you think. Thanks!

@@ -184,13 +185,9 @@ export class painless_lexer extends Lexer {
// @Override
public sempred(_localctx: RuleContext, ruleIndex: number, predIndex: number): boolean {
switch (ruleIndex) {
// DO NOT CHANGE
// This is a manual fix to handle slashes appropriately
case 31:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to add this comment back in, and change it to 32 based on the addition of DOLLAR. Otherwise, the slash is broken again.

Long term, it would be nice to figure out the root of this problem 😄 (briefly explained in the readme).

Painless-Lab-Dev-Tools-Elastic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice find. Updated.
image

case 31:
return this.DIV_sempred(_localctx, predIndex);

// DO NOT CHANGE
// This is a manual fix to handle regexes appropriately
case 77:
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly to above - I think we need to add this comment back in, and change it to 78 based on the addition of DOLLAR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
kbnUiSharedDeps-srcJs 3.8MB 3.8MB +568.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Latest changes LGTM. Thanks for fixing this!

@stu-elastic stu-elastic merged commit ea6be3c into elastic:main Feb 17, 2022
stu-elastic added a commit to stu-elastic/kibana that referenced this pull request Feb 17, 2022
$('fieldname', <default>) was added in elasticsearch/elastic#80518.

This updated the ANTLR grammar so that the syntax check
passes.

painless_lexer.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessLexer.g4
painless_parser.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessParser.g4

Generated by running `npm run build:antlr4ts` from `packages/kbn-monaco`

(cherry picked from commit ea6be3c)
stu-elastic added a commit that referenced this pull request Feb 17, 2022
)

$('fieldname', <default>) was added in elasticsearch/#80518.

This updated the ANTLR grammar so that the syntax check
passes.

painless_lexer.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessLexer.g4
painless_parser.g4 adapted from elasticsearch/modules/lang-painless/src/main/antlr/PainlessParser.g4

Generated by running `npm run build:antlr4ts` from `packages/kbn-monaco`

(cherry picked from commit ea6be3c)
@KOTungseth KOTungseth added the Feature:Data Views Data Views code and UI - index patterns before 8.0 label Mar 2, 2022
@tylersmalley tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:cloud-deploy Create or update a Cloud deployment Feature:Data Views Data Views code and UI - index patterns before 8.0 Feature:Painless Lab Dev tool for learning Painless Feature:Runtime Fields painless painless release_note:fix Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v8.1.0 v8.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants