From 7e617606b088b8622f466cecf983b20672afa17f Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 5 Jun 2019 14:20:52 +0200 Subject: [PATCH] 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 --- lib/internal/util/inspect.js | 74 +++++++----- test/parallel/test-util-inspect.js | 180 ++++++++++++++--------------- 2 files changed, 134 insertions(+), 120 deletions(-) diff --git a/lib/internal/util/inspect.js b/lib/internal/util/inspect.js index 5c58a19c25316b..c018a73a93fe7f 100644 --- a/lib/internal/util/inspect.js +++ b/lib/internal/util/inspect.js @@ -777,7 +777,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; @@ -940,7 +940,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; @@ -972,51 +972,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); } @@ -1419,7 +1441,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, @@ -1428,7 +1450,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 525b97c5e7a09f..794b23b3ece458 100644 --- a/test/parallel/test-util-inspect.js +++ b/test/parallel/test-util-inspect.js @@ -1306,11 +1306,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]')); } { @@ -1940,7 +1940,7 @@ assert.strictEqual(util.inspect('"\'${a}'), "'\"\\'${a}'"); [WeakMap, [[[{}, {}]]], '{ }'], [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]) => { @@ -2180,21 +2180,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: [', @@ -2226,10 +2223,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!',", @@ -2257,15 +2252,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'); @@ -2295,10 +2290,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", ' ]', '}', @@ -2311,26 +2306,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'); @@ -2389,44 +2381,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');