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.

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: handle symbols properly in format() #931

Closed
wants to merge 1 commit into from
Closed

util: handle symbols properly in format() #931

wants to merge 1 commit into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Feb 23, 2015

Currently, if util.format() is called with a string as its first argument, and a Symbol as one of the subsequent arguments, an exception is thrown due to an attempted implicit string conversion. This commit causes Symbols to be explicitly converted.

This bug is due to a discrepancy between how the docs say util.format() work, and what really happens. The docs state:

If there are more arguments than placeholders, the extra arguments are converted to strings with util.inspect() and these strings are concatenated, delimited by a space.

However, only objects actually use util.inspect(). I think we should bring behavior into line with what the docs say, but this is a slightly breaking change.

Closes #927. R=@domenic

Currently, if util.format() is called with a string as its first
argument, and a Symbol as one of the subsequent arguments, an
exception is thrown due to an attempted implicit string conversion.
This commit causes Symbols to be explicitly converted.
@domenic
Copy link
Contributor

domenic commented Feb 23, 2015

LGTM

cjihrig added a commit that referenced this pull request Feb 24, 2015
Currently, if util.format() is called with a string as its first
argument, and a Symbol as one of the subsequent arguments, an
exception is thrown due to an attempted implicit string conversion.
This commit causes Symbols to be explicitly converted.

Fixes: #927
PR-URL: #931
Reviewed-By: Domenic Denicola <[email protected]>
@cjihrig
Copy link
Contributor Author

cjihrig commented Feb 24, 2015

Landed in ed3b057

@cjihrig cjihrig closed this Feb 24, 2015
@cjihrig cjihrig deleted the 927 branch February 24, 2015 15:18
This was referenced Feb 24, 2015
petkaantonov pushed a commit to petkaantonov/io.js that referenced this pull request Feb 25, 2015
Currently, if util.format() is called with a string as its first
argument, and a Symbol as one of the subsequent arguments, an
exception is thrown due to an attempted implicit string conversion.
This commit causes Symbols to be explicitly converted.

Fixes: nodejs#927
PR-URL: nodejs#931
Reviewed-By: Domenic Denicola <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants