-
Notifications
You must be signed in to change notification settings - Fork 30.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
util: support inspecting Map, Set, and Promise #1471
Changes from 7 commits
613a63b
21e4e92
87e36a6
10d88e5
1185ff3
c4d2cd0
ba53e97
79627ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
'use strict'; | ||
|
||
const uv = process.binding('uv'); | ||
const Debug = require('vm').runInDebugContext('Debug'); | ||
|
||
const formatRegExp = /%[sdj%]/g; | ||
exports.format = function(f) { | ||
|
@@ -192,6 +193,14 @@ function arrayToHash(array) { | |
} | ||
|
||
|
||
function inspectPromise(p) { | ||
var mirror = Debug.MakeMirror(p, true); | ||
if (!mirror.isPromise()) | ||
return null; | ||
return {status: mirror.status(), value: mirror.promiseValue().value_}; | ||
} | ||
|
||
|
||
function formatValue(ctx, value, recurseTimes) { | ||
// Provide a hook for user-specified inspect functions. | ||
// Check that value is an object with an inspect function on it | ||
|
@@ -276,14 +285,44 @@ function formatValue(ctx, value, recurseTimes) { | |
} | ||
} | ||
|
||
var base = '', array = false, braces = ['{', '}']; | ||
var base = '', empty, braces, formatter; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Preferably assign It's a minor thing but V8 emits marginally tighter code for strict boolean comparisons. |
||
|
||
// Make Array say that they are Array | ||
if (Array.isArray(value)) { | ||
array = true; | ||
braces = ['[', ']']; | ||
empty = value.length === 0; | ||
formatter = formatArray; | ||
} else if (value instanceof Set) { | ||
braces = ['Set {', '}']; | ||
// With `showHidden`, `length` will display as a hidden property for | ||
// arrays. For consistency's sake, do the same for `size`, even though this | ||
// property isn't selected by Object.getOwnPropertyNames(). | ||
if (ctx.showHidden) | ||
keys.unshift('size'); | ||
empty = value.size === 0; | ||
formatter = formatSet; | ||
} else if (value instanceof Map) { | ||
braces = ['Map {', '}']; | ||
// ditto | ||
if (ctx.showHidden) | ||
keys.unshift('size'); | ||
empty = value.size === 0; | ||
formatter = formatMap; | ||
} else { | ||
// Only create a mirror if the object superficially looks like a Promise | ||
var promiseInternals = value instanceof Promise && inspectPromise(value); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just curious, is there a reason to keep the instanceof check out of inspectPromise? |
||
if (promiseInternals) { | ||
braces = ['Promise {', '}']; | ||
empty = false; | ||
formatter = formatPromise; | ||
} else { | ||
braces = ['{', '}']; | ||
empty = true; // no other data than keys | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Femto-style nit: two spaces before the comment and please capitalize and punctuate it. Ditto for other comments. |
||
formatter = formatObject; | ||
} | ||
} | ||
|
||
empty = empty && keys.length === 0; | ||
|
||
// Make functions say that they are functions | ||
if (typeof value === 'function') { | ||
var n = value.name ? ': ' + value.name : ''; | ||
|
@@ -323,7 +362,7 @@ function formatValue(ctx, value, recurseTimes) { | |
base = ' ' + '[Boolean: ' + formatted + ']'; | ||
} | ||
|
||
if (keys.length === 0 && (!array || value.length === 0)) { | ||
if (empty) { | ||
return braces[0] + base + braces[1]; | ||
} | ||
|
||
|
@@ -337,14 +376,7 @@ function formatValue(ctx, value, recurseTimes) { | |
|
||
ctx.seen.push(value); | ||
|
||
var output; | ||
if (array) { | ||
output = formatArray(ctx, value, recurseTimes, visibleKeys, keys); | ||
} else { | ||
output = keys.map(function(key) { | ||
return formatProperty(ctx, value, recurseTimes, visibleKeys, key, array); | ||
}); | ||
} | ||
var output = formatter(ctx, value, recurseTimes, visibleKeys, keys); | ||
|
||
ctx.seen.pop(); | ||
|
||
|
@@ -397,6 +429,13 @@ function formatError(value) { | |
} | ||
|
||
|
||
function formatObject(ctx, value, recurseTimes, visibleKeys, keys) { | ||
return keys.map(function(key) { | ||
return formatProperty(ctx, value, recurseTimes, visibleKeys, key, false); | ||
}); | ||
} | ||
|
||
|
||
function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
for (var i = 0, l = value.length; i < l; ++i) { | ||
|
@@ -417,6 +456,59 @@ function formatArray(ctx, value, recurseTimes, visibleKeys, keys) { | |
} | ||
|
||
|
||
function formatSet(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
value.forEach(function(v) { | ||
var str = formatValue(ctx, v, | ||
recurseTimes === null ? null : recurseTimes - 1); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For readability, it's probably better to assign the result of the ternary to a variable here and below. Aside: am I the only one who finds it moronic that |
||
output.push(str); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: we should be able to do something like |
||
keys.forEach(function(key) { | ||
output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, | ||
key, false)); | ||
}); | ||
return output; | ||
} | ||
|
||
|
||
function formatMap(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
value.forEach(function(v, k) { | ||
var str = formatValue(ctx, k, | ||
recurseTimes === null ? null : recurseTimes - 1); | ||
str += ' => '; | ||
str += formatValue(ctx, v, recurseTimes === null ? null : recurseTimes - 1); | ||
output.push(str); | ||
}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similarly, |
||
keys.forEach(function(key) { | ||
output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, | ||
key, false)); | ||
}); | ||
return output; | ||
} | ||
|
||
function formatPromise(ctx, value, recurseTimes, visibleKeys, keys) { | ||
var output = []; | ||
var internals = inspectPromise(value); | ||
if (internals.status === 'pending') { | ||
output.push('<pending>'); | ||
} else { | ||
var str = formatValue(ctx, internals.value, | ||
recurseTimes === null ? null : recurseTimes - 1); | ||
if (internals.status === 'rejected') { | ||
output.push('<rejected> ' + str); | ||
} else { | ||
output.push(str); | ||
} | ||
} | ||
keys.forEach(function(key) { | ||
output.push(formatProperty(ctx, value, recurseTimes, visibleKeys, | ||
key, false)); | ||
}); | ||
return output; | ||
} | ||
|
||
|
||
function formatProperty(ctx, value, recurseTimes, visibleKeys, key, array) { | ||
var name, str, desc; | ||
desc = Object.getOwnPropertyDescriptor(value, key) || { value: value[key] }; | ||
|
@@ -488,7 +580,10 @@ function reduceToSingleString(output, base, braces) { | |
|
||
if (length > 60) { | ||
return braces[0] + | ||
(base === '' ? '' : base + '\n ') + | ||
// If the opening "brace" is too large, like in the case of "Set {", | ||
// we need to force the first item to be on the next line or the | ||
// items will not line up correctly. | ||
(base === '' && braces[0].length === 1 ? '' : base + '\n ') + | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this case needs tests |
||
' ' + | ||
output.join(',\n ') + | ||
' ' + | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -235,3 +235,66 @@ if (typeof Symbol !== 'undefined') { | |
assert.equal(util.inspect(subject, options), '[ 1, 2, 3, [length]: 3, [Symbol(symbol)]: 42 ]'); | ||
|
||
} | ||
|
||
// test Set | ||
assert.equal(util.inspect(new Set), 'Set {}'); | ||
assert.equal(util.inspect(new Set([1, 2, 3])), 'Set { 1, 2, 3 }'); | ||
var set = new Set(["foo"]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Femto-nit: prefer single quotes. |
||
set.bar = 42; | ||
assert.equal(util.inspect(set, true), 'Set { \'foo\', [size]: 1, bar: 42 }'); | ||
|
||
// test Map | ||
assert.equal(util.inspect(new Map), 'Map {}'); | ||
assert.equal(util.inspect(new Map([[1, 'a'], [2, 'b'], [3, 'c']])), 'Map { 1 => \'a\', 2 => \'b\', 3 => \'c\' }'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: long line, please wrap at 80 columns. |
||
var map = new Map([["foo", null]]); | ||
map.bar = 42; | ||
assert.equal(util.inspect(map, true), 'Map { \'foo\' => null, [size]: 1, bar: 42 }'); | ||
|
||
// test Promise | ||
assert.equal(util.inspect(Promise.resolve(3)), 'Promise { 3 }'); | ||
assert.equal(util.inspect(Promise.reject(3)), 'Promise { <rejected> 3 }'); | ||
assert.equal(util.inspect(new Promise(function() {})), 'Promise { <pending> }'); | ||
var promise = Promise.resolve('foo'); | ||
promise.bar = 42; | ||
assert.equal(util.inspect(promise), 'Promise { \'foo\', bar: 42 }'); | ||
|
||
// Make sure it doesn't choke on polyfills. Unlike Set/Map, there is no standard | ||
// interface to synchronously inspect a Promise, so our techniques only work on | ||
// a bonafide native Promise. | ||
var oldPromise = Promise; | ||
global.Promise = function() { this.bar = 42; }; | ||
assert.equal(util.inspect(new Promise), '{ bar: 42 }'); | ||
global.Promise = oldPromise; | ||
|
||
|
||
// Test alignment of items in container | ||
// Assumes that the first numeric character is the start of an item. | ||
|
||
function checkAlignment(container) { | ||
var lines = util.inspect(container).split("\n"); | ||
var pos; | ||
lines.forEach(function(line) { | ||
var npos = line.search(/\d/); | ||
if (npos !== -1) { | ||
if (pos !== undefined) | ||
assert.equal(pos, npos, "container items not aligned"); | ||
pos = npos; | ||
} | ||
}); | ||
} | ||
|
||
var big_array = []; | ||
for (var i = 0; i < 100; i++) { | ||
big_array.push(i); | ||
} | ||
|
||
checkAlignment(big_array); | ||
checkAlignment(function(){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Style: space before brace. |
||
var obj = {}; | ||
big_array.forEach(function(v) { | ||
obj[v] = null; | ||
}); | ||
return obj; | ||
}()); | ||
checkAlignment(new Set(big_array)); | ||
checkAlignment(new Map(big_array.map(function (y) { return [y, null] }))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@domenic: Should we add support for
Weak{Set,Map}
inspection throughDebug
as a follow-on to this PR?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noooo that seems very bad, basically exposes iteration to JS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah – I thought it might be a bit fraught with peril; was surprised to see the chrome dev tools expose that info.