[ES|QL] Pretty-printing support for GROK and DISSECT commands (el…

…astic#199646) ## Summary Closes elastic#199480 This fixes `GROK` and `DISSECT` command pretty-printing, in, both, `BasicPrettyPrinter` and `WrappingPrettyPrinting`. The two commands are special, because unlike all other commands they exhibit these two traits: 1. Command arguments are not separated by comma (in all other commands arguments are separated by comma). 2. Command option label is separated by `=` from the option value (in all other commands it is just a space). ``` DISSECT input "pattern" APPEND_SEPARATOR = "separator" | | | | no comma? | | equals sign? ``` ### Checklist Delete any items that are not applicable to this PR. - [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 ### For maintainers - [x] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)
tsullivan · Nov 11, 2024 · 2277e64 · 2277e64
1 parent c611e52
commit 2277e64
Show file tree

Hide file tree

Showing 5 changed files with 169 additions and 15 deletions.
diff --git a/packages/kbn-esql-ast/src/pretty_print/__tests__/basic_pretty_printer.test.ts b/packages/kbn-esql-ast/src/pretty_print/__tests__/basic_pretty_printer.test.ts
@@ -78,15 +78,6 @@ describe('single line query', () => {
       });
     });
 
-    describe('SHOW', () => {
-      /** @todo Enable once show command args are parsed as columns.  */
-      test.skip('info page', () => {
-        const { text } = reprint('SHOW info');
-
-        expect(text).toBe('SHOW info');
-      });
-    });
-
     describe('STATS', () => {
       test('with aggregates assignment', () => {
         const { text } = reprint('FROM a | STATS var = agg(123, fn(true))');
@@ -100,6 +91,30 @@ describe('single line query', () => {
         expect(text).toBe('FROM a | STATS A(1), B(2) BY asdf');
       });
     });
+
+    describe('GROK', () => {
+      test('two basic arguments', () => {
+        const { text } = reprint('FROM search-movies | GROK Awards "text"');
+
+        expect(text).toBe('FROM search-movies | GROK Awards "text"');
+      });
+    });
+
+    describe('DISSECT', () => {
+      test('two basic arguments', () => {
+        const { text } = reprint('FROM index | DISSECT input "pattern"');
+
+        expect(text).toBe('FROM index | DISSECT input "pattern"');
+      });
+
+      test('with APPEND_SEPARATOR option', () => {
+        const { text } = reprint(
+          'FROM index | DISSECT input "pattern" APPEND_SEPARATOR="<separator>"'
+        );
+
+        expect(text).toBe('FROM index | DISSECT input "pattern" APPEND_SEPARATOR = "<separator>"');
+      });
+    });
   });
 
   describe('expressions', () => {

diff --git a/packages/kbn-esql-ast/src/pretty_print/__tests__/wrapping_pretty_printer.test.ts b/packages/kbn-esql-ast/src/pretty_print/__tests__/wrapping_pretty_printer.test.ts
@@ -19,6 +19,83 @@ const reprint = (src: string, opts?: WrappingPrettyPrinterOptions) => {
   return { text };
 };
 
+describe('commands', () => {
+  describe('GROK', () => {
+    test('two basic arguments', () => {
+      const { text } = reprint('FROM search-movies | GROK Awards "text"');
+
+      expect(text).toBe('FROM search-movies | GROK Awards "text"');
+    });
+
+    test('two long arguments', () => {
+      const { text } = reprint(
+        'FROM search-movies | GROK AwardsAwardsAwardsAwardsAwardsAwardsAwardsAwards "texttexttexttexttexttexttexttexttexttexttexttexttexttexttext"'
+      );
+
+      expect('\n' + text).toBe(`
+FROM search-movies
+  | GROK
+      AwardsAwardsAwardsAwardsAwardsAwardsAwardsAwards
+      "texttexttexttexttexttexttexttexttexttexttexttexttexttexttext"`);
+    });
+  });
+
+  describe('DISSECT', () => {
+    test('two basic arguments', () => {
+      const { text } = reprint('FROM index | DISSECT input "pattern"');
+
+      expect(text).toBe('FROM index | DISSECT input "pattern"');
+    });
+
+    test('two long arguments', () => {
+      const { text } = reprint(
+        'FROM index | DISSECT InputInputInputInputInputInputInputInputInputInputInputInputInputInput "PatternPatternPatternPatternPatternPatternPatternPatternPatternPattern"'
+      );
+
+      expect('\n' + text).toBe(`
+FROM index
+  | DISSECT
+      InputInputInputInputInputInputInputInputInputInputInputInputInputInput
+      "PatternPatternPatternPatternPatternPatternPatternPatternPatternPattern"`);
+    });
+
+    test('with APPEND_SEPARATOR option', () => {
+      const { text } = reprint(
+        'FROM index | DISSECT input "pattern" APPEND_SEPARATOR="<separator>"'
+      );
+
+      expect(text).toBe('FROM index | DISSECT input "pattern" APPEND_SEPARATOR = "<separator>"');
+    });
+
+    test('two long arguments with short APPEND_SEPARATOR option', () => {
+      const { text } = reprint(
+        'FROM index | DISSECT InputInputInputInputInputInputInputInputInputInputInputInputInputInput "PatternPatternPatternPatternPatternPatternPatternPatternPatternPattern" APPEND_SEPARATOR="sep"'
+      );
+
+      expect('\n' + text).toBe(`
+FROM index
+  | DISSECT
+      InputInputInputInputInputInputInputInputInputInputInputInputInputInput
+      "PatternPatternPatternPatternPatternPatternPatternPatternPatternPattern"
+        APPEND_SEPARATOR = "sep"`);
+    });
+
+    test('two long arguments with long APPEND_SEPARATOR option', () => {
+      const { text } = reprint(
+        'FROM index | DISSECT InputInputInputInputInputInputInputInputInputInputInputInputInputInput "PatternPatternPatternPatternPatternPatternPatternPatternPatternPattern" APPEND_SEPARATOR="<SeparatorSeparatorSeparatorSeparatorSeparatorSeparatorSeparatorSeparator>"'
+      );
+
+      expect('\n' + text).toBe(`
+FROM index
+  | DISSECT
+      InputInputInputInputInputInputInputInputInputInputInputInputInputInput
+      "PatternPatternPatternPatternPatternPatternPatternPatternPatternPattern"
+        APPEND_SEPARATOR =
+          "<SeparatorSeparatorSeparatorSeparatorSeparatorSeparatorSeparatorSeparator>"`);
+    });
+  });
+});
+
 describe('casing', () => {
   test('can chose command name casing', () => {
     const query = 'FROM index | WHERE a == 123';

diff --git a/packages/kbn-esql-ast/src/pretty_print/basic_pretty_printer.ts b/packages/kbn-esql-ast/src/pretty_print/basic_pretty_printer.ts
@@ -19,6 +19,7 @@ import {
 import { ESQLAstBaseItem, ESQLAstCommand, ESQLAstQueryExpression } from '../types';
 import { ESQLAstExpressionNode, Visitor } from '../visitor';
 import { resolveItem } from '../visitor/utils';
+import { commandOptionsWithEqualsSeparator, commandsWithNoCommaArgSeparator } from './constants';
 import { LeafPrinter } from './leaf_printer';
 
 export interface BasicPrettyPrinterOptions {
@@ -378,7 +379,8 @@ export class BasicPrettyPrinter {
         args += (args ? ', ' : '') + arg;
       }
 
-      const argsFormatted = args ? ` ${args}` : '';
+      const separator = commandOptionsWithEqualsSeparator.has(ctx.node.name) ? ' = ' : ' ';
+      const argsFormatted = args ? `${separator}${args}` : '';
       const optionFormatted = `${option}${argsFormatted}`;
 
       return optionFormatted;
@@ -392,7 +394,10 @@ export class BasicPrettyPrinter {
       let options = '';
 
       for (const source of ctx.visitArguments()) {
-        args += (args ? ', ' : '') + source;
+        const needsSeparator = !!args;
+        const needsComma = !commandsWithNoCommaArgSeparator.has(ctx.node.name);
+        const separator = needsSeparator ? (needsComma ? ',' : '') + ' ' : '';
+        args += separator + source;
       }
 
       for (const option of ctx.visitOptions()) {

diff --git a/packages/kbn-esql-ast/src/pretty_print/constants.ts b/packages/kbn-esql-ast/src/pretty_print/constants.ts
@@ -0,0 +1,52 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the "Elastic License
+ * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
+ * Public License v 1"; you may not use this file except in compliance with, at
+ * your election, the "Elastic License 2.0", the "GNU Affero General Public
+ * License v3.0 only", or the "Server Side Public License, v 1".
+ */
+
+/**
+ * This set tracks commands that don't use commas to separate their
+ * arguments.
+ *
+ * Normally ES|QL command arguments are separated by commas.
+ *
+ * ```
+ * COMMAND arg1, arg2, arg3
+ * ```
+ *
+ * But there are some commands (namely `grok` and `dissect`) which don't
+ * use commas to separate their arguments.
+ *
+ * ```
+ * GROK input "pattern"
+ * DISSECT input "pattern"
+ * ```
+ */
+export const commandsWithNoCommaArgSeparator = new Set(['grok', 'dissect']);
+
+/**
+ * This set tracks command options which use an equals sign to separate
+ * the option label from the option value.
+ *
+ * Most ES|QL commands use a space to separate the option label from the
+ * option value.
+ *
+ * ```
+ * COMMAND arg1, arg2, arg3 OPTION option
+ * FROM index METADATA _id
+ * ```
+ *
+ * However, the `APPEND_SEPARATOR` in the `DISSECT` command uses an equals
+ * sign to separate the option label from the option value.
+ *
+ * ```
+ * DISSECT input "pattern" APPEND_SEPARATOR = "separator"
+ *                                          |
+ *                                          |
+ *                                          equals sign
+ * ```
+ */
+export const commandOptionsWithEqualsSeparator = new Set(['append_separator']);
diff --git a/packages/kbn-esql-ast/src/pretty_print/wrapping_pretty_printer.ts b/packages/kbn-esql-ast/src/pretty_print/wrapping_pretty_printer.ts
@@ -20,6 +20,7 @@ import {
 } from '../visitor';
 import { children, singleItems } from '../visitor/utils';
 import { BasicPrettyPrinter, BasicPrettyPrinterOptions } from './basic_pretty_printer';
+import { commandOptionsWithEqualsSeparator, commandsWithNoCommaArgSeparator } from './constants';
 import { getPrettyPrintStats } from './helpers';
 import { LeafPrinter } from './leaf_printer';
 
@@ -259,6 +260,8 @@ export class WrappingPrettyPrinter {
       }
     }
 
+    const commaBetweenArgs = !commandsWithNoCommaArgSeparator.has(ctx.node.name);
+
     if (!oneArgumentPerLine) {
       ARGS: for (const arg of singleItems(ctx.arguments())) {
         if (arg.type === 'option') {
@@ -271,7 +274,8 @@ export class WrappingPrettyPrinter {
         if (formattedArgLength > largestArg) {
           largestArg = formattedArgLength;
         }
-        let separator = txt ? ',' : '';
+
+        let separator = txt ? (commaBetweenArgs ? ',' : '') : '';
         let fragment = '';
 
         if (needsWrap) {
@@ -329,7 +333,7 @@ export class WrappingPrettyPrinter {
         const arg = ctx.visitExpression(args[i], {
           indent,
           remaining: this.opts.wrap - indent.length,
-          suffix: isLastArg ? '' : ',',
+          suffix: isLastArg ? '' : commaBetweenArgs ? ',' : '',
         });
         const separator = isFirstArg ? '' : '\n';
         const indentation = arg.indented ? '' : indent;
@@ -557,8 +561,9 @@ export class WrappingPrettyPrinter {
         indent: inp.indent,
         remaining: inp.remaining - option.length - 1,
       });
-      const argsFormatted = args.txt ? ` ${args.txt}` : '';
-      const txt = `${option}${argsFormatted}`;
+      const argsFormatted = args.txt ? `${args.txt[0] === '\n' ? '' : ' '}${args.txt}` : '';
+      const separator = commandOptionsWithEqualsSeparator.has(ctx.node.name) ? ' =' : '';
+      const txt = `${option}${separator}${argsFormatted}`;
 
       return { txt, lines: args.lines };
     })