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>

2 files changed, 134 insertions, 120 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
diff --git a/test/parallel/test-util-inspect.js b/test/parallel/test-util-inspect.js
index 03cbebe164..c264c409e5 100644
--- a/test/parallel/test-util-inspect.js
+++ b/test/parallel/test-util-inspect.js
@@ -1328,11 +1328,11 @@ if (typeof Symbol !== 'undefined') {
 {
   const x = new Uint8Array(101);
   assert(util.inspect(x).endsWith('1 more item\n]'));
-  assert(!util.inspect(x, { maxArrayLength: 101 }).endsWith('1 more item\n]'));
+  assert(!util.inspect(x, { maxArrayLength: 101 }).includes('1 more item'));
   assert.strictEqual(util.inspect(x, { maxArrayLength: 0 }),
                      'Uint8Array [ ... 101 more items ]');
-  assert(!util.inspect(x, { maxArrayLength: null }).endsWith('1 more item\n]'));
-  assert(util.inspect(x, { maxArrayLength: Infinity }).endsWith('  0, 0\n]'));
+  assert(!util.inspect(x, { maxArrayLength: null }).includes('1 more item'));
+  assert(util.inspect(x, { maxArrayLength: Infinity }).endsWith(' 0, 0\n]'));
 }
 
 {
@@ -1962,7 +1962,7 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'");
   [WeakMap, [[[{}, {}]]], '{ <items unknown> }'],
   [BigInt64Array,
    [10],
-   '[\n  0n, 0n, 0n,\n  0n, 0n, 0n,\n  0n, 0n, 0n,\n  0n\n]'],
+   '[\n  0n, 0n, 0n, 0n, 0n,\n  0n, 0n, 0n, 0n, 0n\n]'],
   [Date, ['Sun, 14 Feb 2010 11:48:40 GMT'], '2010-02-14T11:48:40.000Z'],
   [Date, ['invalid_date'], 'Invalid Date']
 ].forEach(([base, input, rawExpected]) => {
@@ -2202,21 +2202,18 @@ assert.strictEqual(
     '  b: [ 1, 2, [ 1, 2, { a: 1, b: 2, c: 3 } ] ],',
     "  c: [ 'foo', 4, 444444 ],",
     '  d: [',
-    '       0,    1,    4,    3,   16,    5,   36,',
-    '       7,   64,    9,  100,   11,  144,   13,',
-    '     196,   15,  256,   17,  324,   19,  400,',
-    '      21,  484,   23,  576,   25,  676,   27,',
-    '     784,   29,  900,   31, 1024,   33, 1156,',
-    '      35, 1296,   37, 1444,   39, 1600,   41,',
-    '    1764,   43, 1936,   45, 2116,   47, 2304,',
-    '      49, 2500,   51, 2704,   53, 2916,   55,',
-    '    3136,   57, 3364,   59, 3600,   61, 3844,',
-    '      63, 4096,   65, 4356,   67, 4624,   69,',
-    '    4900,   71, 5184,   73, 5476,   75, 5776,',
-    '      77, 6084,   79, 6400,   81, 6724,   83,',
-    '    7056,   85, 7396,   87, 7744,   89, 8100,',
-    '      91, 8464,   93, 8836,   95, 9216,   97,',
-    '    9604,   99,',
+    '       0,    1,    4,    3,   16,    5,   36,    7,   64,',
+    '       9,  100,   11,  144,   13,  196,   15,  256,   17,',
+    '     324,   19,  400,   21,  484,   23,  576,   25,  676,',
+    '      27,  784,   29,  900,   31, 1024,   33, 1156,   35,',
+    '    1296,   37, 1444,   39, 1600,   41, 1764,   43, 1936,',
+    '      45, 2116,   47, 2304,   49, 2500,   51, 2704,   53,',
+    '    2916,   55, 3136,   57, 3364,   59, 3600,   61, 3844,',
+    '      63, 4096,   65, 4356,   67, 4624,   69, 4900,   71,',
+    '    5184,   73, 5476,   75, 5776,   77, 6084,   79, 6400,',
+    '      81, 6724,   83, 7056,   85, 7396,   87, 7744,   89,',
+    '    8100,   91, 8464,   93, 8836,   95, 9216,   97, 9604,',
+    '      99,',
     '    ... 1 more item',
     '  ],',
     '  e: [',
@@ -2248,10 +2245,8 @@ assert.strictEqual(
     "    'foobar baz'",
     '  ],',
     '  h: [',
-    '    100,   0,   1,',
-    '      2,   3,   4,',
-    '      5,   6,   7,',
-    '      8',
+    '    100, 0, 1, 2, 3,',
+    '      4, 5, 6, 7, 8',
     '  ],',
     '  long: [',
     "    'This text is too long for grouping!',",
@@ -2279,15 +2274,15 @@ assert.strictEqual(
 
   expected = [
     '[',
-    '  1,         1,         1,',
-    '  1,         1,         1,',
-    '  1,         1,         1,',
-    '  1,         1,         1,',
-    '  1,         1,         1,',
-    '  1,         1,         1,',
-    '  1,         1,         1,',
-    '  1,         1,         1,',
-    '  1,         1, 123456789',
+    '  1, 1,         1,',
+    '  1, 1,         1,',
+    '  1, 1,         1,',
+    '  1, 1,         1,',
+    '  1, 1,         1,',
+    '  1, 1,         1,',
+    '  1, 1,         1,',
+    '  1, 1,         1,',
+    '  1, 1, 123456789',
     ']'
   ].join('\n');
 
@@ -2317,10 +2312,10 @@ assert.strictEqual(
     '    b: { x: \u001b[33m5\u001b[39m, c: \u001b[36m[Object]\u001b[39m }',
     '  },',
     '  b: [',
-    "    \u001b[32m'foobar'\u001b[39m,    \u001b[32m'baz'\u001b[39m,",
-    "    \u001b[32m'foobar'\u001b[39m,    \u001b[32m'baz'\u001b[39m,",
-    "    \u001b[32m'foobar'\u001b[39m,    \u001b[32m'baz'\u001b[39m,",
-    "    \u001b[32m'foobar'\u001b[39m,    \u001b[32m'baz'\u001b[39m,",
+    "    \u001b[32m'foobar'\u001b[39m, \u001b[32m'baz'\u001b[39m,",
+    "    \u001b[32m'foobar'\u001b[39m, \u001b[32m'baz'\u001b[39m,",
+    "    \u001b[32m'foobar'\u001b[39m, \u001b[32m'baz'\u001b[39m,",
+    "    \u001b[32m'foobar'\u001b[39m, \u001b[32m'baz'\u001b[39m,",
     "    \u001b[32m'foobar'\u001b[39m",
     '  ]',
     '}',
@@ -2333,26 +2328,23 @@ assert.strictEqual(
 
   expected = [
     '[',
-    '   \u001b[33m0\u001b[39m,  \u001b[33m1\u001b[39m,  \u001b[33m2\u001b[39m,',
-    '   \u001b[33m3\u001b[39m,  \u001b[33m4\u001b[39m,  \u001b[33m5\u001b[39m,',
-    '   \u001b[33m6\u001b[39m,  \u001b[33m7\u001b[39m,  \u001b[33m8\u001b[39m,',
-    '   \u001b[33m9\u001b[39m, \u001b[33m10\u001b[39m, \u001b[33m11\u001b[39m,',
-    '  \u001b[33m12\u001b[39m, \u001b[33m13\u001b[39m, \u001b[33m14\u001b[39m,',
-    '  \u001b[33m15\u001b[39m, \u001b[33m16\u001b[39m, \u001b[33m17\u001b[39m,',
-    '  \u001b[33m18\u001b[39m, \u001b[33m19\u001b[39m, \u001b[33m20\u001b[39m,',
-    '  \u001b[33m21\u001b[39m, \u001b[33m22\u001b[39m, \u001b[33m23\u001b[39m,',
-    '  \u001b[33m24\u001b[39m, \u001b[33m25\u001b[39m, \u001b[33m26\u001b[39m,',
-    '  \u001b[33m27\u001b[39m, \u001b[33m28\u001b[39m, \u001b[33m29\u001b[39m,',
-    '  \u001b[33m30\u001b[39m, \u001b[33m31\u001b[39m, \u001b[33m32\u001b[39m,',
-    '  \u001b[33m33\u001b[39m, \u001b[33m34\u001b[39m, \u001b[33m35\u001b[39m,',
-    '  \u001b[33m36\u001b[39m, \u001b[33m37\u001b[39m, \u001b[33m38\u001b[39m,',
-    '  \u001b[33m39\u001b[39m, \u001b[33m40\u001b[39m, \u001b[33m41\u001b[39m,',
-    '  \u001b[33m42\u001b[39m, \u001b[33m43\u001b[39m, \u001b[33m44\u001b[39m,',
-    '  \u001b[33m45\u001b[39m, \u001b[33m46\u001b[39m, \u001b[33m47\u001b[39m,',
-    '  \u001b[33m48\u001b[39m, \u001b[33m49\u001b[39m, \u001b[33m50\u001b[39m,',
-    '  \u001b[33m51\u001b[39m, \u001b[33m52\u001b[39m, \u001b[33m53\u001b[39m,',
-    '  \u001b[33m54\u001b[39m, \u001b[33m55\u001b[39m, \u001b[33m56\u001b[39m,',
-    '  \u001b[33m57\u001b[39m, \u001b[33m58\u001b[39m, \u001b[33m59\u001b[39m',
+    /* eslint-disable max-len */
+    '   \u001b[33m0\u001b[39m,  \u001b[33m1\u001b[39m,  \u001b[33m2\u001b[39m,  \u001b[33m3\u001b[39m,',
+    '   \u001b[33m4\u001b[39m,  \u001b[33m5\u001b[39m,  \u001b[33m6\u001b[39m,  \u001b[33m7\u001b[39m,',
+    '   \u001b[33m8\u001b[39m,  \u001b[33m9\u001b[39m, \u001b[33m10\u001b[39m, \u001b[33m11\u001b[39m,',
+    '  \u001b[33m12\u001b[39m, \u001b[33m13\u001b[39m, \u001b[33m14\u001b[39m, \u001b[33m15\u001b[39m,',
+    '  \u001b[33m16\u001b[39m, \u001b[33m17\u001b[39m, \u001b[33m18\u001b[39m, \u001b[33m19\u001b[39m,',
+    '  \u001b[33m20\u001b[39m, \u001b[33m21\u001b[39m, \u001b[33m22\u001b[39m, \u001b[33m23\u001b[39m,',
+    '  \u001b[33m24\u001b[39m, \u001b[33m25\u001b[39m, \u001b[33m26\u001b[39m, \u001b[33m27\u001b[39m,',
+    '  \u001b[33m28\u001b[39m, \u001b[33m29\u001b[39m, \u001b[33m30\u001b[39m, \u001b[33m31\u001b[39m,',
+    '  \u001b[33m32\u001b[39m, \u001b[33m33\u001b[39m, \u001b[33m34\u001b[39m, \u001b[33m35\u001b[39m,',
+    '  \u001b[33m36\u001b[39m, \u001b[33m37\u001b[39m, \u001b[33m38\u001b[39m, \u001b[33m39\u001b[39m,',
+    '  \u001b[33m40\u001b[39m, \u001b[33m41\u001b[39m, \u001b[33m42\u001b[39m, \u001b[33m43\u001b[39m,',
+    '  \u001b[33m44\u001b[39m, \u001b[33m45\u001b[39m, \u001b[33m46\u001b[39m, \u001b[33m47\u001b[39m,',
+    '  \u001b[33m48\u001b[39m, \u001b[33m49\u001b[39m, \u001b[33m50\u001b[39m, \u001b[33m51\u001b[39m,',
+    '  \u001b[33m52\u001b[39m, \u001b[33m53\u001b[39m, \u001b[33m54\u001b[39m, \u001b[33m55\u001b[39m,',
+    '  \u001b[33m56\u001b[39m, \u001b[33m57\u001b[39m, \u001b[33m58\u001b[39m, \u001b[33m59\u001b[39m',
+    /* eslint-enable max-len */
     ']'
   ].join('\n');
 
@@ -2411,44 +2403,44 @@ assert.strictEqual(
   );
   expected = [
     '[',
-    "          'Object',           'Function',              'Array',",
-    "          'Number',         'parseFloat',           'parseInt',",
-    "        'Infinity',                'NaN',          'undefined',",
-    "         'Boolean',             'String',             'Symbol',",
-    "            'Date',            'Promise',             'RegExp',",
-    "           'Error',          'EvalError',         'RangeError',",
-    "  'ReferenceError',        'SyntaxError',          'TypeError',",
-    "        'URIError',               'JSON',               'Math',",
-    "         'console',               'Intl',        'ArrayBuffer',",
-    "      'Uint8Array',          'Int8Array',        'Uint16Array',",
-    "      'Int16Array',        'Uint32Array',         'Int32Array',",
-    "    'Float32Array',       'Float64Array',  'Uint8ClampedArray',",
-    "  'BigUint64Array',      'BigInt64Array',           'DataView',",
-    "             'Map',             'BigInt',                'Set',",
-    "         'WeakMap',            'WeakSet',              'Proxy',",
-    "         'Reflect',          'decodeURI', 'decodeURIComponent',",
-    "       'encodeURI', 'encodeURIComponent',             'escape',",
-    "        'unescape',               'eval',           'isFinite',",
-    "           'isNaN',  'SharedArrayBuffer',            'Atomics',",
-    "      'globalThis',        'WebAssembly',             'global',",
-    "         'process',             'GLOBAL',               'root',",
-    "          'Buffer',                'URL',    'URLSearchParams',",
-    "     'TextEncoder',        'TextDecoder',      'clearInterval',",
-    "    'clearTimeout',        'setInterval',         'setTimeout',",
-    "  'queueMicrotask',     'clearImmediate',       'setImmediate',",
-    "          'module',            'require',             'assert',",
-    "     'async_hooks',             'buffer',      'child_process',",
-    "         'cluster',             'crypto',              'dgram',",
-    "             'dns',             'domain',             'events',",
-    "              'fs',               'http',              'http2',",
-    "           'https',          'inspector',                'net',",
-    "              'os',               'path',         'perf_hooks',",
-    "        'punycode',        'querystring',           'readline',",
-    "            'repl',             'stream',     'string_decoder',",
-    "             'tls',       'trace_events',                'tty',",
-    "             'url',                 'v8',                 'vm',",
-    "  'worker_threads',               'zlib',                  '_',",
-    "          '_error',               'util'",
+    "  'Object',         'Function',           'Array',",
+    "  'Number',         'parseFloat',         'parseInt',",
+    "  'Infinity',       'NaN',                'undefined',",
+    "  'Boolean',        'String',             'Symbol',",
+    "  'Date',           'Promise',            'RegExp',",
+    "  'Error',          'EvalError',          'RangeError',",
+    "  'ReferenceError', 'SyntaxError',        'TypeError',",
+    "  'URIError',       'JSON',               'Math',",
+    "  'console',        'Intl',               'ArrayBuffer',",
+    "  'Uint8Array',     'Int8Array',          'Uint16Array',",
+    "  'Int16Array',     'Uint32Array',        'Int32Array',",
+    "  'Float32Array',   'Float64Array',       'Uint8ClampedArray',",
+    "  'BigUint64Array', 'BigInt64Array',      'DataView',",
+    "  'Map',            'BigInt',             'Set',",
+    "  'WeakMap',        'WeakSet',            'Proxy',",
+    "  'Reflect',        'decodeURI',          'decodeURIComponent',",
+    "  'encodeURI',      'encodeURIComponent', 'escape',",
+    "  'unescape',       'eval',               'isFinite',",
+    "  'isNaN',          'SharedArrayBuffer',  'Atomics',",
+    "  'globalThis',     'WebAssembly',        'global',",
+    "  'process',        'GLOBAL',             'root',",
+    "  'Buffer',         'URL',                'URLSearchParams',",
+    "  'TextEncoder',    'TextDecoder',        'clearInterval',",
+    "  'clearTimeout',   'setInterval',        'setTimeout',",
+    "  'queueMicrotask', 'clearImmediate',     'setImmediate',",
+    "  'module',         'require',            'assert',",
+    "  'async_hooks',    'buffer',             'child_process',",
+    "  'cluster',        'crypto',             'dgram',",
+    "  'dns',            'domain',             'events',",
+    "  'fs',             'http',               'http2',",
+    "  'https',          'inspector',          'net',",
+    "  'os',             'path',               'perf_hooks',",
+    "  'punycode',       'querystring',        'readline',",
+    "  'repl',           'stream',             'string_decoder',",
+    "  'tls',            'trace_events',       'tty',",
+    "  'url',            'v8',                 'vm',",
+    "  'worker_threads', 'zlib',               '_',",
+    "  '_error',         'util'",
     ']'
   ].join('\n');