From ef9a71f483bd83174c613ee5bc049f81889646fb Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Tue, 10 Jan 2023 14:38:22 +0100 Subject: [PATCH] Loki Autocomplete: Improve handling of trailing spaces in queries (#61184) * Loki Autocomplete: improve handling of trailing spaces in the query * Monaco Field: rename resizing handler function * Fix typo Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Co-authored-by: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> --- .../monaco-query-field/MonacoQueryField.tsx | 8 +-- .../completions.test.ts | 53 ++++++++++++------- .../monaco-completion-provider/completions.ts | 15 +++--- .../situation.test.ts | 9 +++- .../monaco-completion-provider/situation.ts | 2 + 5 files changed, 56 insertions(+), 31 deletions(-) diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx b/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx index 9a76f99b90d..38464074038 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/MonacoQueryField.tsx @@ -75,6 +75,7 @@ const getStyles = (theme: GrafanaTheme2) => { container: css` border-radius: ${theme.shape.borderRadius()}; border: 1px solid ${theme.components.input.borderColor}; + width: 100%; `, }; }; @@ -177,19 +178,18 @@ const MonacoQueryField = ({ history, onBlur, onRunQuery, initialValue, datasourc // (it will grow taller when necessary) // FIXME: maybe move this functionality into CodeEditor, like: // - const updateElementHeight = () => { + const handleResize = () => { const containerDiv = containerRef.current; if (containerDiv !== null) { const pixelHeight = editor.getContentHeight(); containerDiv.style.height = `${pixelHeight + EDITOR_HEIGHT_OFFSET}px`; - containerDiv.style.width = '100%'; const pixelWidth = containerDiv.clientWidth; editor.layout({ width: pixelWidth, height: pixelHeight }); } }; - editor.onDidContentSizeChange(updateElementHeight); - updateElementHeight(); + editor.onDidContentSizeChange(handleResize); + handleResize(); // handle: shift + enter // FIXME: maybe move this functionality into CodeEditor? editor.addCommand( diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts index d7a19aadcbd..d38edd3ba59 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.test.ts @@ -134,9 +134,14 @@ const afterSelectorCompletions = [ }, ]; -function buildAfterSelectorCompletions(detectedParser: string, otherParser: string, afterPipe: boolean) { +function buildAfterSelectorCompletions( + detectedParser: string, + otherParser: string, + afterPipe: boolean, + hasSpace: boolean +) { const explanation = '(detected)'; - const expectedCompletions = afterSelectorCompletions.map((completion) => { + let expectedCompletions = afterSelectorCompletions.map((completion) => { if (completion.type === 'DETECTED_PARSER_PLACEHOLDER') { return { ...completion, @@ -158,17 +163,22 @@ function buildAfterSelectorCompletions(detectedParser: string, otherParser: stri if (afterPipe) { // Remove pipe - return ( - expectedCompletions - .map((completion) => { - completion.insertText = completion.insertText.replace('|', ''); - return completion; - }) - // Remove != and !~ - .filter((completion) => !completion.insertText.startsWith('!')) - ); + expectedCompletions = expectedCompletions + .map((completion) => { + completion.insertText = completion.insertText.replace('|', '').trimStart(); + return completion; + }) + // Remove != and !~ + .filter((completion) => !completion.insertText.startsWith('!')) + .filter((completion) => (hasSpace ? completion.type !== 'LINE_FILTER' : true)); } + expectedCompletions.forEach((completion) => { + if (completion.type !== 'LINE_FILTER') { + completion.insertText = hasSpace ? completion.insertText.trimStart() : ` ${completion.insertText}`; + } + }); + return expectedCompletions; } @@ -307,34 +317,39 @@ describe('getCompletions', () => { ]); }); - test.each([true, false])( - 'Returns completion options when the situation is AFTER_SELECTOR, detected JSON parser, and afterPipe %s', - async (afterPipe: boolean) => { + test.each([ + [true, true], + [false, true], + [true, false], + [false, false], + ])( + 'Returns completion options when the situation is AFTER_SELECTOR, detected JSON parser, afterPipe %s, and hasSpace: %s', + async (afterPipe: boolean, hasSpace: boolean) => { jest.spyOn(completionProvider, 'getParserAndLabelKeys').mockResolvedValue({ extractedLabelKeys, hasJSON: true, hasLogfmt: false, }); - const situation: Situation = { type: 'AFTER_SELECTOR', logQuery: '', afterPipe }; + const situation: Situation = { type: 'AFTER_SELECTOR', logQuery: '', afterPipe, hasSpace }; const completions = await getCompletions(situation, completionProvider); - const expected = buildAfterSelectorCompletions('json', 'logfmt', afterPipe); + const expected = buildAfterSelectorCompletions('json', 'logfmt', afterPipe, hasSpace); expect(completions).toEqual(expected); } ); test.each([true, false])( - 'Returns completion options when the situation is AFTER_SELECTOR, detected Logfmt parser, and afterPipe %s', + 'Returns completion options when the situation is AFTER_SELECTOR, detected Logfmt parser, afterPipe %s, and hasSpace: %s', async (afterPipe: boolean) => { jest.spyOn(completionProvider, 'getParserAndLabelKeys').mockResolvedValue({ extractedLabelKeys, hasJSON: false, hasLogfmt: true, }); - const situation: Situation = { type: 'AFTER_SELECTOR', logQuery: '', afterPipe }; + const situation: Situation = { type: 'AFTER_SELECTOR', logQuery: '', afterPipe, hasSpace: true }; const completions = await getCompletions(situation, completionProvider); - const expected = buildAfterSelectorCompletions('logfmt', 'json', afterPipe); + const expected = buildAfterSelectorCompletions('logfmt', 'json', afterPipe, true); expect(completions).toEqual(expected); } ); diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts index 2f8e0a83ca5..4e90f2a0bae 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/completions.ts @@ -158,14 +158,13 @@ async function getInGroupingCompletions(logQuery: string, dataProvider: Completi const PARSERS = ['json', 'logfmt', 'pattern', 'regexp', 'unpack']; async function getParserCompletions( - afterPipe: boolean, + prefix: string, hasJSON: boolean, hasLogfmt: boolean, extractedLabelKeys: string[] ) { const allParsers = new Set(PARSERS); const completions: Completion[] = []; - const prefix = afterPipe ? ' ' : '| '; const hasLevelInExtractedLabels = extractedLabelKeys.some((key) => key === 'level'); if (hasJSON) { @@ -210,13 +209,13 @@ async function getParserCompletions( async function getAfterSelectorCompletions( logQuery: string, afterPipe: boolean, + hasSpace: boolean, dataProvider: CompletionDataProvider ): Promise { const { extractedLabelKeys, hasJSON, hasLogfmt } = await dataProvider.getParserAndLabelKeys(logQuery); - const completions: Completion[] = await getParserCompletions(afterPipe, hasJSON, hasLogfmt, extractedLabelKeys); - - const prefix = afterPipe ? ' ' : '| '; + const prefix = `${hasSpace ? '' : ' '}${afterPipe ? '' : '| '}`; + const completions: Completion[] = await getParserCompletions(prefix, hasJSON, hasLogfmt, extractedLabelKeys); extractedLabelKeys.forEach((key) => { completions.push({ @@ -249,7 +248,9 @@ async function getAfterSelectorCompletions( documentation: explainOperator(LokiOperationId.LabelFormat), }); - const lineFilters = getLineFilterCompletions(afterPipe); + // With a space between the pipe and the cursor, we omit line filters + // E.g. `{label="value"} | ` + const lineFilters = afterPipe && hasSpace ? [] : getLineFilterCompletions(afterPipe); return [...lineFilters, ...completions]; } @@ -307,7 +308,7 @@ export async function getCompletions( dataProvider ); case 'AFTER_SELECTOR': - return getAfterSelectorCompletions(situation.logQuery, situation.afterPipe, dataProvider); + return getAfterSelectorCompletions(situation.logQuery, situation.afterPipe, situation.hasSpace, dataProvider); case 'AFTER_UNWRAP': return getAfterUnwrapCompletions(situation.logQuery, dataProvider); case 'IN_AGGREGATION': diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts index 41aa1c50d0b..7541f563fb4 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.test.ts @@ -42,6 +42,7 @@ describe('situation', () => { assertSituation('{level="info"} ^', { type: 'AFTER_SELECTOR', afterPipe: false, + hasSpace: true, logQuery: '{level="info"}', }); @@ -54,18 +55,21 @@ describe('situation', () => { assertSituation('{level="info"} | json ^', { type: 'AFTER_SELECTOR', afterPipe: false, + hasSpace: true, logQuery: '{level="info"} | json', }); - assertSituation('{level="info"} | json | ^', { + assertSituation('{level="info"} | json |^', { type: 'AFTER_SELECTOR', afterPipe: true, + hasSpace: false, logQuery: '{level="info"} | json |', }); assertSituation('count_over_time({level="info"}^[10s])', { type: 'AFTER_SELECTOR', afterPipe: false, + hasSpace: false, logQuery: '{level="info"}', }); @@ -76,18 +80,21 @@ describe('situation', () => { assertSituation('count_over_time({level="info"}^)', { type: 'AFTER_SELECTOR', afterPipe: false, + hasSpace: false, logQuery: '{level="info"}', }); assertSituation('{level="info"} |= "a" | logfmt ^', { type: 'AFTER_SELECTOR', afterPipe: false, + hasSpace: true, logQuery: '{level="info"} |= "a" | logfmt', }); assertSituation('sum(count_over_time({place="luna"} | logfmt |^)) by (place)', { type: 'AFTER_SELECTOR', afterPipe: true, + hasSpace: false, logQuery: '{place="luna"}| logfmt |', }); }); diff --git a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts index 2eef2f782fe..fbee96313ba 100644 --- a/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts +++ b/public/app/plugins/datasource/loki/components/monaco-query-field/monaco-completion-provider/situation.ts @@ -119,6 +119,7 @@ export type Situation = | { type: 'AFTER_SELECTOR'; afterPipe: boolean; + hasSpace: boolean; logQuery: string; } | { @@ -450,6 +451,7 @@ function resolveLogOrLogRange(node: SyntaxNode, text: string, pos: number, after return { type: 'AFTER_SELECTOR', afterPipe, + hasSpace: text.endsWith(' '), logQuery: getLogQueryFromMetricsQuery(text).trim(), }; }