util: improve .inspect() array grouping

This improves a couple minor things:

* Arrays that contain entries other than `number` or `bigint` are
  ordered to the left instead of the right.
* The bias towards more columns got increased. That mainly increases
  the number of columns for arrays that contain lots of short entries.
* Columns are now more dense in case they would otherwise have extra
  whitespace in-between two columns.
* The maximum columns got increased from 10 to 15.
* The maximum number of columns per `compact` was increased from
  3 to 4.

PR-URL: https://github.com/nodejs/node/pull/28070
Refs: https://github.com/nodejs/node/issues/27690
Reviewed-By: James M Snell <jasnell@gmail.com>

1 files changed, 48 insertions, 26 deletions

diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js
index e9dc8d83ea..ed68ebed16 100644
--- a/lib/internal/util/inspect.js
+++ b/lib/internal/util/inspect.js
@@ -800,7 +800,7 @@ function formatRaw(ctx, value, recurseTimes, typedArray) {
   }
 
   const res = reduceToSingleString(
-    ctx, output, base, braces, extrasType, recurseTimes);
+    ctx, output, base, braces, extrasType, recurseTimes, value);
   const budget = ctx.budget[ctx.indentationLvl] || 0;
   const newLength = budget + res.length;
   ctx.budget[ctx.indentationLvl] = newLength;
@@ -965,7 +965,7 @@ function formatError(err, constructor, tag, ctx) {
   return stack;
 }
 
-function groupArrayElements(ctx, output) {
+function groupArrayElements(ctx, output, value) {
   let totalLength = 0;
   let maxLength = 0;
   let i = 0;
@@ -997,51 +997,73 @@ function groupArrayElements(ctx, output) {
       (totalLength / actualMax > 5 || maxLength <= 6)) {
 
     const approxCharHeights = 2.5;
-    const bias = 1;
+    const biasedMax = Math.max(actualMax - 4, 1);
     // Dynamically check how many columns seem possible.
     const columns = Math.min(
       // Ideally a square should be drawn. We expect a character to be about 2.5
       // times as high as wide. This is the area formula to calculate a square
       // which contains n rectangles of size `actualMax * approxCharHeights`.
       // Divide that by `actualMax` to receive the correct number of columns.
-      // The added bias slightly increases the columns for short entries.
+      // The added bias increases the columns for short entries.
       Math.round(
         Math.sqrt(
-          approxCharHeights * (actualMax - bias) * outputLength
-        ) / (actualMax - bias)
+          approxCharHeights * biasedMax * outputLength
+        ) / biasedMax
       ),
       // Do not exceed the breakLength.
       Math.floor((ctx.breakLength - ctx.indentationLvl) / actualMax),
       // Limit array grouping for small `compact` modes as the user requested
       // minimal grouping.
-      ctx.compact * 3,
-      // Limit the columns to a maximum of ten.
-      10
+      ctx.compact * 4,
+      // Limit the columns to a maximum of fifteen.
+      15
     );
     // Return with the original output if no grouping should happen.
     if (columns <= 1) {
       return output;
     }
-    // Calculate the maximum length of all entries that are visible in the first
-    // column of the group.
     const tmp = [];
-    let firstLineMaxLength = dataLen[0];
-    for (i = columns; i < dataLen.length; i += columns) {
-      if (dataLen[i] > firstLineMaxLength)
-        firstLineMaxLength = dataLen[i];
+    const maxLineLength = [];
+    for (let i = 0; i < columns; i++) {
+      let lineMaxLength = 0;
+      for (let j = i; j < output.length; j += columns) {
+        if (dataLen[j] > lineMaxLength)
+          lineMaxLength = dataLen[j];
+      }
+      lineMaxLength += separatorSpace;
+      maxLineLength[i] = lineMaxLength;
+    }
+    let order = 'padStart';
+    if (value !== undefined) {
+      for (let i = 0; i < output.length; i++) {
+        // eslint-disable-next-line valid-typeof
+        if (typeof value[i] !== 'number' && typeof value[i] !== 'bigint') {
+          order = 'padEnd';
+          break;
+        }
+      }
     }
     // Each iteration creates a single line of grouped entries.
-    for (i = 0; i < outputLength; i += columns) {
-      // Calculate extra color padding in case it's active. This has to be done
-      // line by line as some lines might contain more colors than others.
-      let colorPadding = output[i].length - dataLen[i];
-      // Add padding to the first column of the output.
-      let str = output[i].padStart(firstLineMaxLength + colorPadding, ' ');
+    for (let i = 0; i < outputLength; i += columns) {
       // The last lines may contain less entries than columns.
       const max = Math.min(i + columns, outputLength);
-      for (var j = i + 1; j < max; j++) {
-        colorPadding = output[j].length - dataLen[j];
-        str += `, ${output[j].padStart(maxLength + colorPadding, ' ')}`;
+      let str = '';
+      let j = i;
+      for (; j < max - 1; j++) {
+        // Calculate extra color padding in case it's active. This has to be
+        // done line by line as some lines might contain more colors than
+        // others.
+        const padding = maxLineLength[j - i] + output[j].length - dataLen[j];
+        str += `${output[j]}, `[order](padding, ' ');
+      }
+      if (order === 'padStart') {
+        const padding = maxLineLength[j - i] +
+                        output[j].length -
+                        dataLen[j] -
+                        separatorSpace;
+        str += output[j].padStart(padding, ' ');
+      } else {
+        str += output[j];
       }
       tmp.push(str);
     }
@@ -1444,7 +1466,7 @@ function isBelowBreakLength(ctx, output, start, base) {
 }
 
 function reduceToSingleString(
-  ctx, output, base, braces, extrasType, recurseTimes) {
+  ctx, output, base, braces, extrasType, recurseTimes, value) {
   if (ctx.compact !== true) {
     if (typeof ctx.compact === 'number' && ctx.compact >= 1) {
       // Memorize the original output length. In case the the output is grouped,
@@ -1453,7 +1475,7 @@ function reduceToSingleString(
       // Group array elements together if the array contains at least six
       // separate entries.
       if (extrasType === kArrayExtrasType && entries > 6) {
-        output = groupArrayElements(ctx, output);
+        output = groupArrayElements(ctx, output, value);
       }
       // `ctx.currentDepth` is set to the most inner depth of the currently
       // inspected object part while `recurseTimes` is the actual current depth