console: console.dir takes options object: Fixes #5428 #5436

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
4 participants

console.dir will forward an options arg to util.inspect. options.customInspect will default to false if not specified, all other options use the util.inspect defaults.

Documentation and a test also updated.

Fixes #5428

Thank you for contributing this pull request! Here are a few pointers to make sure your submission will be considered for inclusion.

Commit alexwhitman/node@c10265ea8cf7ecbc74cc734c98ea7753d41e259f has the following error(s):

  • Commit message must indicate the subsystem this commit changes

Please see CONTRIBUTING.md for more information

@bnoordhuis bnoordhuis commented on an outdated diff May 8, 2013

lib/console.js
@@ -65,8 +65,14 @@ Console.prototype.warn = function() {
Console.prototype.error = Console.prototype.warn;
-Console.prototype.dir = function(object) {
- this._stdout.write(util.inspect(object, { customInspect: false }) + '\n');
+Console.prototype.dir = function(object, options) {
+ if (!options) {
+ options = {};
+ }
+ if (!options.customInspect) {
+ options.customInspect = false;
@bnoordhuis

bnoordhuis May 8, 2013

Owner

Don't modify the options object that the user passed in. Use options = util._extend({ customInspect: false }, options) to make a copy. util._extend() handles non-object arguments gracefully so you don't have to check for !options.

@bnoordhuis bnoordhuis commented on an outdated diff May 8, 2013

test/simple/test-console.js
@@ -50,6 +50,7 @@ console.log(custom_inspect);
// test console.dir()
console.dir(custom_inspect);
+console.dir({ foo: { bar: 'baz' }}, { depth: 0 });
@bnoordhuis

bnoordhuis May 8, 2013

Owner

Maybe add a test where customInspect !== false.

@alexwhitman @alexwhitman alexwhitman console: console.dir takes options object
console.dir will forward an options arg to util.inspect.
options.customInspect will default to false if not specified, all
other options use the util.inspect defaults.

Documentation and a test also updated.
96adbec

@bnoordhuis Thanks for the review. I've updated the pull request.

This functionality was merged in joyent#7754. Thanks for your patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment