-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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 1 commit
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 |
---|---|---|
|
@@ -276,12 +276,24 @@ function formatValue(ctx, value, recurseTimes) { | |
} | ||
} | ||
|
||
var base = '', array = false, braces = ['{', '}']; | ||
var base = '', braces; | ||
|
||
// Make Array say that they are Array | ||
if (Array.isArray(value)) { | ||
array = true; | ||
braces = ['[', ']']; | ||
} 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'); | ||
} else if (value instanceof Map) { | ||
braces = ['Map {', '}']; | ||
// ditto | ||
if (ctx.showHidden) | ||
keys.unshift('size'); | ||
} else { | ||
braces = ['{', '}']; | ||
} | ||
|
||
// Make functions say that they are functions | ||
|
@@ -323,7 +335,10 @@ function formatValue(ctx, value, recurseTimes) { | |
base = ' ' + '[Boolean: ' + formatted + ']'; | ||
} | ||
|
||
if (keys.length === 0 && (!array || value.length === 0)) { | ||
if (keys.length === 0 && | ||
(!Array.isArray(value) || value.length === 0) && | ||
(!(value instanceof Set) || value.size === 0) && | ||
(!(value instanceof Map) || value.size === 0)) { | ||
return braces[0] + base + braces[1]; | ||
} | ||
|
||
|
@@ -338,11 +353,15 @@ function formatValue(ctx, value, recurseTimes) { | |
ctx.seen.push(value); | ||
|
||
var output; | ||
if (array) { | ||
if (Array.isArray(value)) { | ||
output = formatArray(ctx, value, recurseTimes, visibleKeys, keys); | ||
} else if (value instanceof Set) { | ||
output = formatSet(ctx, value, recurseTimes, visibleKeys, keys); | ||
} else if (value instanceof Map) { | ||
output = formatMap(ctx, value, recurseTimes, visibleKeys, keys); | ||
} else { | ||
output = keys.map(function(key) { | ||
return formatProperty(ctx, value, recurseTimes, visibleKeys, key, array); | ||
return formatProperty(ctx, value, recurseTimes, visibleKeys, key, false); | ||
}); | ||
} | ||
|
||
|
@@ -417,6 +436,38 @@ 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); | ||
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 formatProperty(ctx, value, recurseTimes, visibleKeys, key, array) { | ||
var name, str, desc; | ||
desc = Object.getOwnPropertyDescriptor(value, key) || { value: value[key] }; | ||
|
@@ -488,7 +539,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,17 @@ 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 {}') | ||
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: semicolons here and below 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. I have that in a new commit yet to be pushed. Is it normal that make jshint doesn't touch tests? 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. Yeah, for some reason that choice was made a long time ago. Wouldn't call it "normal" though :) |
||
assert.equal(util.inspect(new Set([1, 2, 3])), 'Set { 1, 2, 3 }') | ||
var set = new Set(["foo"]) | ||
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\' }') | ||
var map = new Map([["foo", null]]) | ||
map.bar = 42 | ||
assert.equal(util.inspect(map, true), 'Map { \'foo\' => null, [size]: 1, bar: 42 }') |
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.
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
recurseTimes
is sometimes a number and sometimes not?