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.format not printing % when there's multiple arguments #13665

Closed
naddison36 opened this issue Jun 13, 2017 · 7 comments
Closed

util.format not printing % when there's multiple arguments #13665

naddison36 opened this issue Jun 13, 2017 · 7 comments
Labels
util Issues and PRs related to the built-in util module.

Comments

@naddison36
Copy link

  • v8.1.0:
  • Darwin MacBook-Pro.local 16.6.0 Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64 x86_64:
  • Subsystem:

The following code in node 6.10.3 prints percent: 10%, fraction: 0.1

console.log('percent: %d%, fraction: %d', 10, 0.1);

In node 8.1.0 it prints percent: 10 fraction: 0.1. Notice the % is no longer printed.

The following code gets back to the original node 6.10.3 output

console.log('percent: %d%%, fraction: %d', 10, 0.1);

Note the %% to print a single % in node 8

Also worth noting this problem doesn't happen if you are just printing one variable in node 8. The following code prints percent: 10%

console.log('percent: %d%', 10);

Should node 8 be formatting the same results as node 6? Or is this a functional change?

@mscdex mscdex added the util Issues and PRs related to the built-in util module. label Jun 13, 2017
@mscdex
Copy link
Contributor

mscdex commented Jun 13, 2017

This is probably due to #12407.

@mscdex
Copy link
Contributor

mscdex commented Jun 13, 2017

/cc @nodejs/ctc

@mscdex mscdex added the v8.x label Jun 13, 2017
@Trott
Copy link
Member

Trott commented Jun 14, 2017

/cc @jseijas

targos added a commit to targos/node that referenced this issue Jun 14, 2017
In util.format, if a percent sign without a known type is encountered,
just print it instead of silently ignoring it and the next character.

Fixes: nodejs#13665
@targos
Copy link
Member

targos commented Jun 14, 2017

Possible fix: #13674

@targos
Copy link
Member

targos commented Jun 14, 2017

Ideally we should throw an error in such cases. The correct format string here is 'percent: %d%%, fraction: %d'.

@silverwind
Copy link
Contributor

Yes, a throw would be ideal, but from my tests browsers don't throw on it either, so It'd be better to not throw for compat. I tested the case console.log('percent: %d%, fraction: %d', 10, 0.1); in a few browsers:

Firefox: percent: 10%, fraction: 0
Chrome: percent: 10%, fraction: 0
Safari: percent: 10[object Object] fraction: %6d

Quite an interesting result in Safari (also present in WebKit Nightly).

@naddison36
Copy link
Author

I can confirm the correct format of console.log('percent: %d%%, fraction: %d', 10, 0.1); gives the desired output of percent: 10%, fraction: 0.1 in Node 6.10.3 and 0.10.48

addaleax pushed a commit that referenced this issue Jun 17, 2017
In util.format, if a percent sign without a known type is encountered,
just print it instead of silently ignoring it and the next character.

PR-URL: #13674
Fixes: #13665
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
addaleax pushed a commit that referenced this issue Jun 21, 2017
In util.format, if a percent sign without a known type is encountered,
just print it instead of silently ignoring it and the next character.

PR-URL: #13674
Fixes: #13665
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
util Issues and PRs related to the built-in util module.
Projects
None yet
Development

No branches or pull requests

5 participants