From 7cc3eae35b9ed13ac3e008c577ea7342778bf874 Mon Sep 17 00:00:00 2001 From: Stratoula Kalafateli Date: Tue, 10 Sep 2024 11:30:38 +0200 Subject: [PATCH] [ES|QL] Prettiffying the query (#191269) ## Summary The wrap by pipes CTA doesnt work correctly when pipes are being used inside functions or in a comment in an ES|QL query. This PR fixes this bug by using the prettifying api. I was thinking to change the tooltip text to mention that it doesnt only wrap by pipes but also prettifies but I decided against it and kept the tooltip as it is for 2 reasons: - this is the most important thing that it does - prettifying means nothing for the majority of our users ![meow](https://github.com/user-attachments/assets/c2189218-4367-4d4a-9f9e-2d6f88f731d2) ### Checklist - [ ] [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 --------- Co-authored-by: Elastic Machine --- packages/kbn-esql-ast/index.ts | 2 + packages/kbn-esql-utils/index.ts | 2 + packages/kbn-esql-utils/src/index.ts | 2 + .../src/utils/query_parsing_helpers.test.ts | 39 +++++++++++++++++++ .../src/utils/query_parsing_helpers.ts | 14 ++++++- .../editor_footer/query_wrap_component.tsx | 16 ++++---- .../kbn-text-based-editor/src/helpers.test.ts | 31 +-------------- packages/kbn-text-based-editor/src/helpers.ts | 8 ---- 8 files changed, 66 insertions(+), 48 deletions(-) diff --git a/packages/kbn-esql-ast/index.ts b/packages/kbn-esql-ast/index.ts index b4cff8aaa9a450..fe7c8bcaab23fb 100644 --- a/packages/kbn-esql-ast/index.ts +++ b/packages/kbn-esql-ast/index.ts @@ -42,3 +42,5 @@ export { getAstAndSyntaxErrors } from './src/ast_parser'; export { ESQLErrorListener } from './src/antlr_error_listener'; export { Walker, type WalkerOptions, walk } from './src/walker'; + +export { BasicPrettyPrinter } from './src/pretty_print/basic_pretty_printer'; diff --git a/packages/kbn-esql-utils/index.ts b/packages/kbn-esql-utils/index.ts index 042446a8c9dbba..dce93150f0facd 100644 --- a/packages/kbn-esql-utils/index.ts +++ b/packages/kbn-esql-utils/index.ts @@ -25,6 +25,8 @@ export { getTimeFieldFromESQLQuery, getStartEndParams, hasStartEndParams, + prettifyQuery, + isQueryWrappedByPipes, retieveMetadataColumns, TextBasedLanguages, } from './src'; diff --git a/packages/kbn-esql-utils/src/index.ts b/packages/kbn-esql-utils/src/index.ts index 53ecb30e6b8446..c214574099e460 100644 --- a/packages/kbn-esql-utils/src/index.ts +++ b/packages/kbn-esql-utils/src/index.ts @@ -17,6 +17,8 @@ export { removeDropCommandsFromESQLQuery, hasTransformationalCommand, getTimeFieldFromESQLQuery, + prettifyQuery, + isQueryWrappedByPipes, retieveMetadataColumns, } from './utils/query_parsing_helpers'; export { appendToESQLQuery, appendWhereClauseToESQLQuery } from './utils/append_to_query'; diff --git a/packages/kbn-esql-utils/src/utils/query_parsing_helpers.test.ts b/packages/kbn-esql-utils/src/utils/query_parsing_helpers.test.ts index 0c14568903ce5f..3c56856d1abf6b 100644 --- a/packages/kbn-esql-utils/src/utils/query_parsing_helpers.test.ts +++ b/packages/kbn-esql-utils/src/utils/query_parsing_helpers.test.ts @@ -13,6 +13,8 @@ import { removeDropCommandsFromESQLQuery, hasTransformationalCommand, getTimeFieldFromESQLQuery, + prettifyQuery, + isQueryWrappedByPipes, retieveMetadataColumns, } from './query_parsing_helpers'; @@ -178,6 +180,43 @@ describe('esql query helpers', () => { }); }); + describe('prettifyQuery', function () { + it('should return the code wrapped', function () { + const code = prettifyQuery('FROM index1 | KEEP field1, field2 | SORT field1', false); + expect(code).toEqual('FROM index1\n | KEEP field1, field2\n | SORT field1'); + }); + + it('should return the code unwrapped', function () { + const code = prettifyQuery('FROM index1 \n| KEEP field1, field2 \n| SORT field1', true); + expect(code).toEqual('FROM index1 | KEEP field1, field2 | SORT field1'); + }); + + it('should return the code unwrapped and trimmed', function () { + const code = prettifyQuery( + 'FROM index1 \n| KEEP field1, field2 \n| SORT field1', + true + ); + expect(code).toEqual('FROM index1 | KEEP field1, field2 | SORT field1'); + }); + }); + + describe('isQueryWrappedByPipes', function () { + it('should return false if the query is not wrapped', function () { + const flag = isQueryWrappedByPipes('FROM index1 | KEEP field1, field2 | SORT field1'); + expect(flag).toBeFalsy(); + }); + + it('should return true if the query is wrapped', function () { + const flag = isQueryWrappedByPipes('FROM index1 /n| KEEP field1, field2 /n| SORT field1'); + expect(flag).toBeTruthy(); + }); + + it('should return true if the query is wrapped and prettified', function () { + const flag = isQueryWrappedByPipes('FROM index1 /n | KEEP field1, field2 /n | SORT field1'); + expect(flag).toBeTruthy(); + }); + }); + describe('retieveMetadataColumns', () => { it('should return metadata columns if they exist', () => { expect(retieveMetadataColumns('from a metadata _id, _ignored | eval b = 1')).toStrictEqual([ diff --git a/packages/kbn-esql-utils/src/utils/query_parsing_helpers.ts b/packages/kbn-esql-utils/src/utils/query_parsing_helpers.ts index 0af72d17c0bbea..e0c67db41864b3 100644 --- a/packages/kbn-esql-utils/src/utils/query_parsing_helpers.ts +++ b/packages/kbn-esql-utils/src/utils/query_parsing_helpers.ts @@ -6,6 +6,7 @@ * your election, the "Elastic License 2.0", the "GNU Affero General Public * License v3.0 only", or the "Server Side Public License, v 1". */ +import { getAstAndSyntaxErrors, Walker, walk, BasicPrettyPrinter } from '@kbn/esql-ast'; import type { ESQLSource, @@ -14,7 +15,6 @@ import type { ESQLSingleAstItem, ESQLCommandOption, } from '@kbn/esql-ast'; -import { getAstAndSyntaxErrors, Walker, walk } from '@kbn/esql-ast'; const DEFAULT_ESQL_LIMIT = 1000; @@ -114,6 +114,18 @@ export const getTimeFieldFromESQLQuery = (esql: string) => { return column?.name; }; +export const isQueryWrappedByPipes = (query: string): boolean => { + const { ast } = getAstAndSyntaxErrors(query); + const numberOfCommands = ast.length; + const pipesWithNewLine = query.split('\n |'); + return numberOfCommands === pipesWithNewLine?.length; +}; + +export const prettifyQuery = (query: string, isWrapped: boolean): string => { + const { ast } = getAstAndSyntaxErrors(query); + return BasicPrettyPrinter.print(ast, { multiline: !isWrapped }); +}; + export const retieveMetadataColumns = (esql: string): string[] => { const { ast } = getAstAndSyntaxErrors(esql); const options: ESQLCommandOption[] = []; diff --git a/packages/kbn-text-based-editor/src/editor_footer/query_wrap_component.tsx b/packages/kbn-text-based-editor/src/editor_footer/query_wrap_component.tsx index cc6b62c7357188..8abcc475a9390f 100644 --- a/packages/kbn-text-based-editor/src/editor_footer/query_wrap_component.tsx +++ b/packages/kbn-text-based-editor/src/editor_footer/query_wrap_component.tsx @@ -10,7 +10,7 @@ import React, { useMemo } from 'react'; import { i18n } from '@kbn/i18n'; import { EuiFlexItem, EuiToolTip, EuiButtonIcon } from '@elastic/eui'; -import { getWrappedInPipesCode } from '../helpers'; +import { prettifyQuery, isQueryWrappedByPipes } from '@kbn/esql-utils'; export function QueryWrapComponent({ code, @@ -19,10 +19,8 @@ export function QueryWrapComponent({ code: string; updateQuery: (qs: string) => void; }) { - const isWrappedInPipes = useMemo(() => { - const pipes = code.split('|'); - const pipesWithNewLine = code?.split('\n|'); - return pipes?.length === pipesWithNewLine?.length; + const isWrappedByPipes = useMemo(() => { + return isQueryWrappedByPipes(code); }, [code]); return ( @@ -30,7 +28,7 @@ export function QueryWrapComponent({ { - const updatedCode = getWrappedInPipesCode(code, isWrappedInPipes); + const updatedCode = prettifyQuery(code, isWrappedByPipes); if (code !== updatedCode) { updateQuery(updatedCode); } diff --git a/packages/kbn-text-based-editor/src/helpers.test.ts b/packages/kbn-text-based-editor/src/helpers.test.ts index ef01f9da3baa0b..fc1008e18c152e 100644 --- a/packages/kbn-text-based-editor/src/helpers.test.ts +++ b/packages/kbn-text-based-editor/src/helpers.test.ts @@ -8,13 +8,7 @@ */ import { dataViewPluginMocks } from '@kbn/data-views-plugin/public/mocks'; -import { - parseErrors, - parseWarning, - getWrappedInPipesCode, - getIndicesList, - getRemoteIndicesList, -} from './helpers'; +import { parseErrors, parseWarning, getIndicesList, getRemoteIndicesList } from './helpers'; describe('helpers', function () { describe('parseErrors', function () { @@ -210,29 +204,6 @@ describe('helpers', function () { }); }); - describe('getWrappedInPipesCode', function () { - it('should return the code wrapped', function () { - const code = getWrappedInPipesCode('FROM index1 | keep field1, field2 | order field1', false); - expect(code).toEqual('FROM index1\n| keep field1, field2\n| order field1'); - }); - - it('should return the code unwrapped', function () { - const code = getWrappedInPipesCode( - 'FROM index1 \n| keep field1, field2 \n| order field1', - true - ); - expect(code).toEqual('FROM index1 | keep field1, field2 | order field1'); - }); - - it('should return the code unwrapped and trimmed', function () { - const code = getWrappedInPipesCode( - 'FROM index1 \n| keep field1, field2 \n| order field1', - true - ); - expect(code).toEqual('FROM index1 | keep field1, field2 | order field1'); - }); - }); - describe('getIndicesList', function () { it('should return also system indices with hidden flag on', async function () { const dataViewsMock = dataViewPluginMocks.createStartContract(); diff --git a/packages/kbn-text-based-editor/src/helpers.ts b/packages/kbn-text-based-editor/src/helpers.ts index f972649d967a76..5e7e9fb0914004 100644 --- a/packages/kbn-text-based-editor/src/helpers.ts +++ b/packages/kbn-text-based-editor/src/helpers.ts @@ -196,14 +196,6 @@ export const getDocumentationSections = async (language: string) => { } }; -export const getWrappedInPipesCode = (code: string, isWrapped: boolean): string => { - const pipes = code?.split('|'); - const codeNoLines = pipes?.map((pipe) => { - return pipe.replaceAll('\n', '').trim(); - }); - return codeNoLines.join(isWrapped ? ' | ' : '\n| '); -}; - export const getIndicesList = async (dataViews: DataViewsPublicPluginStart) => { const indices = await dataViews.getIndices({ showAllIndices: false,