fix: calculates remaining space for options to avoid line breaking (#64)

partially addresses elastic/kibana#257200

## Summary

This PR changes how the remaining space on the line of a command is
calculated to decide when to break options into a new line or not.

## Details

**Before the fix:** The decision to break command options (e.g. STATS’s
BY ip) to a new line used this condition: `options.length > opts.wrap -
inp.remaining - cmd.length - 1 - args.txt.length`. For every command,
inp.remaining was set to the same value as opts.wrap (e.g. 500), so
`opts.wrap - inp.remaining` was always 0. The “remaining width” for
options became negative (e.g. 0 − 6 − 1 − 85 = −92), and so
`options.length > remaining` was always true (e.g. 5 > −92). As a
result, options were always broken to a new line, regardless of wrap.
Example: with `{ wrap: 120 and multiline: true }`, a STATS line like `|
STATS count = COUNT(*), avg = AVG(bytes), p95 = PERCENTILE(bytes, 95),
ext = VALUES(tags.keyword) BY ip` would still be split so that `BY ip`
appeared on the next line, even though it would have fit on the same
line.

**After the fix:** The code now computes how much of the line is already
used by the prefix (indent + pipe tab + "| " + command name + space +
args) and treats the rest as the width available for options. So
`linePrefixLength = indent.length + pipeTab.length + 2 + cmd.length + 1
+ args.txt.length`, and `remainingForOptions = opts.wrap -
linePrefixLength`. Options are broken only when `options.length >
remainingForOptions`. Example: with `{ wrap: 120 }`, a prefix of length
96 leaves 24 characters for options; "BY ip" has length 5, so 5 > 24 is
false and BY stays on the same line as STATS. With `{ wrap: 80 }`, the
same prefix might leave no room (or negative room), so 5 >
remainingForOptions becomes true and BY is correctly broken to a new
line. So the new structure represents “space left on the current line
for options” and only breaks when the options text would not fit in that
space.

### Checklist

- [x] Unit tests have been added or updated.
- [x] The PR title has the correct [conventional
commit](https://www.conventionalcommits.org/en/v1.0.0/) label in the
title, this is crucial for the correct versioning of this package. Check
the following cheat shit.
  | Title Label | Release |
  |---|---|
  | `breaking: true (!)` | `major` |
  | `feat` | `minor` |
  | `fix` | `patch` |
  | `refactor` | `patch` |
  | `perf` | `patch` |
  | `build` | `patch` |
  | `docs`  | `patch` |
  | `chore` | `patch` |
  | `revert` | `patch` |

Author: momovdg

src/pretty_print/__tests__/wrapping_pretty_printer.comments.test.ts

@@ -1024,15 +1024,13 @@ FROM index`;
   /* before subquery */ (/* inside start */ FROM index2 /* after source */
     | WHERE a > 10 /* after where */
     | EVAL b = a * 2
-    | STATS cnt = COUNT(*)
-          BY c
+    | STATS cnt = COUNT(*) BY c
     | SORT cnt DESC
     | LIMIT 10) /* after first subquery */,
   index3,
   (FROM index4 | STATS COUNT(*)) /* after second */
   | WHERE d > 10
-  | STATS max = MAX(*)
-        BY e
+  | STATS max = MAX(*) BY e
   | SORT max DESC`;
 
       assertReprint(query, expected);

src/pretty_print/__tests__/wrapping_pretty_printer.test.ts

@@ -5,14 +5,14 @@
  * 2.0.
  */
 
-import { parse } from '../../parser';
+import { Parser } from '../../parser';
 import type { ESQLMap } from '../../types';
 import { Walker } from '../../ast/walker';
 import type { WrappingPrettyPrinterOptions } from '../wrapping_pretty_printer';
 import { WrappingPrettyPrinter } from '../wrapping_pretty_printer';
 
 const reprint = (src: string, opts?: WrappingPrettyPrinterOptions) => {
-  const { root } = parse(src);
+  const { root } = Parser.parse(src);
   const text = WrappingPrettyPrinter.print(root, opts);
 
   // console.log(JSON.stringify(root.commands, null, 2));
@@ -230,8 +230,7 @@ FROM index
 
       expect(text).toBe(
         `FROM k8s
-  | STATS count = COUNT()
-        BY @timestamp = BUCKET(@timestamp, 1 MINUTE)
+  | STATS count = COUNT() BY @timestamp = BUCKET(@timestamp, 1 MINUTE)
   | CHANGE_POINT count
         ON @timestamp
         AS type, pvalue
@@ -329,8 +328,7 @@ FROM index
       expect(text).toBe(`FROM index
   | FORK
       (
-          STATS count = COUNT()
-          BY category
+          STATS count = COUNT() BY category
         | WHERE count > 10
         | SORT count DESC
       )
@@ -782,6 +780,29 @@ FROM index
 -   | LIMIT 10`);
     });
 
+    test('STATS with multiple agg fields and BY: with default wrap, breaks at BY when line exceeds width', () => {
+      const query = `FROM kibana_sample_data_logs
+  | STATS count = COUNT(*), avg = AVG(bytes), p95 = PERCENTILE(bytes, 95), ext = VALUES(tags.keyword) BY ip
+  | EVAL newField = CASE(count < 100, "groupA", count > 100 AND count < 500, "groupB", "Other")
+  | KEEP newField`;
+      const text = reprint(query).text;
+      // With default wrap (80), STATS args overflow so options (BY) are broken to a new line
+      expect(text).toMatch(/\|\s*STATS[\s\S]*?\n\s*BY ip/m);
+    });
+
+    test('STATS with multiple agg fields and BY: with large wrap, query fits on one line so BY stays on same line as STATS', () => {
+      const query = `FROM kibana_sample_data_logs
+      | STATS count = COUNT(*), avg = AVG(bytes), p95 = PERCENTILE(bytes, 95), ext = VALUES(tags.keyword) BY ip
+      | EVAL newField = CASE(count < 100, "groupA", count > 100 AND count < 500, "groupB", "Other")
+      | KEEP newField`;
+      const text = reprint(query, { wrap: 120, multiline: true }).text;
+
+      // With multiline: true and wrap 120, STATS line fits so BY is not broken to a new line
+      const statsLine = text.split('\n').find((l) => l.includes('STATS') && l.includes('BY ip'));
+      expect(statsLine).toBeDefined();
+      expect(statsLine!.trim()).toMatch(/^\| STATS .+ BY ip$/);
+    });
+
     test('wraps function list', () => {
       const query = `
 FROM index
@@ -940,7 +961,7 @@ FROM index
   describe('map expression', () => {
     test('empty map', () => {
       const src = `ROW F(0, {"a": 0})`;
-      const { root } = parse(src);
+      const { root } = Parser.parse(src);
       const map = Walker.match(root, { type: 'map' }) as ESQLMap;
 
       map.entries = [];
@@ -956,7 +977,7 @@ FROM index
 
     test('empty map (multiline)', () => {
       const src = `ROW F(0, {"a": 0}) | LIMIT 1`;
-      const { root } = parse(src);
+      const { root } = Parser.parse(src);
       const map = Walker.match(root, { type: 'map' }) as ESQLMap;
 
       map.entries = [];
@@ -1503,15 +1524,13 @@ describe('subqueries (parens)', () => {
   (FROM index2
     | WHERE a > 10
     | EVAL b = a * 2
-    | STATS cnt = COUNT(*)
-          BY c
+    | STATS cnt = COUNT(*) BY c
     | SORT cnt DESC
     | LIMIT 10),
   index3,
   (FROM index4 | STATS COUNT(*))
   | WHERE d > 10
-  | STATS max = MAX(*)
-        BY e
+  | STATS max = MAX(*) BY e
   | SORT max DESC`
     );
   });

src/pretty_print/wrapping_pretty_printer.ts

@@ -915,11 +915,20 @@ export class WrappingPrettyPrinter {
         options += (options ? ' ' : '') + out.txt;
       }
 
+      // Remaining width for options on the current line: wrap minus (indent + "| " + cmd + " " + args)
+      const pipeTab = opts.pipeTab ?? '  ';
+      const pipeAndSpaceLength = '| '.length;
+      const spaceBetweenCmdAndArgsLength = 1;
+      const linePrefixLength =
+        inp.indent.length +
+        pipeTab.length +
+        pipeAndSpaceLength +
+        cmd.length +
+        spaceBetweenCmdAndArgsLength +
+        args.txt.length;
+      const remainingForOptions = opts.wrap - linePrefixLength;
       breakOptions =
-        breakOptions ||
-        args.lines > 1 ||
-        optionsLines > 1 ||
-        options.length > opts.wrap - inp.remaining - cmd.length - 1 - args.txt.length;
+        breakOptions || args.lines > 1 || optionsLines > 1 || options.length > remainingForOptions;
 
       if (breakOptions) {
         options = optionsTxt.join('\n' + optionIndent);