From f450e228b38d317a57d906f6c59f6e69d1dd458d Mon Sep 17 00:00:00 2001 From: Drew Tate Date: Mon, 23 Sep 2024 08:23:23 -0600 Subject: [PATCH] [ES|QL] new pattern for `SORT` autocomplete (#193595) ## Summary Part of https://github.com/elastic/kibana/issues/189662. This PR - updates the autocomplete behavior for `SORT` to be in line with other field-list-based experiences like `KEEP` - introduces a shared function, `handleFragment`, which is used to abstract some of the logic required to support this behavior - bulks up the `SORT` tests - restores the function suggestions which I noticed got lost in https://github.com/elastic/kibana/pull/189959 **Before** https://github.com/user-attachments/assets/cad1d073-c010-426f-9628-c0fc6b65eb3c **After** https://github.com/user-attachments/assets/e148ae58-4430-482c-9f8e-c55779c4d822 ### 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 --------- Co-authored-by: Stratoula Kalafateli --- .../autocomplete.command.sort.test.ts | 206 +++++++-- .../src/autocomplete/__tests__/helpers.ts | 16 +- .../src/autocomplete/autocomplete.test.ts | 21 +- .../src/autocomplete/autocomplete.ts | 437 ++++++++++++------ .../autocomplete/commands/sort/helper.test.ts | 3 - .../src/autocomplete/commands/sort/helper.ts | 4 - 6 files changed, 480 insertions(+), 207 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..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,21 +7,101 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import { setup, getFieldNamesByType } from './helpers'; +import { + setup, + getFieldNamesByType, + attachTriggerCommand, + getFunctionSignaturesByReturnType, +} from './helpers'; describe('autocomplete.suggest', () => { describe('SORT ( [ ASC / DESC ] [ NULLS FIST / NULLS LAST ] )+', () => { describe('SORT ...', () => { - test('suggests command on first character', async () => { + 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 /', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); + await assertSuggestions('from a | sort keyw/', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); await assertSuggestions( - 'from a | sort /', - [...getFieldNamesByType('any')].map((field) => `${field} `) + '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) ); + }); + it('suggests subsequent column after comma', async () => { + const { assertSuggestions } = await setup(); + + await assertSuggestions('from a | sort keywordField, /', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); + await assertSuggestions('from a | sort keywordField, doubl/', [ + ...expectedFieldSuggestions, + ...expectedFunctionSuggestions, + ]); await assertSuggestions( - 'from a | sort column, /', - [...getFieldNamesByType('any')].map((field) => `${field} `) + '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) ); }); }); @@ -30,39 +110,57 @@ 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 () => { 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', + ', ', '| ', ]); }); @@ -70,36 +168,76 @@ describe('autocomplete.suggest', () => { test('when user starts to type NULLS modifiers', async () => { const { assertSuggestions } = await setup(); - await assertSuggestions('from a | sort stringField N/', ['NULLS FIRST ', 'NULLS LAST ']); - await assertSuggestions('from a | sort stringField null/', ['NULLS FIRST ', 'NULLS LAST ']); + // @TODO check for replacement range + 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', ]); }); 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/', + [ + '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', + { text: 'NULLS LAST', rangeToReplace: { start: 27, end: 35 } }, + { text: 'NULLS FIRST', rangeToReplace: { start: 27, end: 35 } }, + ].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/', + ['ASC', 'DESC', 'NULLS LAST', 'NULLS FIRST'].map(attachTriggerCommand) + ); + await assertSuggestions( + 'from a | sort stringField NULLS LAS/', + ['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/__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..095cb2ffc9d14 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'; @@ -287,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', () => { @@ -327,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', () => { @@ -699,16 +706,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, diff --git a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts index a6ee324fd2621..41060675a73a8 100644 --- a/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts +++ b/packages/kbn-esql-validation-autocomplete/src/autocomplete/autocomplete.ts @@ -194,10 +194,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( @@ -567,6 +563,11 @@ async function getExpressionSuggestionsByType( !comparisonFunctions.map((fn) => fn.name).includes(previousWord); const references = { fields: fieldsMap, variables: anyVariables }; + if (command.name === 'sort') { + return await suggestForSortCmd(innerText, getFieldsByType, (col) => + Boolean(getColumnByName(col, references)) + ); + } const suggestions: SuggestionRawDefinition[] = []; @@ -616,59 +617,43 @@ 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) { + const fieldFragmentSuggestions = await handleFragment( + innerText, + (fragment) => Boolean(getColumnByName(fragment, references)), + (_fragment: string, rangeToReplace?: { start: number; end: number }) => { + // 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 }) => { + // COMMAND field if (['grok', 'dissect'].includes(command.name)) { - return []; + return fieldSuggestions.map((suggestion) => ({ + ...suggestion, + text: suggestion.text + ' ', + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })); } - // now we know that the user has already entered a column, - // so suggest comma and pipe + 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') { @@ -925,53 +910,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())); @@ -1689,33 +1676,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 @@ -1846,10 +1839,68 @@ 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', - text: 'ASC ', + text: 'ASC', detail: '', kind: 'Keyword', sortText: '1-ASC', @@ -1857,7 +1908,7 @@ const sortModifierSuggestions = { } as SuggestionRawDefinition, DESC: { label: 'DESC', - text: 'DESC ', + text: 'DESC', detail: '', kind: 'Keyword', sortText: '1-DESC', @@ -1865,7 +1916,7 @@ const sortModifierSuggestions = { } as SuggestionRawDefinition, NULLS_FIRST: { label: 'NULLS FIRST', - text: 'NULLS FIRST ', + text: 'NULLS FIRST', detail: '', kind: 'Keyword', sortText: '2-NULLS FIRST', @@ -1873,7 +1924,7 @@ const sortModifierSuggestions = { } as SuggestionRawDefinition, NULLS_LAST: { label: 'NULLS LAST', - text: 'NULLS LAST ', + text: 'NULLS LAST', detail: '', kind: 'Keyword', sortText: '2-NULLS LAST', @@ -1881,8 +1932,14 @@ 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 prependSpace = (s: SuggestionRawDefinition) => ({ ...s, text: ' ' + s.text }); + + const { pos, nulls } = getSortPos(innerText); switch (pos) { case 'space2': { @@ -1891,54 +1948,128 @@ export const suggestForSortCmd = async (innerText: string, getFieldsByType: GetF sortModifierSuggestions.DESC, sortModifierSuggestions.NULLS_FIRST, sortModifierSuggestions.NULLS_LAST, - ...getFinalSuggestions({ - comma: true, - }), + pipeCompleteItem, + { ...commaCompleteItem, text: ', ', command: TRIGGER_SUGGESTION_COMMAND }, ]; } case 'order': { - const suggestions: SuggestionRawDefinition[] = []; - for (const modifier of Object.values(sortModifierSuggestions)) { - if (modifier.label.startsWith(order)) { - suggestions.push(modifier); + 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, + })); } - } - return suggestions; + ); } 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; - const suggestions: SuggestionRawDefinition[] = []; - for (const modifier of Object.values(sortModifierSuggestions)) { - if (modifier.label.startsWith(nulls)) { - suggestions.push({ - ...modifier, - rangeToReplace: { - start, - end, - }, - }); + return handleFragment( + innerText, + (fragment) => + ['FIRST', 'LAST'].some((completeWord) => noCaseCompare(completeWord, fragment)), + (_fragment) => { + const end = innerText.length + 1; + const start = end - nulls.length; + return Object.values(sortModifierSuggestions).map((suggestion) => ({ + ...suggestion, + // we can't use the range generated by handleFragment here + // because it doesn't really support multi-word completions + rangeToReplace: { start, end }, + })); + }, + (fragment, rangeToReplace) => { + return [ + { ...pipeCompleteItem, text: ' | ' }, + { ...commaCompleteItem, text: ', ' }, + ].map((suggestion) => ({ + ...suggestion, + filterText: fragment, + text: fragment + suggestion.text, + rangeToReplace, + command: TRIGGER_SUGGESTION_COMMAND, + })); } - } - return suggestions; + ); } case 'space4': { return [ - ...getFinalSuggestions({ - comma: true, - }), + pipeCompleteItem, + { ...commaCompleteItem, text: ', ', command: TRIGGER_SUGGESTION_COMMAND }, ]; } } - return (await getFieldsByType('any', [], { advanceCursor: true })) as SuggestionRawDefinition[]; + 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 [ + ...pushItUpInTheList( + fieldSuggestions.map((suggestion) => ({ + ...suggestion, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })), + true + ), + ...functionSuggestions, + ]; + }, + (fragment: string, rangeToReplace: { start: number; end: number }) => { + // SORT field + return [ + { ...pipeCompleteItem, text: ' | ' }, + { ...commaCompleteItem, text: ', ' }, + prependSpace(sortModifierSuggestions.ASC), + prependSpace(sortModifierSuggestions.DESC), + prependSpace(sortModifierSuggestions.NULLS_FIRST), + prependSpace(sortModifierSuggestions.NULLS_LAST), + ].map((s) => ({ + ...s, + filterText: fragment, + text: fragment + s.text, + command: TRIGGER_SUGGESTION_COMMAND, + rangeToReplace, + })); + } + ); }; 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', () => { 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, }; };