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

[ES|QL] Implements wrapping pretty-printer #190589

Merged
merged 3 commits into from
Aug 20, 2024

Conversation

vadimkibana
Copy link
Contributor

@vadimkibana vadimkibana commented Aug 15, 2024

Summary

Partially addresses #182257

  • Improves the basic "one-line" printer BasicPrettyPrinter, notable changes:
    • It is now possible better specify if query keywords should be uppercased
    • Better formatting columns names, adds backquotes when escaping needed: `name👍`
    • Wraps cast expressions into brackets, where needed: (1 + 2)::string instead of 1 + 2::string
  • Adds initial implementations of the more complex WrappingPrettyPrinter.
    • "Initial implementation" because it probably covers 80-90% of the cases, some follow up will be needed.
    • The WrappingPrettyPrinter formats the query like Prettier, it tries to format AST nodes horizontally as lists, but based on various conditions breaks the lines and indents them.

Cases handled by the WrappingPrettyPrinter

Below are examples of some of the cases handled by the WrappingPrettyPrinter. (See test files for many more cases.)

Short queries

Queries with less than 4 commands and if they do not require wrapping are formatted to a single line.

Source:

FROM index | WHERE a == 123

Result:

FROM index | WHERE a == 123
Argument wrapping

Command arguments are wrapped (at wrapping threshold, defaults to 80).

Source:

FROM index, another_index, yet_another_index, on-more-index, last_index, very_last_index, ok_this_is_the_last_index

Result:

FROM index, another_index, yet_another_index, on-more-index, last_index,
     very_last_index, ok_this_is_the_last_index
Argument breaking

Command argument combinations which result into a single argument occupying a whole line (due to that argument being long, or because the surrounding argument combination results into such a case), except the last argument, results into the argument list being broken by line.

Source:

FROM xxxxxxxxxx, yyyyyyyyyyy, zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz,
  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,  // <------------ this one
  bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb, ccccccc, ggggggggg

Result:

FROM
  xxxxxxxxxx,
  yyyyyyyyyyy,
  zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz,
  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
  bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,
  ccccccc,
  ggggggggg
Binary expression chain vertical flattening

Binary expressions of the same precedence are vertically flattened, if wrapping is required. Same as it is done by Prettier, where there is an indentation after the first line to allow for different precedence expressions.

All expressions have the same precedence

Source:

FROM index
| STATS super_function_name(11111111111111.111 + 11111111111111.111 + 11111111111111.111 + 11111111111111.111 + 11111111111111.111))

Result:

FROM index
  | STATS
      SUPER_FUNCTION_NAME(
        11111111111111.111 +
          11111111111111.111 +
          11111111111111.111 +
          11111111111111.111 +
          11111111111111.111)
Expressions with additive and multiplicative precedence mixed

Source:

FROM index
| STATS super_function_name(11111111111111.111 + 3333333333333.3333 * 3333333333333.3333 * 3333333333333.3333 * 3333333333333.3333 + 11111111111111.111 + 11111111111111.111 + 11111111111111.111 + 11111111111111.111))

Result:

FROM index
  | STATS
      SUPER_FUNCTION_NAME(
        11111111111111.111 +
          3333333333333.3335 *
            3333333333333.3335 *
            3333333333333.3335 *
            3333333333333.3335 +
          11111111111111.111 +
          11111111111111.111 +
          11111111111111.111 +
          11111111111111.111)

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@vadimkibana vadimkibana requested a review from a team as a code owner August 15, 2024 10:32
@vadimkibana vadimkibana added review release_note:skip Skip the PR/issue when compiling release notes Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Feature:ES|QL ES|QL related features in Kibana Team:ESQL ES|QL related features in Kibana v8.16.0 labels Aug 15, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-esql (Team:ESQL)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@vadimkibana
Copy link
Contributor Author

@elasticmachine merge upstream

@vadimkibana vadimkibana changed the title [ES|QL] Implements wrapping pretty printer [ES|QL] Implements wrapping pretty-printer Aug 19, 2024
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
@kbn/esql-ast 1 2 +1

Total ESLint disabled count

id before after diff
@kbn/esql-ast 2 3 +1

History

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

Copy link
Contributor

@drewdaemon drewdaemon left a comment

Choose a reason for hiding this comment

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

Beautiful work

@vadimkibana vadimkibana merged commit 8316cbf into elastic:main Aug 20, 2024
20 checks passed
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:ES|QL ES|QL related features in Kibana release_note:skip Skip the PR/issue when compiling release notes review Team:DataDiscovery Discover, search (e.g. data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. Team:ESQL ES|QL related features in Kibana v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants