From 08653c6e014444338983bbd8e09ea81274685431 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 19 Sep 2024 11:07:53 -0600 Subject: [PATCH 1/9] set up expectations --- .../autocomplete.command.sort.test.ts | 87 +++++++++++++++---- .../src/autocomplete/__tests__/helpers.ts | 16 +++- .../src/autocomplete/autocomplete.test.ts | 11 +-- 3 files changed, 87 insertions(+), 27 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts index 924790ed470f5..4332edf7f030b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts @@ -7,21 +7,67 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { setup, getFieldNamesByType } from './helpers'; +import { setup, getFieldNamesByType, attachTriggerCommand } from './helpers'; describe('autocomplete.suggest', () => { describe('SORT ( [ ASC / DESC ] [ NULLS FIST / NULLS LAST ] )+', () => { describe('SORT ...', () => { - test('suggests command on first character', async () => { + test.only('suggests column on first character', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions( + 'from a | sort /', + [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort keyw/', + [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort keywordField/', + [ + { + filterText: 'keywordField', + text: 'keywordField, ', + }, + { + filterText: 'keywordField', + text: 'keywordField | ', + }, + { + filterText: 'keywordField', + text: 'keywordField ASC ', + }, + { + filterText: 'keywordField', + text: 'keywordField DESC ', + }, + { + filterText: 'keywordField', + text: 'keywordField NULLS FIRST ', + }, + { + filterText: 'keywordField', + text: 'keywordField NULLS LAST ', + }, + ].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort column, /', + [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) + ); + }); + + test('partial columns', async () => { const { assertSuggestions } = await setup(); await assertSuggestions( 'from a | sort /', - [...getFieldNamesByType('any')].map((field) => `${field} `) + [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) ); await assertSuggestions( 'from a | sort column, /', - [...getFieldNamesByType('any')].map((field) => `${field} `) + [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) ); }); }); @@ -30,14 +76,10 @@ describe('autocomplete.suggest', () => { test('suggests all modifiers on first space', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions('from a | sort stringField /', [ - 'ASC ', - 'DESC ', - 'NULLS FIRST ', - 'NULLS LAST ', - ',', - '| ', - ]); + await assertSuggestions( + 'from a | sort stringField /', + ['ASC ', 'DESC ', 'NULLS FIRST ', 'NULLS LAST ', ', ', '| '].map(attachTriggerCommand) + ); }); test('when user starts to type ASC modifier', async () => { @@ -70,6 +112,7 @@ describe('autocomplete.suggest', () => { test('when user starts to type NULLS modifiers', async () => { const { assertSuggestions } = await setup(); + // @TODO check for replacement range await assertSuggestions('from a | sort stringField N/', ['NULLS FIRST ', 'NULLS LAST ']); await assertSuggestions('from a | sort stringField null/', ['NULLS FIRST ', 'NULLS LAST ']); await assertSuggestions('from a | sort stringField nulls/', [ @@ -85,15 +128,27 @@ describe('autocomplete.suggest', () => { test('when user types NULLS FIRST', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions('from a | sort stringField NULLS F/', ['NULLS FIRST ']); - await assertSuggestions('from a | sort stringField NULLS FI/', ['NULLS FIRST ']); + await assertSuggestions( + 'from a | sort stringField NULLS F/', + ['NULLS FIRST '].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort stringField NULLS FI/', + ['NULLS FIRST '].map(attachTriggerCommand) + ); }); test('when user types NULLS LAST', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions('from a | sort stringField NULLS L/', ['NULLS LAST ']); - await assertSuggestions('from a | sort stringField NULLS LAS/', ['NULLS LAST ']); + await assertSuggestions( + 'from a | sort stringField NULLS L/', + ['NULLS LAST '].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort stringField NULLS LAS/', + ['NULLS LAST '].map(attachTriggerCommand) + ); }); test('after nulls are entered, suggests comma or pipe', async () => { diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts index 7d09f2c82c4b7..93fd194d93a54 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/helpers.ts @@ -17,7 +17,7 @@ import { groupingFunctionDefinitions } from '../../definitions/grouping'; import * as autocomplete from '../autocomplete'; import type { ESQLCallbacks } from '../../shared/types'; import type { EditorContext, SuggestionRawDefinition } from '../types'; -import { TIME_SYSTEM_PARAMS, getSafeInsertText } from '../factories'; +import { TIME_SYSTEM_PARAMS, TRIGGER_SUGGESTION_COMMAND, getSafeInsertText } from '../factories'; import { getFunctionSignatures } from '../../definitions/helpers'; import { ESQLRealField } from '../../validation/types'; import { @@ -348,3 +348,17 @@ export const setup = async (caret = '/') => { assertSuggestions, }; }; + +/** + * Attaches the trigger command to an expected suggestion to make + * sure the suggestions menu will be opened when the suggestion is accepted. + */ +export const attachTriggerCommand = ( + s: string | PartialSuggestionWithText +): PartialSuggestionWithText => + typeof s === 'string' + ? { + text: s, + command: TRIGGER_SUGGESTION_COMMAND, + } + : { ...s, command: TRIGGER_SUGGESTION_COMMAND }; diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index 2a8c8e53fcab7..423c18038d8c4 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -30,6 +30,7 @@ import { PartialSuggestionWithText, TIME_PICKER_SUGGESTION, setup, + attachTriggerCommand, } from './__tests__/helpers'; import { METADATA_FIELDS } from '../shared/constants'; import { ESQL_COMMON_NUMERIC_TYPES, ESQL_STRING_TYPES } from '../shared/esql_types'; @@ -699,16 +700,6 @@ describe('autocomplete', () => { * NOTE: Monaco uses an Invoke trigger kind when the show suggestions action is triggered (e.g. accepting the "FROM" suggestion) */ - const attachTriggerCommand = ( - s: string | PartialSuggestionWithText - ): PartialSuggestionWithText => - typeof s === 'string' - ? { - text: s, - command: TRIGGER_SUGGESTION_COMMAND, - } - : { ...s, command: TRIGGER_SUGGESTION_COMMAND }; - const attachAsSnippet = (s: PartialSuggestionWithText): PartialSuggestionWithText => ({ ...s, asSnippet: true, From 209c83bf60c9d3dd1b9b2f39d1b1bfc34a077af2 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 19 Sep 2024 11:12:48 -0600 Subject: [PATCH 2/9] open suggestions menu when the field suggestion is accepted --- .../src/autocomplete/autocomplete.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 50937abbde9fc..302ce095f0566 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1933,5 +1933,8 @@ export const suggestForSortCmd = async (innerText: string, getFieldsByType: GetF } } - return (await getFieldsByType('any', [], { advanceCursor: true })) as SuggestionRawDefinition[]; + return (await getFieldsByType('any', [], { + advanceCursor: true, + openSuggestions: true, + })) as SuggestionRawDefinition[]; }; From 8b2f849d56c3ee5bdeedfe382b35342891871b07 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 19 Sep 2024 11:57:45 -0600 Subject: [PATCH 3/9] handling field fragments in SORT --- .../autocomplete.command.sort.test.ts | 66 +++--- .../src/autocomplete/autocomplete.ts | 207 ++++++++++++------ 2 files changed, 167 insertions(+), 106 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts index 4332edf7f030b..340d3ffa7a5a8 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts @@ -23,39 +23,39 @@ describe('autocomplete.suggest', () => { 'from a | sort keyw/', [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) ); - await assertSuggestions( - 'from a | sort keywordField/', - [ - { - filterText: 'keywordField', - text: 'keywordField, ', - }, - { - filterText: 'keywordField', - text: 'keywordField | ', - }, - { - filterText: 'keywordField', - text: 'keywordField ASC ', - }, - { - filterText: 'keywordField', - text: 'keywordField DESC ', - }, - { - filterText: 'keywordField', - text: 'keywordField NULLS FIRST ', - }, - { - filterText: 'keywordField', - text: 'keywordField NULLS LAST ', - }, - ].map(attachTriggerCommand) - ); - await assertSuggestions( - 'from a | sort column, /', - [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) - ); + // await assertSuggestions( + // 'from a | sort keywordField/', + // [ + // { + // filterText: 'keywordField', + // text: 'keywordField, ', + // }, + // { + // filterText: 'keywordField', + // text: 'keywordField | ', + // }, + // { + // filterText: 'keywordField', + // text: 'keywordField ASC ', + // }, + // { + // filterText: 'keywordField', + // text: 'keywordField DESC ', + // }, + // { + // filterText: 'keywordField', + // text: 'keywordField NULLS FIRST ', + // }, + // { + // filterText: 'keywordField', + // text: 'keywordField NULLS LAST ', + // }, + // ].map(attachTriggerCommand) + // ); + // await assertSuggestions( + // 'from a | sort column, /', + // [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) + // ); }); test('partial columns', async () => { diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 302ce095f0566..8c29e2f81863a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -193,10 +193,6 @@ export async function suggest( } if (astContext.type === 'expression') { - if (astContext.command.name === 'sort') { - return await suggestForSortCmd(innerText, getFieldsByType); - } - // suggest next possible argument, or option // otherwise a variable return getExpressionSuggestionsByType( @@ -564,6 +560,12 @@ async function getExpressionSuggestionsByType( const references = { fields: fieldsMap, variables: anyVariables }; + if (command.name === 'sort') { + return await suggestForSortCmd(innerText, getFieldsByType, (col) => + Boolean(getColumnByName(col, references)) + ); + } + const suggestions: SuggestionRawDefinition[] = []; // in this flow there's a clear plan here from argument definitions so try to follow it @@ -612,59 +614,33 @@ async function getExpressionSuggestionsByType( } ); - /** - * @TODO — this string scanning is crude and can't support all cases - * Checking for a partial word and computing the replacement range should - * really be done using the AST node, but we'll have to refactor further upstream - * to make that available. This is a quick fix to support the most common case. - */ - const lastWord = findFinalWord(innerText); - if (lastWord !== '') { - // ... | - - const rangeToReplace = { - start: innerText.length - lastWord.length + 1, - end: innerText.length + 1, - }; - - // check if lastWord is an existing field - const column = getColumnByName(lastWord, references); - if (column) { - if (['grok', 'dissect'].includes(command.name)) { - return []; - } - // now we know that the user has already entered a column, - // so suggest comma and pipe + const fieldFragmentSuggestions = await handleFragment( + innerText, + (fragment) => Boolean(getColumnByName(fragment, references)), + (fragment: string, rangeToReplace?: { start: number; end: number }) => { + // SORT fie + return fieldSuggestions.map((suggestion) => ({ + ...suggestion, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })); + }, + (fragment: string, rangeToReplace: { start: number; end: number }) => { + // SORT field return [ { ...pipeCompleteItem, text: ' | ' }, { ...commaCompleteItem, text: ', ' }, ].map((s) => ({ ...s, - filterText: lastWord, - text: lastWord + s.text, + filterText: fragment, + text: fragment + s.text, command: TRIGGER_SUGGESTION_COMMAND, rangeToReplace, })); - } else { - suggestions.push( - ...fieldSuggestions.map((suggestion) => ({ - ...suggestion, - text: suggestion.text + (['grok', 'dissect'].includes(command.name) ? ' ' : ''), - command: TRIGGER_SUGGESTION_COMMAND, - rangeToReplace, - })) - ); } - } else { - // ... | - suggestions.push( - ...fieldSuggestions.map((suggestion) => ({ - ...suggestion, - text: suggestion.text + (['grok', 'dissect'].includes(command.name) ? ' ' : ''), - command: TRIGGER_SUGGESTION_COMMAND, - })) - ); - } + ); + + suggestions.push(...fieldFragmentSuggestions); } } if (argDef.type === 'function' || argDef.type === 'any') { @@ -1839,6 +1815,64 @@ async function getOptionArgsSuggestions( return suggestions; } +/** + * This function handles the logic to suggest completions + * for a given fragment of text in a generic way. A good example is + * a field name. + * + * When typing a field name, there are three scenarios + * + * 1. user hasn't begun typing + * KEEP / + * + * 2. user is typing a partial field name + * KEEP fie/ + * + * 3. user has typed a complete field name + * KEEP field/ + * + * This function provides a framework for handling all three scenarios in a clean way. + * + * @param innerText - the query text before the current cursor position + * @param isFragmentComplete — return true if the fragment is complete + * @param getSuggestionsForIncomplete — gets suggestions for an incomplete fragment + * @param getSuggestionsForComplete - gets suggestions for a complete fragment + * @returns + */ +function handleFragment( + innerText: string, + isFragmentComplete: (fragment: string) => boolean, + getSuggestionsForIncomplete: ( + fragment: string, + rangeToReplace?: { start: number; end: number } + ) => SuggestionRawDefinition[] | Promise, + getSuggestionsForComplete: ( + fragment: string, + rangeToReplace: { start: number; end: number } + ) => SuggestionRawDefinition[] | Promise +): SuggestionRawDefinition[] | Promise { + /** + * @TODO — this string manipulation is crude and can't support all cases + * Checking for a partial word and computing the replacement range should + * really be done using the AST node, but we'll have to refactor further upstream + * to make that available. This is a quick fix to support the most common case. + */ + const fragment = findFinalWord(innerText); + if (!fragment) { + return getSuggestionsForIncomplete(''); + } else { + const rangeToReplace = { + start: innerText.length - fragment.length + 1, + end: innerText.length + 1, + }; + if (isFragmentComplete(fragment)) { + return getSuggestionsForComplete(fragment, rangeToReplace); + } else { + return getSuggestionsForIncomplete(fragment, rangeToReplace); + } + } +} + const sortModifierSuggestions = { ASC: { label: 'ASC', @@ -1874,8 +1908,12 @@ const sortModifierSuggestions = { } as SuggestionRawDefinition, }; -export const suggestForSortCmd = async (innerText: string, getFieldsByType: GetFieldsByTypeFn) => { - const { pos, order, nulls } = getSortPos(innerText); +export const suggestForSortCmd = async ( + innerText: string, + getFieldsByType: GetFieldsByTypeFn, + columnExists: (column: string) => boolean +): Promise => { + const { pos, nulls } = getSortPos(innerText); switch (pos) { case 'space2': { @@ -1890,13 +1928,7 @@ export const suggestForSortCmd = async (innerText: string, getFieldsByType: GetF ]; } case 'order': { - const suggestions: SuggestionRawDefinition[] = []; - for (const modifier of Object.values(sortModifierSuggestions)) { - if (modifier.label.startsWith(order)) { - suggestions.push(modifier); - } - } - return suggestions; + return Object.values(sortModifierSuggestions); } case 'space3': { return [ @@ -1910,19 +1942,13 @@ export const suggestForSortCmd = async (innerText: string, getFieldsByType: GetF case 'nulls': { const end = innerText.length + 1; const start = end - nulls.length; - const suggestions: SuggestionRawDefinition[] = []; - for (const modifier of Object.values(sortModifierSuggestions)) { - if (modifier.label.startsWith(nulls)) { - suggestions.push({ - ...modifier, - rangeToReplace: { - start, - end, - }, - }); - } - } - return suggestions; + return Object.values(sortModifierSuggestions).map((modifier) => ({ + ...modifier, + rangeToReplace: { + start, + end, + }, + })); } case 'space4': { return [ @@ -1933,8 +1959,43 @@ export const suggestForSortCmd = async (innerText: string, getFieldsByType: GetF } } - return (await getFieldsByType('any', [], { - advanceCursor: true, + const fieldSuggestions = await getFieldsByType('any', [], { openSuggestions: true, - })) as SuggestionRawDefinition[]; + }); + + return await handleFragment( + innerText, + columnExists, + (_fragment: string, rangeToReplace?: { start: number; end: number }) => { + // SORT fie + return fieldSuggestions.map((suggestion) => ({ + ...suggestion, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })); + }, + (fragment: string, rangeToReplace: { start: number; end: number }) => { + // SORT field + return [ + { ...pipeCompleteItem, text: ' | ' }, + { ...commaCompleteItem, text: ', ' }, + { ...sortModifierSuggestions.ASC, text: ' ' + sortModifierSuggestions.ASC.text }, + { ...sortModifierSuggestions.DESC, text: ' ' + sortModifierSuggestions.DESC.text }, + { + ...sortModifierSuggestions.NULLS_FIRST, + text: ' ' + sortModifierSuggestions.NULLS_FIRST.text, + }, + { + ...sortModifierSuggestions.NULLS_LAST, + text: ' ' + sortModifierSuggestions.NULLS_LAST.text, + }, + ].map((s) => ({ + ...s, + filterText: fragment, + text: fragment + s.text, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })); + } + ); }; From bde5b90c038debea7219ab7102b06eaaf1bdeb00 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 19 Sep 2024 14:40:46 -0600 Subject: [PATCH 4/9] implement the new, fragment-based pattern --- .../autocomplete.command.sort.test.ts | 191 ++++++++++++------ .../src/autocomplete/autocomplete.ts | 98 +++++---- .../src/autocomplete/commands/sort/helper.ts | 4 - 3 files changed, 191 insertions(+), 102 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts index 340d3ffa7a5a8..82e9f33f8f0d5 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts @@ -12,62 +12,86 @@ import { setup, getFieldNamesByType, attachTriggerCommand } from './helpers'; describe('autocomplete.suggest', () => { describe('SORT ( [ ASC / DESC ] [ NULLS FIST / NULLS LAST ] )+', () => { describe('SORT ...', () => { - test.only('suggests column on first character', async () => { + test('suggests column', async () => { const { assertSuggestions } = await setup(); await assertSuggestions( 'from a | sort /', - [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) + [...getFieldNamesByType('any')].map(attachTriggerCommand) ); await assertSuggestions( 'from a | sort keyw/', - [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) - ); - // await assertSuggestions( - // 'from a | sort keywordField/', - // [ - // { - // filterText: 'keywordField', - // text: 'keywordField, ', - // }, - // { - // filterText: 'keywordField', - // text: 'keywordField | ', - // }, - // { - // filterText: 'keywordField', - // text: 'keywordField ASC ', - // }, - // { - // filterText: 'keywordField', - // text: 'keywordField DESC ', - // }, - // { - // filterText: 'keywordField', - // text: 'keywordField NULLS FIRST ', - // }, - // { - // filterText: 'keywordField', - // text: 'keywordField NULLS LAST ', - // }, - // ].map(attachTriggerCommand) - // ); - // await assertSuggestions( - // 'from a | sort column, /', - // [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) - // ); + [...getFieldNamesByType('any')].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort keywordField/', + [ + { + filterText: 'keywordField', + text: 'keywordField, ', + }, + { + filterText: 'keywordField', + text: 'keywordField | ', + }, + { + filterText: 'keywordField', + text: 'keywordField ASC', + }, + { + filterText: 'keywordField', + text: 'keywordField DESC', + }, + { + filterText: 'keywordField', + text: 'keywordField NULLS FIRST', + }, + { + filterText: 'keywordField', + text: 'keywordField NULLS LAST', + }, + ].map(attachTriggerCommand) + ); }); - - test('partial columns', async () => { + it('suggests subsequent column after comma', async () => { const { assertSuggestions } = await setup(); await assertSuggestions( - 'from a | sort /', - [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) + 'from a | sort keywordField, /', + [...getFieldNamesByType('any')].map(attachTriggerCommand) ); await assertSuggestions( - 'from a | sort column, /', - [...getFieldNamesByType('any')].map((field) => `${field} `).map(attachTriggerCommand) + 'from a | sort keywordField, doubl/', + [...getFieldNamesByType('any')].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort keywordField, doubleField/', + [ + { + filterText: 'doubleField', + text: 'doubleField, ', + }, + { + filterText: 'doubleField', + text: 'doubleField | ', + }, + { + filterText: 'doubleField', + text: 'doubleField ASC', + }, + { + filterText: 'doubleField', + text: 'doubleField DESC', + }, + { + filterText: 'doubleField', + text: 'doubleField NULLS FIRST', + }, + { + filterText: 'doubleField', + text: 'doubleField NULLS LAST', + }, + ].map(attachTriggerCommand) ); }); }); @@ -78,33 +102,55 @@ describe('autocomplete.suggest', () => { await assertSuggestions( 'from a | sort stringField /', - ['ASC ', 'DESC ', 'NULLS FIRST ', 'NULLS LAST ', ', ', '| '].map(attachTriggerCommand) + ['ASC', 'DESC', 'NULLS FIRST', 'NULLS LAST', ', ', '| '].map(attachTriggerCommand) ); }); test('when user starts to type ASC modifier', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions('from a | sort stringField A/', ['ASC ']); + await assertSuggestions( + 'from a | sort stringField A/', + ['ASC', 'DESC', 'NULLS FIRST', 'NULLS LAST'].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort stringField ASC/', + ['ASC NULLS FIRST', 'ASC NULLS LAST', 'ASC, ', 'ASC | '].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort stringField asc/', + ['asc NULLS FIRST', 'asc NULLS LAST', 'asc, ', 'asc | '].map(attachTriggerCommand) + ); }); test('when user starts to type DESC modifier', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions('from a | sort stringField d/', ['DESC ']); - await assertSuggestions('from a | sort stringField des/', ['DESC ']); - await assertSuggestions('from a | sort stringField DES/', ['DESC ']); + await assertSuggestions( + 'from a | sort stringField D/', + ['ASC', 'DESC', 'NULLS FIRST', 'NULLS LAST'].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort stringField DESC/', + ['DESC NULLS FIRST', 'DESC NULLS LAST', 'DESC, ', 'DESC | '].map(attachTriggerCommand) + ); + await assertSuggestions('from a | sort stringField desc/', [ + 'desc NULLS FIRST', + 'desc NULLS LAST', + 'desc, ', + 'desc | ', + ]); }); }); - describe('... [ NULLS FIST / NULLS LAST ]', () => { - test('suggests command on first character', async () => { + describe('... [ NULLS FIRST / NULLS LAST ]', () => { + test('suggests nulls modifier after order modifier + space', async () => { const { assertSuggestions } = await setup(); await assertSuggestions('from a | sort stringField ASC /', [ - 'NULLS FIRST ', - 'NULLS LAST ', - ',', + 'NULLS FIRST', + 'NULLS LAST', + ', ', '| ', ]); }); @@ -113,15 +159,29 @@ describe('autocomplete.suggest', () => { const { assertSuggestions } = await setup(); // @TODO check for replacement range - await assertSuggestions('from a | sort stringField N/', ['NULLS FIRST ', 'NULLS LAST ']); - await assertSuggestions('from a | sort stringField null/', ['NULLS FIRST ', 'NULLS LAST ']); + await assertSuggestions('from a | sort stringField N/', [ + 'ASC', + 'DESC', + 'NULLS FIRST', + 'NULLS LAST', + ]); + await assertSuggestions('from a | sort stringField null/', [ + 'ASC', + 'DESC', + 'NULLS FIRST', + 'NULLS LAST', + ]); await assertSuggestions('from a | sort stringField nulls/', [ - 'NULLS FIRST ', - 'NULLS LAST ', + 'ASC', + 'DESC', + 'NULLS FIRST', + 'NULLS LAST', ]); await assertSuggestions('from a | sort stringField nulls /', [ - 'NULLS FIRST ', - 'NULLS LAST ', + 'ASC', + 'DESC', + 'NULLS FIRST', + 'NULLS LAST', ]); }); @@ -130,11 +190,11 @@ describe('autocomplete.suggest', () => { await assertSuggestions( 'from a | sort stringField NULLS F/', - ['NULLS FIRST '].map(attachTriggerCommand) + ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand) ); await assertSuggestions( 'from a | sort stringField NULLS FI/', - ['NULLS FIRST '].map(attachTriggerCommand) + ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand) ); }); @@ -143,18 +203,21 @@ describe('autocomplete.suggest', () => { await assertSuggestions( 'from a | sort stringField NULLS L/', - ['NULLS LAST '].map(attachTriggerCommand) + ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand) ); await assertSuggestions( 'from a | sort stringField NULLS LAS/', - ['NULLS LAST '].map(attachTriggerCommand) + ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand) ); }); test('after nulls are entered, suggests comma or pipe', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions('from a | sort stringField NULLS LAST /', [',', '| ']); + await assertSuggestions( + 'from a | sort stringField NULLS LAST /', + [', ', '| '].map(attachTriggerCommand) + ); }); }); }); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 8c29e2f81863a..cb2b95e8468f4 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -617,7 +617,7 @@ async function getExpressionSuggestionsByType( const fieldFragmentSuggestions = await handleFragment( innerText, (fragment) => Boolean(getColumnByName(fragment, references)), - (fragment: string, rangeToReplace?: { start: number; end: number }) => { + (_fragment: string, rangeToReplace?: { start: number; end: number }) => { // SORT fie return fieldSuggestions.map((suggestion) => ({ ...suggestion, @@ -1876,7 +1876,7 @@ function handleFragment( const sortModifierSuggestions = { ASC: { label: 'ASC', - text: 'ASC ', + text: 'ASC', detail: '', kind: 'Keyword', sortText: '1-ASC', @@ -1884,7 +1884,7 @@ const sortModifierSuggestions = { } as SuggestionRawDefinition, DESC: { label: 'DESC', - text: 'DESC ', + text: 'DESC', detail: '', kind: 'Keyword', sortText: '1-DESC', @@ -1892,7 +1892,7 @@ const sortModifierSuggestions = { } as SuggestionRawDefinition, NULLS_FIRST: { label: 'NULLS FIRST', - text: 'NULLS FIRST ', + text: 'NULLS FIRST', detail: '', kind: 'Keyword', sortText: '2-NULLS FIRST', @@ -1900,7 +1900,7 @@ const sortModifierSuggestions = { } as SuggestionRawDefinition, NULLS_LAST: { label: 'NULLS LAST', - text: 'NULLS LAST ', + text: 'NULLS LAST', detail: '', kind: 'Keyword', sortText: '2-NULLS LAST', @@ -1913,7 +1913,9 @@ export const suggestForSortCmd = async ( getFieldsByType: GetFieldsByTypeFn, columnExists: (column: string) => boolean ): Promise => { - const { pos, nulls } = getSortPos(innerText); + const prependSpace = (s: SuggestionRawDefinition) => ({ ...s, text: ' ' + s.text }); + + const { pos } = getSortPos(innerText); switch (pos) { case 'space2': { @@ -1922,39 +1924,73 @@ export const suggestForSortCmd = async ( sortModifierSuggestions.DESC, sortModifierSuggestions.NULLS_FIRST, sortModifierSuggestions.NULLS_LAST, - ...getFinalSuggestions({ - comma: true, - }), + pipeCompleteItem, + { ...commaCompleteItem, text: ', ', command: TRIGGER_SUGGESTION_COMMAND }, ]; } case 'order': { - return Object.values(sortModifierSuggestions); + return handleFragment( + innerText, + (fragment) => ['ASC', 'DESC'].some((completeWord) => noCaseCompare(completeWord, fragment)), + (_fragment, rangeToReplace) => { + return Object.values(sortModifierSuggestions).map((suggestion) => ({ + ...suggestion, + rangeToReplace, + })); + }, + (fragment, rangeToReplace) => { + return [ + { ...pipeCompleteItem, text: ' | ' }, + { ...commaCompleteItem, text: ', ' }, + prependSpace(sortModifierSuggestions.NULLS_FIRST), + prependSpace(sortModifierSuggestions.NULLS_LAST), + ].map((suggestion) => ({ + ...suggestion, + filterText: fragment, + text: fragment + suggestion.text, + rangeToReplace, + command: TRIGGER_SUGGESTION_COMMAND, + })); + } + ); } case 'space3': { return [ sortModifierSuggestions.NULLS_FIRST, sortModifierSuggestions.NULLS_LAST, - ...getFinalSuggestions({ - comma: true, - }), + pipeCompleteItem, + { ...commaCompleteItem, text: ', ', command: TRIGGER_SUGGESTION_COMMAND }, ]; } case 'nulls': { - const end = innerText.length + 1; - const start = end - nulls.length; - return Object.values(sortModifierSuggestions).map((modifier) => ({ - ...modifier, - rangeToReplace: { - start, - end, + return handleFragment( + innerText, + (fragment) => + ['FIRST', 'LAST'].some((completeWord) => noCaseCompare(completeWord, fragment)), + (_fragment, rangeToReplace) => { + return Object.values(sortModifierSuggestions).map((suggestion) => ({ + ...suggestion, + rangeToReplace, + })); }, - })); + (fragment, rangeToReplace) => { + return [ + { ...pipeCompleteItem, text: ' | ' }, + { ...commaCompleteItem, text: ', ' }, + ].map((suggestion) => ({ + ...suggestion, + filterText: fragment, + text: fragment + suggestion.text, + rangeToReplace, + command: TRIGGER_SUGGESTION_COMMAND, + })); + } + ); } case 'space4': { return [ - ...getFinalSuggestions({ - comma: true, - }), + pipeCompleteItem, + { ...commaCompleteItem, text: ', ', command: TRIGGER_SUGGESTION_COMMAND }, ]; } } @@ -1979,16 +2015,10 @@ export const suggestForSortCmd = async ( return [ { ...pipeCompleteItem, text: ' | ' }, { ...commaCompleteItem, text: ', ' }, - { ...sortModifierSuggestions.ASC, text: ' ' + sortModifierSuggestions.ASC.text }, - { ...sortModifierSuggestions.DESC, text: ' ' + sortModifierSuggestions.DESC.text }, - { - ...sortModifierSuggestions.NULLS_FIRST, - text: ' ' + sortModifierSuggestions.NULLS_FIRST.text, - }, - { - ...sortModifierSuggestions.NULLS_LAST, - text: ' ' + sortModifierSuggestions.NULLS_LAST.text, - }, + prependSpace(sortModifierSuggestions.ASC), + prependSpace(sortModifierSuggestions.DESC), + prependSpace(sortModifierSuggestions.NULLS_FIRST), + prependSpace(sortModifierSuggestions.NULLS_LAST), ].map((s) => ({ ...s, filterText: fragment, diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts index dfa5ce0f4d5f7..96546eff7d391 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.ts @@ -40,14 +40,12 @@ export interface SortCaretPosition { | 'space3' | 'nulls' | 'space4'; - order: string; nulls: string; } export const getSortPos = (query: string): SortCaretPosition => { const match = query.match(regex); let pos: SortCaretPosition['pos'] = 'none'; - let order: SortCaretPosition['order'] = ''; let nulls: SortCaretPosition['nulls'] = ''; if (match?.groups?.space4) { @@ -59,7 +57,6 @@ export const getSortPos = (query: string): SortCaretPosition => { pos = 'space3'; } else if (match?.groups?.order) { pos = 'order'; - order = match.groups.order.toUpperCase(); } else if (match?.groups?.space2) { pos = 'space2'; } else if (match?.groups?.column) { @@ -78,7 +75,6 @@ export const getSortPos = (query: string): SortCaretPosition => { return { pos, - order, nulls, }; }; From 38d766ef0a28ceba8b27285ba109b269e07ffe72 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 19 Sep 2024 14:53:41 -0600 Subject: [PATCH 5/9] restore the correct range replacement behavior --- .../__tests__/autocomplete.command.sort.test.ts | 14 ++++++++++++-- .../src/autocomplete/autocomplete.ts | 10 +++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts index 82e9f33f8f0d5..b6d70dfe6ac4d 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts @@ -190,11 +190,21 @@ describe('autocomplete.suggest', () => { await assertSuggestions( 'from a | sort stringField NULLS F/', - ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand) + [ + 'ASC', + 'DESC', + { text: 'NULLS LAST', rangeToReplace: { start: 27, end: 34 } }, + { text: 'NULLS FIRST', rangeToReplace: { start: 27, end: 34 } }, + ].map(attachTriggerCommand) ); await assertSuggestions( 'from a | sort stringField NULLS FI/', - ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand) + [ + 'ASC', + 'DESC', + { text: 'NULLS LAST', rangeToReplace: { start: 27, end: 35 } }, + { text: 'NULLS FIRST', rangeToReplace: { start: 27, end: 35 } }, + ].map(attachTriggerCommand) ); }); diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index cb2b95e8468f4..adb418f7eda1e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1915,7 +1915,7 @@ export const suggestForSortCmd = async ( ): Promise => { const prependSpace = (s: SuggestionRawDefinition) => ({ ...s, text: ' ' + s.text }); - const { pos } = getSortPos(innerText); + const { pos, nulls } = getSortPos(innerText); switch (pos) { case 'space2': { @@ -1967,10 +1967,14 @@ export const suggestForSortCmd = async ( innerText, (fragment) => ['FIRST', 'LAST'].some((completeWord) => noCaseCompare(completeWord, fragment)), - (_fragment, rangeToReplace) => { + (_fragment) => { + const end = innerText.length + 1; + const start = end - nulls.length; return Object.values(sortModifierSuggestions).map((suggestion) => ({ ...suggestion, - rangeToReplace, + // we can't use the range generated by handleFragment here + // because it doesn't really support multi-word completions + rangeToReplace: { start, end }, })); }, (fragment, rangeToReplace) => { From 862d692b97e5fe2d13cd4468e5866a7fc25a399f Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Thu, 19 Sep 2024 15:11:31 -0600 Subject: [PATCH 6/9] adopt handleFragment for sources --- .../src/autocomplete/autocomplete.ts | 90 ++++++++++--------- 1 file changed, 46 insertions(+), 44 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index adb418f7eda1e..8030ca2d0411b 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -897,53 +897,55 @@ async function getExpressionSuggestionsByType( if (lastIndex && lastIndex.text && lastIndex.text !== EDITOR_MARKER) { const sources = await getSources(); - const sourceIdentifier = lastIndex.text.replace(EDITOR_MARKER, ''); - if (sourceExists(sourceIdentifier, new Set(sources.map(({ name }) => name)))) { - const exactMatch = sources.find(({ name: _name }) => _name === sourceIdentifier); - if (exactMatch?.dataStreams) { - // this is an integration name, suggest the datastreams - addSuggestionsBasedOnQuote( - buildSourcesDefinitions( + const suggestionsToAdd = await handleFragment( + innerText, + (fragment) => + sourceExists(fragment, new Set(sources.map(({ name: sourceName }) => sourceName))), + (_fragment, rangeToReplace) => { + return getSourceSuggestions(sources).map((suggestion) => ({ + ...suggestion, + rangeToReplace, + })); + }, + (fragment, rangeToReplace) => { + const exactMatch = sources.find(({ name: _name }) => _name === fragment); + if (exactMatch?.dataStreams) { + // this is an integration name, suggest the datastreams + const definitions = buildSourcesDefinitions( exactMatch.dataStreams.map(({ name }) => ({ name, isIntegration: false })) - ) - ); - } else { - // this is a complete source name - const rangeToReplace = { - start: innerText.length - sourceIdentifier.length + 1, - end: innerText.length + 1, - }; + ); - const suggestionsToAdd: SuggestionRawDefinition[] = [ - { - ...pipeCompleteItem, - filterText: sourceIdentifier, - text: sourceIdentifier + ' | ', - command: TRIGGER_SUGGESTION_COMMAND, - rangeToReplace, - }, - { - ...commaCompleteItem, - filterText: sourceIdentifier, - text: sourceIdentifier + ', ', - command: TRIGGER_SUGGESTION_COMMAND, - rangeToReplace, - }, - { - ...buildOptionDefinition(metadataOption), - filterText: sourceIdentifier, - text: sourceIdentifier + ' METADATA ', - asSnippet: false, // turn this off because $ could be contained within the source name - rangeToReplace, - }, - ]; - - addSuggestionsBasedOnQuote(suggestionsToAdd); + return canRemoveQuote ? removeQuoteForSuggestedSources(definitions) : definitions; + } else { + const _suggestions: SuggestionRawDefinition[] = [ + { + ...pipeCompleteItem, + filterText: fragment, + text: fragment + ' | ', + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + }, + { + ...commaCompleteItem, + filterText: fragment, + text: fragment + ', ', + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + }, + { + ...buildOptionDefinition(metadataOption), + filterText: fragment, + text: fragment + ' METADATA ', + asSnippet: false, // turn this off because $ could be contained within the source name + rangeToReplace, + }, + ]; + + return _suggestions; + } } - } else { - // Just a partial source name - await addSuggestionsBasedOnQuote(getSourceSuggestions(sources)); - } + ); + addSuggestionsBasedOnQuote(suggestionsToAdd); } else { // FROM or no index/text await addSuggestionsBasedOnQuote(getSourceSuggestions(await getSources())); From 2ce56b3b9f2e79d28204bad8cbb146a053e8ea2d Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 20 Sep 2024 08:06:00 -0600 Subject: [PATCH 7/9] fix grok and dissect behavior --- .../src/autocomplete/autocomplete.test.ts | 10 ++++++++-- .../src/autocomplete/autocomplete.ts | 14 ++++++++++++-- .../src/autocomplete/commands/sort/helper.test.ts | 3 --- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts index 423c18038d8c4..095cb2ffc9d14 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.test.ts @@ -288,7 +288,10 @@ describe('autocomplete', () => { 'from a | grok key/', getFieldNamesByType(ESQL_STRING_TYPES).map((name) => `${name} `) ); - testSuggestions('from a | grok keywordField/', []); + testSuggestions( + 'from a | grok keywordField/', + ['keywordField ', 'textField '].map(attachTriggerCommand) + ); }); describe('dissect', () => { @@ -328,7 +331,10 @@ describe('autocomplete', () => { 'from a | dissect key/', getFieldNamesByType(ESQL_STRING_TYPES).map((name) => `${name} `) ); - testSuggestions('from a | dissect keywordField/', []); + testSuggestions( + 'from a | dissect keywordField/', + ['keywordField ', 'textField '].map(attachTriggerCommand) + ); }); describe('limit', () => { diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 8030ca2d0411b..fe3f6ab13bc93 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -618,15 +618,25 @@ async function getExpressionSuggestionsByType( innerText, (fragment) => Boolean(getColumnByName(fragment, references)), (_fragment: string, rangeToReplace?: { start: number; end: number }) => { - // SORT fie + // COMMAND fie return fieldSuggestions.map((suggestion) => ({ ...suggestion, + text: suggestion.text + (['grok', 'dissect'].includes(command.name) ? ' ' : ''), command: TRIGGER_SUGGESTION_COMMAND, rangeToReplace, })); }, (fragment: string, rangeToReplace: { start: number; end: number }) => { - // SORT field + // COMMAND field + if (['grok', 'dissect'].includes(command.name)) { + return fieldSuggestions.map((suggestion) => ({ + ...suggestion, + text: suggestion.text + ' ', + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })); + } + return [ { ...pipeCompleteItem, text: ' | ' }, { ...commaCompleteItem, text: ', ' }, diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts index 82059b6b7765c..6617bcc6908f3 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/commands/sort/helper.test.ts @@ -31,13 +31,10 @@ test('returns correct position on complete modifier matches', () => { test('returns ASC/DESC matched text', () => { expect(getSortPos('from a | sort col ASC').pos).toBe('order'); - expect(getSortPos('from a | sort col asc').order).toBe('ASC'); expect(getSortPos('from a | sort col as').pos).toBe('order'); - expect(getSortPos('from a | sort col as').order).toBe('AS'); expect(getSortPos('from a | sort col DE').pos).toBe('order'); - expect(getSortPos('from a | sort col DE').order).toBe('DE'); }); test('returns NULLS FIRST/NULLS LAST matched text', () => { From a94929a3d00b6b54727ee0f5a23d86203e12f949 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 20 Sep 2024 09:21:18 -0600 Subject: [PATCH 8/9] use handleFragment for METADATA --- .../src/autocomplete/autocomplete.ts | 60 ++++++++++--------- 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index fe3f6ab13bc93..9b26cd07cb6c3 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -1674,33 +1674,39 @@ async function getOptionArgsSuggestions( if (option.name === 'metadata') { const existingFields = new Set(option.args.filter(isColumnItem).map(({ name }) => name)); const filteredMetaFields = METADATA_FIELDS.filter((name) => !existingFields.has(name)); - const lastWord = findFinalWord(innerText); - if (lastWord) { - // METADATA something - const isField = METADATA_FIELDS.includes(lastWord); - if (isField) { - // METADATA field - suggestions.push({ - ...pipeCompleteItem, - text: lastWord + ' | ', - filterText: lastWord, - command: TRIGGER_SUGGESTION_COMMAND, - }); - if (filteredMetaFields.length > 1) { - suggestions.push({ - ...commaCompleteItem, - text: lastWord + ', ', - filterText: lastWord, - command: TRIGGER_SUGGESTION_COMMAND, - }); - } - } else { - suggestions.push(...buildFieldsDefinitions(filteredMetaFields)); - } - } else if (isNewExpression) { - // METADATA - // METADATA field, - suggestions.push(...buildFieldsDefinitions(filteredMetaFields)); + if (isNewExpression) { + suggestions.push( + ...(await handleFragment( + innerText, + (fragment) => METADATA_FIELDS.includes(fragment), + (_fragment, rangeToReplace) => + buildFieldsDefinitions(filteredMetaFields).map((suggestion) => ({ + ...suggestion, + rangeToReplace, + })), + (fragment, rangeToReplace) => { + const _suggestions = [ + { + ...pipeCompleteItem, + text: fragment + ' | ', + filterText: fragment, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + }, + ]; + if (filteredMetaFields.length > 1) { + _suggestions.push({ + ...commaCompleteItem, + text: fragment + ', ', + filterText: fragment, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + }); + } + return _suggestions; + } + )) + ); } else { if (existingFields.size > 0) { // METADATA field From 40c2e833f05a5018bee7b22e69753a869b81ace3 Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Fri, 20 Sep 2024 10:43:51 -0600 Subject: [PATCH 9/9] restore function suggestions --- .../autocomplete.command.sort.test.ts | 44 ++++++++++++------- .../src/autocomplete/autocomplete.ts | 27 +++++++++--- 2 files changed, 48 insertions(+), 23 deletions(-) diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts index b6d70dfe6ac4d..c934c4d72e35a 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/__tests__/autocomplete.command.sort.test.ts @@ -7,22 +7,32 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { setup, getFieldNamesByType, attachTriggerCommand } from './helpers'; +import { + setup, + getFieldNamesByType, + attachTriggerCommand, + getFunctionSignaturesByReturnType, +} from './helpers'; describe('autocomplete.suggest', () => { describe('SORT ( [ ASC / DESC ] [ NULLS FIST / NULLS LAST ] )+', () => { describe('SORT ...', () => { + const expectedFieldSuggestions = getFieldNamesByType('any').map(attachTriggerCommand); + const expectedFunctionSuggestions = getFunctionSignaturesByReturnType('sort', 'any', { + scalar: true, + }).map(attachTriggerCommand); + test('suggests column', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions( - 'from a | sort /', - [...getFieldNamesByType('any')].map(attachTriggerCommand) - ); - await assertSuggestions( - 'from a | sort keyw/', - [...getFieldNamesByType('any')].map(attachTriggerCommand) - ); + await assertSuggestions('from a | sort /', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); + await assertSuggestions('from a | sort keyw/', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); await assertSuggestions( 'from a | sort keywordField/', [ @@ -56,14 +66,14 @@ describe('autocomplete.suggest', () => { it('suggests subsequent column after comma', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions( - 'from a | sort keywordField, /', - [...getFieldNamesByType('any')].map(attachTriggerCommand) - ); - await assertSuggestions( - 'from a | sort keywordField, doubl/', - [...getFieldNamesByType('any')].map(attachTriggerCommand) - ); + await assertSuggestions('from a | sort keywordField, /', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); + await assertSuggestions('from a | sort keywordField, doubl/', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); await assertSuggestions( 'from a | sort keywordField, doubleField/', [ diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index 9b26cd07cb6c3..cd06895bbd12e 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -559,7 +559,6 @@ async function getExpressionSuggestionsByType( const canHaveAssignments = ['eval', 'stats', 'row'].includes(command.name); const references = { fields: fieldsMap, variables: anyVariables }; - if (command.name === 'sort') { return await suggestForSortCmd(innerText, getFieldsByType, (col) => Boolean(getColumnByName(col, references)) @@ -2020,17 +2019,33 @@ export const suggestForSortCmd = async ( const fieldSuggestions = await getFieldsByType('any', [], { openSuggestions: true, }); + const functionSuggestions = await getFieldsOrFunctionsSuggestions( + ['any'], + 'sort', + undefined, + getFieldsByType, + { + functions: true, + fields: false, + } + ); return await handleFragment( innerText, columnExists, (_fragment: string, rangeToReplace?: { start: number; end: number }) => { // SORT fie - return fieldSuggestions.map((suggestion) => ({ - ...suggestion, - command: TRIGGER_SUGGESTION_COMMAND, - rangeToReplace, - })); + return [ + ...pushItUpInTheList( + fieldSuggestions.map((suggestion) => ({ + ...suggestion, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })), + true + ), + ...functionSuggestions, + ]; }, (fragment: string, rangeToReplace: { start: number; end: number }) => { // SORT field