Skip to content
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.

Sign up for GitHub

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: implement %o as formatting specifier #14558

Closed
wants to merge 5 commits into from
Closed

util: implement %o as formatting specifier #14558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2dc1ba6
util: implement %o as formatting specifier
gla5001 Jul 31, 2017
c0826e7
util: implement %o as formatting specifier
gla5001 Aug 1, 2017
0b373b4
util: implement %o as formatting specifier.
gla5001 Aug 1, 2017
436765e
util: implement %o as formatting specifier. doc fix
gla5001 Aug 1, 2017
6bffe78
util: implement %o as formatting specifier
gla5001 Aug 2, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions doc/api/util.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,14 @@ corresponding argument. Supported placeholders are:
* `%f` - Floating point value.
* `%j` - JSON. Replaced with the string `'[Circular]'` if the argument
contains circular references.
* `%o` - Object. A string representation of an object
with generic JavaScript object formatting.
Similar to `util.inspect()` with options `{ showHidden: true, depth: 4, showProxy: true }`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the default for depth? I think we shouldn't change its default.

I'm okay with showProxy, but some people may interpret it as violating a security contract (where Proxies are supposed to be transparent)…

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{depth: 2} - https://github.com/nodejs/node/blob/master/lib/util.js#L57

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So what was the rationale for using 4 instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though I didn't originate it, IMHO it comes from optimally useful formatting and 2 is too small.
Since we can't do interactive expansion like in a GUI, Ideally I'd say it needs to be Infinity, but that's unreasonable.

This does make me think about a configurable util.inspect.extendedFormattingOptions or even util.format.optimallyUsefulFormatter but that could happen in a later PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for fun, I took a stroll down memory lane:
This is where is where depth was introduced a2714be
Two hour later the defaults were set de15214
Values seem to be a personal preferance https://groups.google.com/d/msg/nodejs/rAYFIeJ_72U/_bWLezvZavEJ

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just using null?

This will show the full object including non-enumerable symbols and properties.
* `%O` - Object. A string representation of an object
with generic JavaScript object formatting.
Similar to `util.inspect()` without options.
This will show the full object not including non-enumerable symbols and properties.
* `%%` - single percent sign (`'%'`). This does not consume an argument.

If the placeholder does not have a corresponding argument, the placeholder is
Expand Down
11 changes: 11 additions & 0 deletions lib/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,17 @@ function format(f) {
str += f.slice(lastPos, i);
str += String(arguments[a++]);
break;
case 79: // 'O'
if (lastPos < i)
str += f.slice(lastPos, i);
str += inspect(arguments[a++]);
break;
case 111: // 'o'
if (lastPos < i)
str += f.slice(lastPos, i);
str += inspect(arguments[a++],
{ showHidden: true, depth: 4, showProxy: true });
break;
case 37: // '%'
if (lastPos < i)
str += f.slice(lastPos, i);
Expand Down
82 changes: 82 additions & 0 deletions test/parallel/test-util-format.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,84 @@ assert.strictEqual(util.format('%j', '42'), '"42"');
assert.strictEqual(util.format('%j %j', 42, 43), '42 43');
assert.strictEqual(util.format('%j %j', 42), '42 %j');

// Object format specifier
const obj = {
foo: 'bar',
foobar: 1,
func: function() {}
};
const nestedObj = {
foo: 'bar',
foobar: {
foo: 'bar',
func: function() {}
}
};
assert.strictEqual(util.format('%o'), '%o');
assert.strictEqual(util.format('%o', 42), '42');
assert.strictEqual(util.format('%o', 'foo'), '\'foo\'');
assert.strictEqual(
util.format('%o', obj),
'{ foo: \'bar\',\n' +
' foobar: 1,\n' +
' func: \n' +
' { [Function: func]\n' +
' [length]: 0,\n' +
' [name]: \'func\',\n' +
' [prototype]: func { [constructor]: [Circular] } } }');
assert.strictEqual(
util.format('%o', nestedObj),
'{ foo: \'bar\',\n' +
' foobar: \n' +
' { foo: \'bar\',\n' +
' func: \n' +
' { [Function: func]\n' +
' [length]: 0,\n' +
' [name]: \'func\',\n' +
' [prototype]: func { [constructor]: [Circular] } } } }');
assert.strictEqual(
util.format('%o %o', obj, obj),
'{ foo: \'bar\',\n' +
' foobar: 1,\n' +
' func: \n' +
' { [Function: func]\n' +
' [length]: 0,\n' +
' [name]: \'func\',\n' +
' [prototype]: func { [constructor]: [Circular] } } }' +
' { foo: \'bar\',\n' +
' foobar: 1,\n' +
' func: \n' +
' { [Function: func]\n' +
' [length]: 0,\n' +
' [name]: \'func\',\n' +
' [prototype]: func { [constructor]: [Circular] } } }');
assert.strictEqual(
util.format('%o %o', obj),
'{ foo: \'bar\',\n' +
' foobar: 1,\n' +
' func: \n' +
' { [Function: func]\n' +
' [length]: 0,\n' +
' [name]: \'func\',\n' +
' [prototype]: func { [constructor]: [Circular] } } } %o');

assert.strictEqual(util.format('%O'), '%O');
assert.strictEqual(util.format('%O', 42), '42');
assert.strictEqual(util.format('%O', 'foo'), '\'foo\'');
assert.strictEqual(
util.format('%O', obj),
'{ foo: \'bar\', foobar: 1, func: [Function: func] }');
assert.strictEqual(
util.format('%O', nestedObj),
'{ foo: \'bar\', foobar: { foo: \'bar\', func: [Function: func] } }');
assert.strictEqual(
util.format('%O %O', obj, obj),
'{ foo: \'bar\', foobar: 1, func: [Function: func] } ' +
'{ foo: \'bar\', foobar: 1, func: [Function: func] }');
assert.strictEqual(
util.format('%O %O', obj),
'{ foo: \'bar\', foobar: 1, func: [Function: func] } %O');

// Various format specifiers
assert.strictEqual(util.format('%%s%s', 'foo'), '%sfoo');
assert.strictEqual(util.format('%s:%s'), '%s:%s');
Expand All @@ -125,6 +203,10 @@ assert.strictEqual(util.format('%f:%f'), '%f:%f');
assert.strictEqual(util.format('o: %j, a: %j', {}, []), 'o: {}, a: []');
assert.strictEqual(util.format('o: %j, a: %j', {}), 'o: {}, a: %j');
assert.strictEqual(util.format('o: %j, a: %j'), 'o: %j, a: %j');
assert.strictEqual(util.format('o: %o, a: %O', {}, []), 'o: {}, a: []');
assert.strictEqual(util.format('o: %o, a: %o', {}), 'o: {}, a: %o');
assert.strictEqual(util.format('o: %O, a: %O'), 'o: %O, a: %O');


// Invalid format specifiers
assert.strictEqual(util.format('a% b', 'x'), 'a% b x');
Expand Down