Skip to content

Conversation

@addaleax
Copy link
Member

@addaleaxaddaleax commented Jul 9, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

util

Description of change

Inspect boxed symbol objects in the same way other boxed primitives are inspected.

Fixes: #7639

CI: https://ci.nodejs.org/job/node-test-commit/4039/

Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: nodejs#7639
@addaleaxaddaleax added the util Issues and PRs related to the built-in util module. label Jul 9, 2016
@targos
Copy link
Member

LGTM

3 similar comments
@bnoordhuis
Copy link
Member

LGTM

@benjamingr
Copy link
Member

LGTM

@cjihrig
Copy link
Contributor

LGTM


// test boxed primitives output the correct values
assert.equal(util.inspect(newString('test')),'[String: \'test\']');
assert.equal(util.inspect(Object(Symbol('test'))),'[Symbol: Symbol(test)]');
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this look like having redundant information?

Copy link
Member

Choose a reason for hiding this comment

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

No, IMO. It conveys that it's a symbol wrapped in an object; i.e., that typeof object === 'object' but that typeof object.valueOf() === 'symbol'.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

@thefourtheye I think learning the difference between Symbol(test) and [Symbol: test] might be a bit harder for people new to symbols.

This way might not be perfect, but I think the consistency with how other boxed values are displayed makes it easier to reason about what exactly is being displayed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense. LGTM.

@addaleax
Copy link
MemberAuthor

Landed in 35e8c94

@addaleaxaddaleax deleted the util-inspect-symbol branch July 17, 2016 16:11
addaleax added a commit that referenced this pull request Jul 17, 2016
Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: #7639 PR-URL: #7641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
evanlucas pushed a commit that referenced this pull request Jul 19, 2016
Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: #7639 PR-URL: #7641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@evanlucasevanlucas mentioned this pull request Jul 19, 2016
evanlucas pushed a commit that referenced this pull request Jul 20, 2016
Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: #7639 PR-URL: #7641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
evanlucas added a commit that referenced this pull request Jul 21, 2016
Notable changes: * **buffer**: * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) #7602 * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) #7176 * **deps**: Upgrade to V8 5.0.71.xx. (Ben Noordhuis) #7531 * **repl**: Fix issue with function redeclaration. (Prince J Wesley) #7794 * **util**: Fix inspecting of boxed symbols. (Anna Henningsen) #7641 PR-URL: #7782
evanlucas added a commit that referenced this pull request Jul 21, 2016
Notable changes: * **buffer**: * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) #7602 * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) #7176 * **deps**: * Upgrade to V8 5.0.71.xx. (Ben Noordhuis) #7531 * Backport V8 instanceof bugfix (Franziska Hinkelmann) #7638 * **repl**: Fix issue with function redeclaration. (Prince J Wesley) #7794 * **util**: Fix inspecting of boxed symbols. (Anna Henningsen) #7641 PR-URL: #7782
evanlucas added a commit that referenced this pull request Jul 21, 2016
Notable changes: * **buffer**: * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) #7602 * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) #7176 * **deps**: * Upgrade to V8 5.0.71.xx. (Ben Noordhuis) #7531 * Backport V8 instanceof bugfix (Franziska Hinkelmann) #7638 * **repl**: Fix issue with function redeclaration. (Prince J Wesley) #7794 * **util**: Fix inspecting of boxed symbols. (Anna Henningsen) #7641 PR-URL: #7782
lukesampson pushed a commit to ScoopInstaller/Scoop that referenced this pull request Jul 24, 2016
### Notable changes * **buffer**: * Improve performance of Buffer.from(str, 'hex') and Buffer#write(str, 'hex'). (Christopher Jeffrey) [#7602](nodejs/node#7602) * Fix creating from zero-length ArrayBuffer. (Ingvar Stepanyan) [#7176](nodejs/node#7176) * **deps**: * Upgrade to V8 5.0.71.xx. (Ben Noordhuis) [#7531](nodejs/node#7531) * Backport V8 instanceof bugfix (Franziska Hinkelmann) [#7638](nodejs/node#7638) * **repl**: Fix issue with function redeclaration. (Prince J Wesley) [#7794](nodejs/node#7794) * **util**: Fix inspecting of boxed symbols. (Anna Henningsen) [#7641](nodejs/node#7641)
MylesBorins pushed a commit that referenced this pull request Aug 30, 2016
Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: #7639 PR-URL: #7641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 28, 2016
Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: #7639 PR-URL: #7641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: #7639 PR-URL: #7641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
Inspect boxed symbol objects in the same way other boxed primitives are inspected. Fixes: #7639 PR-URL: #7641 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Oct 26, 2016
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

utilIssues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@addaleax@targos@bnoordhuis@benjamingr@cjihrig@thefourtheye@MylesBorins