Feat: format output from toExpression (elastic#663)

* chore: minor code refactor

* feat: change the function separator

use newlines before the pipes, instead of just keeping everything on 1 line

* feat: naive argument formatting

put arguments on new line when existing line is over 80 characters

* test: fix the toExpression output check

* chore: update element expressions

match the new formatting rules so changing values from the sidebar is less jarring

* fix: handle nested formatting differently

* fix: don't wrap subexpression args

* chore: typo fix

* chore: attempt to make arg value formatting clearer

break up the nested ternary, add comments

common/lib/__tests__/ast.toExpression.js

@@ -457,10 +457,10 @@ describe('ast toExpression', () => {
 
       const expression = toExpression(astObj);
       const expected = [
-        'csv input="year,make,model,price',
+        'csv \n  input="year,make,model,price',
         '2016,honda,cr-v,23845',
         '2016,honda,fit,15890,',
-        '2016,honda,civic,18640" | line x={distinct f="year"} y={sum f="price"} colors={distinct f="model"}',
+        '2016,honda,civic,18640"\n| line x={distinct f="year"} y={sum f="price"} colors={distinct f="model"}',
       ];
       expect(expression).to.equal(expected.join('\n'));
     });
@@ -551,11 +551,11 @@ describe('ast toExpression', () => {
 
       const expression = toExpression(astObj);
       const expected = [
-        'csv input="year,make,model,price',
+        'csv \n  input="year,make,model,price',
         '2016,honda,cr-v,23845',
         '2016,honda,fit,15890,',
-        '2016,honda,civic,18640" | pointseries x={distinct f="year"} y={sum f="price"} ' +
-          'colors={distinct f="model"} | line pallette={getColorPallette name="elastic"}',
+        '2016,honda,civic,18640"\n| pointseries x={distinct f="year"} y={sum f="price"} ' +
+          'colors={distinct f="model"}\n| line pallette={getColorPallette name="elastic"}',
       ];
       expect(expression).to.equal(expected.join('\n'));
     });

common/lib/ast.js

@@ -1,49 +1,73 @@
 import { getType } from '../lib/get_type';
 import { parse } from './grammar';
 
-function getArgumentString(arg, argKey) {
+function getArgumentString(arg, argKey, level = 0) {
   const type = getType(arg);
 
-  function maybeArgKey(argString) {
+  function maybeArgKey(argKey, argString) {
     return argKey == null || argKey === '_' ? argString : `${argKey}=${argString}`;
   }
 
-  function escapeSpecialCharacters(string) {
-    return string.replace(/[\\"]/g, '\\$&'); // $& means the whole matched string
+  if (type === 'string') {
+    // correctly (re)escape double quotes
+    const escapedArg = arg.replace(/[\\"]/g, '\\$&'); // $& means the whole matched string
+    return maybeArgKey(argKey, `"${escapedArg}"`);
   }
 
-  if (type === 'string') return maybeArgKey(`"${escapeSpecialCharacters(arg)}"`);
-  if (type === 'boolean' || type === 'null' || type === 'number') return maybeArgKey(`${arg}`);
-  if (type === 'expression') return maybeArgKey(`{${getExpression(arg.chain)}}`);
+  if (type === 'boolean' || type === 'null' || type === 'number') {
+    // use values directly
+    return maybeArgKey(argKey, `${arg}`);
+  }
+
+  if (type === 'expression') {
+    // build subexpressions
+    return maybeArgKey(argKey, `{${getExpression(arg.chain, level + 1)}}`);
+  }
 
+  // unknown type, throw with type value
   throw new Error(`Invalid argument type in AST: ${type}`);
 }
 
-function getExpressionArgs(block) {
+function getExpressionArgs(block, level = 0) {
   const args = block.arguments;
+  const hasValidArgs = typeof args === 'object' && args != null && !Array.isArray(args);
 
-  if (!hasValidateArgs(args)) throw new Error('Arguments can only be an object');
+  if (!hasValidArgs) throw new Error('Arguments can only be an object');
 
   const argKeys = Object.keys(args);
-  return argKeys.map(argKey => {
-    const multiArgs = args[argKey];
+  const MAX_LINE_LENGTH = 80; // length before wrapping arguments
+  return argKeys.map(argKey =>
+    args[argKey].reduce((acc, arg) => {
+      const argString = getArgumentString(arg, argKey, level);
+      const lineLength = acc.split('\n').pop().length;
+
+      // if arg values are too long, move it to the next line
+      if (level === 0 && lineLength + argString.length > MAX_LINE_LENGTH) {
+        return `${acc}\n  ${argString}`;
+      }
 
-    return multiArgs.reduce((acc, arg) => acc.concat(getArgumentString(arg, argKey)), []).join(' ');
-  });
-}
+      // append arg values to existing arg values
+      if (lineLength > 0) {
+        return `${acc} ${argString}`;
+      }
 
-function hasValidateArgs(args) {
-  return typeof args === 'object' && args != null && !Array.isArray(args);
+      // start the accumulator with the first arg value
+      return argString;
+    }, '')
+  );
 }
 
 function fnWithArgs(fnName, args) {
   if (!args || args.length === 0) return fnName;
   return `${fnName} ${args.join(' ')}`;
 }
 
-function getExpression(chain) {
+function getExpression(chain, level = 0) {
   if (!chain) throw new Error('Expressions must contain a chain');
 
+  // break new functions onto new lines if we're not in a nested/sub-expression
+  const separator = level > 0 ? ' | ' : '\n| ';
+
   return chain
     .map(chainObj => {
       const type = getType(chainObj);
@@ -52,12 +76,12 @@ function getExpression(chain) {
         const fn = chainObj.function;
         if (!fn || fn.length === 0) throw new Error('Functions must have a function name');
 
-        const expArgs = getExpressionArgs(chainObj);
+        const expArgs = getExpressionArgs(chainObj, level);
 
         return fnWithArgs(fn, expArgs);
       }
     }, [])
-    .join(' | ');
+    .join(separator);
 }
 
 export function fromExpression(expression, type = 'expression') {

public/elements/debug/debug.js

@@ -5,5 +5,6 @@ export const debug = () => ({
   displayName: 'Debug',
   help: 'Just dumps the configuration of the element',
   image: header,
-  expression: 'demodata | render as=debug',
+  expression: `demodata
+| render as=debug`,
 });

public/elements/dropdown_filter/index.js

@@ -6,6 +6,7 @@ export const dropdownFilter = () => ({
   help: 'A dropdown from which you can select values for an "exactly" filter',
   image: header,
   height: 50,
-  expression: 'demodata | dropdownControl valueColumn=project filterColumn=project',
+  expression: `demodata
+| dropdownControl valueColumn=project filterColumn=project`,
   filter: '',
 });

public/elements/image/image.js

@@ -5,5 +5,6 @@ export const image = () => ({
   displayName: 'Image',
   help: 'A static image.',
   image: header,
-  expression: 'image mode="contain" | render',
+  expression: `image mode="contain"
+| render`,
 });

public/elements/markdown/markdown.js

@@ -5,7 +5,9 @@ export const markdown = () => ({
   displayName: 'Markdown',
   help: 'Markup from Markdown',
   image: header,
-  expression: `filters | demodata | markdown "### Welcome to the Markdown Element.
+  expression: `filters
+| demodata
+| markdown "### Welcome to the Markdown Element.
 
 Good news! You're already connected to some demo data!
 

public/elements/pie/pie.js

@@ -7,5 +7,9 @@ export const pie = () => ({
   height: 300,
   help: 'An customizable element for making pie charts from your data',
   image: header,
-  expression: 'filters | demodata | pointseries color="state" size="max(price)" | pie | render',
+  expression: `filters
+| demodata
+| pointseries color="state" size="max(price)"
+| pie
+| render`,
 });

public/elements/plot/plot.js

@@ -5,6 +5,9 @@ export const plot = () => ({
   displayName: 'Coordinate plot',
   help: 'An customizable XY plot for making line, bar or dot charts from your data',
   image: header,
-  expression:
-    'filters | demodata | pointseries x="time" y="sum(price)" color="state" | plot defaultStyle={seriesStyle points=5} | render',
+  expression: `filters
+| demodata
+| pointseries x="time" y="sum(price)" color="state"
+| plot defaultStyle={seriesStyle points=5}
+| render`,
 });

public/elements/repeatImage/index.js

@@ -5,5 +5,9 @@ export const repeatImage = () => ({
   displayName: 'Image Repeat',
   help: 'Repeats an image N times',
   image: header,
-  expression: 'demodata | pointseries size="mean(cost)" | getCell | repeatImage | render',
+  expression: `demodata
+| pointseries size="mean(cost)"
+| getCell
+| repeatImage
+| render`,
 });

public/elements/revealImage/index.js

@@ -5,6 +5,9 @@ export const revealImage = () => ({
   displayName: 'Image Reveal',
   help: 'Reveals a percentage of an image',
   image: header,
-  expression:
-    'demodata | pointseries size="sum(min(cost) / max(cost))" | getCell | revealImage origin=bottom | render',
+  expression: `demodata
+| pointseries size="sum(min(cost) / max(cost))"
+| getCell
+| revealImage origin=bottom
+| render`,
 });

public/elements/table/table.js

@@ -5,5 +5,8 @@ export const table = () => ({
   displayName: 'Data Table',
   help: 'A scrollable grid for displaying data in a tabular format',
   image: header,
-  expression: 'filters | demodata | table | render',
+  expression: `filters
+| demodata
+| table
+| render`,
 });

public/elements/time_filter/time_filter.js

@@ -6,6 +6,7 @@ export const timeFilter = () => ({
   help: 'Set a time window',
   image: header,
   height: 50,
-  expression: 'timefilterControl compact=true column=@timestamp | render as=time_filter',
+  expression: `timefilterControl compact=true column=@timestamp
+| render as=time_filter`,
   filter: 'timefilter column=@timestamp from=now-24h to=now',
 });