[ES|QL] Comment parsing and pretty-printing #192173
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vadimkibana
changed the title
|
@elasticmachine merge upstream |
stratoula
reviewed
Sep 25, 2024
| const { root } = parse(src, { withFormatting: true }); | ||
| const text = BasicPrettyPrinter.print(root); | ||
|
|
||
| // console.log(JSON.stringify(ast, null, 2)); |
There was a problem hiding this comment.
Is it on purpose there Vadim?
There was a problem hiding this comment.
I left it on purpose, as it is handy when writing these tests. But I can, of course, remove if it does not fit the style quide.
There was a problem hiding this comment.
Nah, just asking. I am fine with it
@stratoula That text based editor is using the basic pretty printer, let's update it but in a separate PR. Also, in your example you are pretty printing a single line comment (which always ends with a line break) to a single line. The query cannot possibly have single line comments and be formatted to a single line. Maybe worth discussing in a team sync what we want to do in this situation. |
True I was expecting to change into a single line comment but maybe is an edge case. About :
Sure, lets do it on a follow up PR 👍 |
stratoula
approved these changes
Sep 25, 2024
There was a problem hiding this comment.
I would appreciate another round of review here from @drewdaemon but for me LGTM! Amazing work as always
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Async chunks
Public APIs missing exports
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
References to deprecated APIs
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
drewdaemon
approved these changes
Sep 25, 2024
There was a problem hiding this comment.
Amazing work @vadimkibana ! I'm so glad that we'll now have a way to stop making autocomplete suggestions inside of comments 🤦♂️
I did notice a very minor quirk where the editor doesn't extend as far as I expect it to.
Screen.Recording.2024-09-25.at.3.32.44.PM.mov
stratoula
added
backport:prev-minor
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
pushed a commit
to kibanamachine/kibana
that referenced
this pull request
Sep 26, 2024
## Summary
TL;DR
- Adds ability to parse out comments from source to AST.
- Adds ability for every AST node to have *decoration*—comments,
which can be attached from left, top, and right from the node.
- Implements routine which attached comments to AST nodes.
- In `BasicPrettyPrinter` adds support only for *left* and *right*
comment printing, as the basic printer prints only on one line.
- In `WrappingPrettyPrinter` adds support for all comment printing for
all AST nodes.
- Introduces a `Query` object and `query` AST node, which represent
thole query—the root node, list of commands.
- The ES|QL AST example plugin now displays the pretty-printed text
version.
### Comments
This PR introduced an optional `formatting` field for all AST nodes. In
the `formatting` field one can specify comment decorations from
different sides of a node.
When parsing, once can now specify the `{ withComments: true }` option,
which will collect all comments from the source while parsing using the
`collectDecorations` routine. It will then also call the
`attachDecorations`, which walks the AST and assigns each comment to
some AST node.
Further, traversal and pretty-print API have been updated to work with
comments:
- The `Walker` has been updated to be able to walk all comments from the
AST.
- The `BasicPrettyPrinter` adds support only for *left* and *right*
inline comment printing, as the basic printer prints only on one line.
- The `WrappingPrettyPrinter` adds support for all comment printing for
all AST nodes. It switches to line-break printing mode if it detects
there are comments with line breaks (those could be multi-line comments,
or single line comments—single line comments are always followed
by a line break). It also correctly inserts punctuation, when an AST
node is surrounded by comments.
### Parsing utils
All parsing utils have been moved to the `/parser` sub-folder.
Files in the `/parser` folder have been renamed as per Kibana convention
to reflect what is inside the file. For example, the `EsqlErrorListener`
class is in a file named `esql_error_listener.ts`.
A `Query` class and `ESQLAstQueryExpression` AST nodes have been
introduced. They represent the result of a full query parse. (Before
that, the AST root was just an array of command nodes, now the AST root
is represented by the `ESQLAstQueryExpression` node.)
### Builder
I have started the implementation of the `Builder` static class in the
`/builder` folder. It is simply a collection of stateless AST node
factories—functions which construct AST nodes.
Some of the `Builder` methods are already used by the parser, more will
follow. We will also use the `Builder` in upcoming [*Mutation
API*](elastic#191812).
### ES|QL Example Plugin
This PR sets up Storybook and implements few Storybook stories for the
ES|QL AST example plugin, run it with:
```
yarn storybook esql_ast_inspector
```
This PR updates the *ES|QL AST Explorer* example plugin. Start Kibana
with example plugins enabled:
```
yarn start --run-examples
```
And navigate to
[`/app/esql_ast_inspector`](http://localhost:5601/app/esql_ast_inspector)
to see the new example plugin UI.
### Checklist
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
### For maintainers
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
---------
Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>
(cherry picked from commit 2217337)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
kibanamachine
added a commit
that referenced
this pull request
Sep 26, 2024
# Backport This will backport the following commits from `main` to `8.x`: - [[ES|QL] Comment parsing and pretty-printing (#192173)](#192173) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Vadim Kibana","email":"82822460+vadimkibana@users.noreply.github.com"},"sourceCommit":{"committedDate":"2024-09-26T10:34:38Z","message":"[ES|QL] Comment parsing and pretty-printing (#192173)\n\n## Summary\r\n\r\nTL;DR\r\n\r\n- Adds ability to parse out comments from source to AST.\r\n- Adds ability for every AST node to have *decoration*—comments,\r\nwhich can be attached from left, top, and right from the node.\r\n- Implements routine which attached comments to AST nodes.\r\n- In `BasicPrettyPrinter` adds support only for *left* and *right*\r\ncomment printing, as the basic printer prints only on one line.\r\n- In `WrappingPrettyPrinter` adds support for all comment printing for\r\nall AST nodes.\r\n- Introduces a `Query` object and `query` AST node, which represent\r\nthole query—the root node, list of commands.\r\n- The ES|QL AST example plugin now displays the pretty-printed text\r\nversion.\r\n\r\n\r\n### Comments\r\n\r\nThis PR introduced an optional `formatting` field for all AST nodes. In\r\nthe `formatting` field one can specify comment decorations from\r\ndifferent sides of a node.\r\n\r\nWhen parsing, once can now specify the `{ withComments: true }` option,\r\nwhich will collect all comments from the source while parsing using the\r\n`collectDecorations` routine. It will then also call the\r\n`attachDecorations`, which walks the AST and assigns each comment to\r\nsome AST node.\r\n\r\nFurther, traversal and pretty-print API have been updated to work with\r\ncomments:\r\n\r\n- The `Walker` has been updated to be able to walk all comments from the\r\nAST.\r\n- The `BasicPrettyPrinter` adds support only for *left* and *right*\r\ninline comment printing, as the basic printer prints only on one line.\r\n- The `WrappingPrettyPrinter` adds support for all comment printing for\r\nall AST nodes. It switches to line-break printing mode if it detects\r\nthere are comments with line breaks (those could be multi-line comments,\r\nor single line comments—single line comments are always followed\r\nby a line break). It also correctly inserts punctuation, when an AST\r\nnode is surrounded by comments.\r\n\r\n\r\n### Parsing utils\r\n\r\nAll parsing utils have been moved to the `/parser` sub-folder.\r\n\r\nFiles in the `/parser` folder have been renamed as per Kibana convention\r\nto reflect what is inside the file. For example, the `EsqlErrorListener`\r\nclass is in a file named `esql_error_listener.ts`.\r\n\r\nA `Query` class and `ESQLAstQueryExpression` AST nodes have been\r\nintroduced. They represent the result of a full query parse. (Before\r\nthat, the AST root was just an array of command nodes, now the AST root\r\nis represented by the `ESQLAstQueryExpression` node.)\r\n\r\n\r\n### Builder\r\n\r\nI have started the implementation of the `Builder` static class in the\r\n`/builder` folder. It is simply a collection of stateless AST node\r\nfactories—functions which construct AST nodes.\r\n\r\nSome of the `Builder` methods are already used by the parser, more will\r\nfollow. We will also use the `Builder` in upcoming [*Mutation\r\nAPI*](https://github.com/elastic/kibana/issues/191812).\r\n\r\n\r\n### ES|QL Example Plugin\r\n\r\nThis PR sets up Storybook and implements few Storybook stories for the\r\nES|QL AST example plugin, run it with:\r\n\r\n```\r\nyarn storybook esql_ast_inspector\r\n```\r\n\r\nThis PR updates the *ES|QL AST Explorer* example plugin. Start Kibana\r\nwith example plugins enabled:\r\n\r\n```\r\nyarn start --run-examples\r\n```\r\n\r\nAnd navigate to\r\n[`/app/esql_ast_inspector`](http://localhost:5601/app/esql_ast_inspector)\r\nto see the new example plugin UI.\r\n\r\n\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [x] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>\r\nCo-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>","sha":"2217337c5d91340ba67e0bedaab0762502518993","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["review","release_note:skip","v9.0.0","backport:prev-minor","Feature:ES|QL","Team:ESQL","v8.16.0"],"title":"[ES|QL] Comment parsing and pretty-printing","number":192173,"url":"https://github.com/elastic/kibana/pull/192173","mergeCommit":{"message":"[ES|QL] Comment parsing and pretty-printing (#192173)\n\n## Summary\r\n\r\nTL;DR\r\n\r\n- Adds ability to parse out comments from source to AST.\r\n- Adds ability for every AST node to have *decoration*—comments,\r\nwhich can be attached from left, top, and right from the node.\r\n- Implements routine which attached comments to AST nodes.\r\n- In `BasicPrettyPrinter` adds support only for *left* and *right*\r\ncomment printing, as the basic printer prints only on one line.\r\n- In `WrappingPrettyPrinter` adds support for all comment printing for\r\nall AST nodes.\r\n- Introduces a `Query` object and `query` AST node, which represent\r\nthole query—the root node, list of commands.\r\n- The ES|QL AST example plugin now displays the pretty-printed text\r\nversion.\r\n\r\n\r\n### Comments\r\n\r\nThis PR introduced an optional `formatting` field for all AST nodes. In\r\nthe `formatting` field one can specify comment decorations from\r\ndifferent sides of a node.\r\n\r\nWhen parsing, once can now specify the `{ withComments: true }` option,\r\nwhich will collect all comments from the source while parsing using the\r\n`collectDecorations` routine. It will then also call the\r\n`attachDecorations`, which walks the AST and assigns each comment to\r\nsome AST node.\r\n\r\nFurther, traversal and pretty-print API have been updated to work with\r\ncomments:\r\n\r\n- The `Walker` has been updated to be able to walk all comments from the\r\nAST.\r\n- The `BasicPrettyPrinter` adds support only for *left* and *right*\r\ninline comment printing, as the basic printer prints only on one line.\r\n- The `WrappingPrettyPrinter` adds support for all comment printing for\r\nall AST nodes. It switches to line-break printing mode if it detects\r\nthere are comments with line breaks (those could be multi-line comments,\r\nor single line comments—single line comments are always followed\r\nby a line break). It also correctly inserts punctuation, when an AST\r\nnode is surrounded by comments.\r\n\r\n\r\n### Parsing utils\r\n\r\nAll parsing utils have been moved to the `/parser` sub-folder.\r\n\r\nFiles in the `/parser` folder have been renamed as per Kibana convention\r\nto reflect what is inside the file. For example, the `EsqlErrorListener`\r\nclass is in a file named `esql_error_listener.ts`.\r\n\r\nA `Query` class and `ESQLAstQueryExpression` AST nodes have been\r\nintroduced. They represent the result of a full query parse. (Before\r\nthat, the AST root was just an array of command nodes, now the AST root\r\nis represented by the `ESQLAstQueryExpression` node.)\r\n\r\n\r\n### Builder\r\n\r\nI have started the implementation of the `Builder` static class in the\r\n`/builder` folder. It is simply a collection of stateless AST node\r\nfactories—functions which construct AST nodes.\r\n\r\nSome of the `Builder` methods are already used by the parser, more will\r\nfollow. We will also use the `Builder` in upcoming [*Mutation\r\nAPI*](https://github.com/elastic/kibana/issues/191812).\r\n\r\n\r\n### ES|QL Example Plugin\r\n\r\nThis PR sets up Storybook and implements few Storybook stories for the\r\nES|QL AST example plugin, run it with:\r\n\r\n```\r\nyarn storybook esql_ast_inspector\r\n```\r\n\r\nThis PR updates the *ES|QL AST Explorer* example plugin. Start Kibana\r\nwith example plugins enabled:\r\n\r\n```\r\nyarn start --run-examples\r\n```\r\n\r\nAnd navigate to\r\n[`/app/esql_ast_inspector`](http://localhost:5601/app/esql_ast_inspector)\r\nto see the new example plugin UI.\r\n\r\n\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [x] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>\r\nCo-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>","sha":"2217337c5d91340ba67e0bedaab0762502518993"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192173","number":192173,"mergeCommit":{"message":"[ES|QL] Comment parsing and pretty-printing (#192173)\n\n## Summary\r\n\r\nTL;DR\r\n\r\n- Adds ability to parse out comments from source to AST.\r\n- Adds ability for every AST node to have *decoration*—comments,\r\nwhich can be attached from left, top, and right from the node.\r\n- Implements routine which attached comments to AST nodes.\r\n- In `BasicPrettyPrinter` adds support only for *left* and *right*\r\ncomment printing, as the basic printer prints only on one line.\r\n- In `WrappingPrettyPrinter` adds support for all comment printing for\r\nall AST nodes.\r\n- Introduces a `Query` object and `query` AST node, which represent\r\nthole query—the root node, list of commands.\r\n- The ES|QL AST example plugin now displays the pretty-printed text\r\nversion.\r\n\r\n\r\n### Comments\r\n\r\nThis PR introduced an optional `formatting` field for all AST nodes. In\r\nthe `formatting` field one can specify comment decorations from\r\ndifferent sides of a node.\r\n\r\nWhen parsing, once can now specify the `{ withComments: true }` option,\r\nwhich will collect all comments from the source while parsing using the\r\n`collectDecorations` routine. It will then also call the\r\n`attachDecorations`, which walks the AST and assigns each comment to\r\nsome AST node.\r\n\r\nFurther, traversal and pretty-print API have been updated to work with\r\ncomments:\r\n\r\n- The `Walker` has been updated to be able to walk all comments from the\r\nAST.\r\n- The `BasicPrettyPrinter` adds support only for *left* and *right*\r\ninline comment printing, as the basic printer prints only on one line.\r\n- The `WrappingPrettyPrinter` adds support for all comment printing for\r\nall AST nodes. It switches to line-break printing mode if it detects\r\nthere are comments with line breaks (those could be multi-line comments,\r\nor single line comments—single line comments are always followed\r\nby a line break). It also correctly inserts punctuation, when an AST\r\nnode is surrounded by comments.\r\n\r\n\r\n### Parsing utils\r\n\r\nAll parsing utils have been moved to the `/parser` sub-folder.\r\n\r\nFiles in the `/parser` folder have been renamed as per Kibana convention\r\nto reflect what is inside the file. For example, the `EsqlErrorListener`\r\nclass is in a file named `esql_error_listener.ts`.\r\n\r\nA `Query` class and `ESQLAstQueryExpression` AST nodes have been\r\nintroduced. They represent the result of a full query parse. (Before\r\nthat, the AST root was just an array of command nodes, now the AST root\r\nis represented by the `ESQLAstQueryExpression` node.)\r\n\r\n\r\n### Builder\r\n\r\nI have started the implementation of the `Builder` static class in the\r\n`/builder` folder. It is simply a collection of stateless AST node\r\nfactories—functions which construct AST nodes.\r\n\r\nSome of the `Builder` methods are already used by the parser, more will\r\nfollow. We will also use the `Builder` in upcoming [*Mutation\r\nAPI*](https://github.com/elastic/kibana/issues/191812).\r\n\r\n\r\n### ES|QL Example Plugin\r\n\r\nThis PR sets up Storybook and implements few Storybook stories for the\r\nES|QL AST example plugin, run it with:\r\n\r\n```\r\nyarn storybook esql_ast_inspector\r\n```\r\n\r\nThis PR updates the *ES|QL AST Explorer* example plugin. Start Kibana\r\nwith example plugins enabled:\r\n\r\n```\r\nyarn start --run-examples\r\n```\r\n\r\nAnd navigate to\r\n[`/app/esql_ast_inspector`](http://localhost:5601/app/esql_ast_inspector)\r\nto see the new example plugin UI.\r\n\r\n\r\n\r\n\r\n\r\n\r\n### Checklist\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios\r\n\r\n\r\n### For maintainers\r\n\r\n- [x] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>\r\nCo-authored-by: Stratoula Kalafateli <efstratia.kalafateli@elastic.co>","sha":"2217337c5d91340ba67e0bedaab0762502518993"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Vadim Kibana <82822460+vadimkibana@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Partially addresses #182257
TL;DR
BasicPrettyPrinteradds support only for left and right comment printing, as the basic printer prints only on one line.WrappingPrettyPrinteradds support for all comment printing for all AST nodes.Queryobject andqueryAST node, which represent thole query—the root node, list of commands.Comments
This PR introduced an optional
formattingfield for all AST nodes. In theformattingfield one can specify comment decorations from different sides of a node.When parsing, once can now specify the
{ withComments: true }option, which will collect all comments from the source while parsing using thecollectDecorationsroutine. It will then also call theattachDecorations, which walks the AST and assigns each comment to some AST node.Further, traversal and pretty-print API have been updated to work with comments:
Walkerhas been updated to be able to walk all comments from the AST.BasicPrettyPrinteradds support only for left and right inline comment printing, as the basic printer prints only on one line.WrappingPrettyPrinteradds support for all comment printing for all AST nodes. It switches to line-break printing mode if it detects there are comments with line breaks (those could be multi-line comments, or single line comments—single line comments are always followed by a line break). It also correctly inserts punctuation, when an AST node is surrounded by comments.Parsing utils
All parsing utils have been moved to the
/parsersub-folder.Files in the
/parserfolder have been renamed as per Kibana convention to reflect what is inside the file. For example, theEsqlErrorListenerclass is in a file namedesql_error_listener.ts.A
Queryclass andESQLAstQueryExpressionAST nodes have been introduced. They represent the result of a full query parse. (Before that, the AST root was just an array of command nodes, now the AST root is represented by theESQLAstQueryExpressionnode.)Builder
I have started the implementation of the
Builderstatic class in the/builderfolder. It is simply a collection of stateless AST node factories—functions which construct AST nodes.Some of the
Buildermethods are already used by the parser, more will follow. We will also use theBuilderin upcoming Mutation API.ES|QL Example Plugin
This PR sets up Storybook and implements few Storybook stories for the ES|QL AST example plugin, run it with:
This PR updates the ES|QL AST Explorer example plugin. Start Kibana with example plugins enabled:
And navigate to
/app/esql_ast_inspectorto see the new example plugin UI.Checklist
For maintainers