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

console.log and util.format formatting specifiers #10292

Closed
jsinmotion opened this issue Dec 15, 2016 · 31 comments
Closed

console.log and util.format formatting specifiers #10292

jsinmotion opened this issue Dec 15, 2016 · 31 comments
Assignees
Labels
console Issues and PRs related to the console subsystem. feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module.

Comments

@jsinmotion
Copy link

The Chromium console supports format specifiers: https://developers.google.com/web/tools/chrome-devtools/console/console-write#string_substitution_and_formatting

In particular, both %i and %d specify an integral argument. Node.js does not perform the conversion for %i:

/* node.js */
> console.log("%i", 1234)
%i 1234
/* google chrome */
> console.log("%i", 1234)
1234

I think the fix is really simple: the relevant code is at https://github.com/nodejs/node/blob/master/lib/util.js#L86-L117 . Just adding the i case should be enough:

      switch (f.charCodeAt(i + 1)) {
        case 105: // 'i' <-- this is the new line, and it should appear just before the `d` case to make the fall through work
        case 100: // 'd'

If it makes sense I can send a PR

@italoacasas italoacasas added console Issues and PRs related to the console subsystem. util Issues and PRs related to the built-in util module. labels Dec 15, 2016
@mscdex mscdex added feature request Issues that request new features to be added to Node.js. and removed console Issues and PRs related to the console subsystem. labels Dec 15, 2016
@silverwind
Copy link
Contributor

silverwind commented Dec 15, 2016

Not sure if aliasing %i to the current%d is a good idea. In all browsers that I've tested, those two convert to integer (while providing %f as a means to format floats). In node,%d serves a double purpose of supporting both float and integer.

%f behaviour is inconsistent between browsers. For console.log('%f', 1.5) Safari and Firefox print '1.500000' while Chrome does '1.5'.

I'd propose the following steps to fix:

  • Make %d convert to integer and alias %i to it. This would be a breaking change.
  • Introduce %f with once we're clear what to do about the 0 padding.

@Fishrock123 Fishrock123 added the console Issues and PRs related to the console subsystem. label Dec 15, 2016
@Fishrock123
Copy link
Contributor

Fishrock123 commented Dec 15, 2016

As mentioned in #1716 (comment) we don't implement the console "spec" and honestly I don't think we are really about to. :/

We could do %f if it's reasonable I guess, honestly don't see much reason to not support %i in whatever way we think it should work either.

@jsinmotion
Copy link
Author

jsinmotion commented Dec 15, 2016

I agree that there is no point in chasing a "spec" that doesn't exist, but this is a case where the major browser vendors agree!

As of 2016 Dec 15 this is the current conversion specifier support:

conversion chrome ie firefox node 6.9.1
%d Integer Integer Integer Number
%i Integer Integer Integer
%s String String String String
%f Float Float Float
%o Object Object Object
%O Object Object
%b Binary
%x hex
%e exp
%c N/A
%j JSON

IMHO nodejs should support %d %i %s %f %o in the same way that other browsers support the feature. The others, like IE's %x for hex, probably aren't critical.

This table is based on the following browser vendor resources:

EDIT: originally supported following browsers use of %o but that would be confusing since printf uses %o for octal integers.

@Fishrock123
Copy link
Contributor

Well, there is definitely a "spec" but it's not really official to the language by any means: https://github.com/DeveloperToolsWG/console-object/

@jsinmotion
Copy link
Author

The browsers actually do something wholly unexpected with the string conversions:

console.log("%s", {toString:function() { return "foo"; }})

My expectation is that "%s" would use the appropriate toString method, so that the output would be "foo". nodejs actually does what I would expect. Oddly, Chrome and Safari print "Object" rather than calling the object's toString.

Since I think node is actually doing the right thing here, I will close now and try to bring this discussion to the relevant browser vendors before revisiting in node

@silverwind
Copy link
Contributor

silverwind commented Dec 16, 2016

Let's leave this open, there's certainly room for improvement on our side, and I like you nice table :)

@jsinmotion
Copy link
Author

@silverwind The confounding factor related to the trailing digits in the floating format is support for the printf width and precision fields. Firefox has limited support but Chrome completely ignores those fields. If you want to see a difference in FF, try console.log('%.1f', 1.5) -- that should print 1.5 in FF. If I had to guess, chrome's behavior for %f is closest to the POSIX %.17g.

Since there are NPM modules like printj that fully replicate the POSIX printf spec, it seems fair to argue that node core should only support basic substitutions and people should fall back to userland code for heavy lifting.

@robertkowalski @terinjokes since you are mentioned in the whatwg spec, maybe you can chime in.

@silverwind
Copy link
Contributor

silverwind commented Dec 16, 2016

@jsinmotion thanks. I too think we should start at basic support for %f. Control over decimals can come after that. I went with parseFloat for now as it's consistent to what Chrome does and what the WHATWG spec says:

https://console.spec.whatwg.org/#formatting-specifiers

@terinjokes
Copy link

@Fishrock123 While not an official part of the language, browsers have been pretty keen on working with the WHATWG console standard. We'd love it if Node.js can support what it can, and perhaps provide feedback on what it can't.

@jsinmotion At this point, I don't think there's been any discussion on supporting POSIX (or Firefox)-like width/precision. If it's something you're really interested in, perhaps open a ticket for us to chat with interested parties.

@silverwind I support adding %i and %f to match console in the browsers. It would be nice if Node.js supports %d the same way.

@Fishrock123
Copy link
Contributor

@terinjokes Is there a good place to have active cross-collaboration? We are sort of stuck with some of the things we have, and work in a very different environment which renders some things such as .clear() or differentiated .warn() impossible to implement.

@Fishrock123
Copy link
Contributor

I guess just at https://github.com/whatwg/console? Some of the issues seem kinda outdated there with relatively little discussion.

@terinjokes
Copy link

@Fishrock123 Nope, that's the place. It's more active than it looks. Compared to Node.js, probably significantly less active, but there's also a much smaller group of people desiring to work on it. 😄

@terinjokes
Copy link

terinjokes commented Dec 19, 2016

@Fishrock123 Please note that "differentiated .warn()" isn't actually part of the specification. The Printer concept is intentionally underspecified.

The printer operation is implementation-defined.

If you can differentiate, then awesome, but if you can't, then do something that makes sense.

As for .clear() we actually already handle the Node.js case

If possible for the environment, clear the console. Otherwise, do nothing.

@silverwind
Copy link
Contributor

I'd certainly welcome us following a spec, but there seems to be some opposition within the node collaborators to get mildly breaking changes like %d for integers in.

Can we get a decision from @nodejs/ctc on whether we can follow this spec?

@jsinmotion
Copy link
Author

jsinmotion commented Dec 19, 2016

@terinjokes I think the spec needs some clarifications first: for example, the spec does not define format specifier so it is unclear whether implementations should handle cases like %6.9f. I will raise those and share my opinions in https://github.com/whatwg/console repo; it seems off-topic to have prolonged discussions about the console spec here.

+1 to nodejs following a version of the console spec, or at least supporting the features that the browsers agree upon. I'd be willing to put in some time on the spec front if the node collaborators want to see something clarified. Looked into the situation, made an effort, concluded that it is a waste of time unless the console group is willing to treat the entire server-side use case on parity with the client-side use case. Better off sticking to what makes sense

@Fishrock123
Copy link
Contributor

I'm willing to work with a spec, but only if it's for us too, we need to be able to work on and influence it like a browser can. Attempting to conform to it without that is just putting ourselves into a hole haha. :)

I have hopes, it seems positive, but I also have past reservations.

@terinjokes
Copy link

@Fishrock123 I understand, and I'm glad to have your feedback. See you over there!

@terinjokes
Copy link

@jsinmotion While I understand your frustration (or perhaps a disappointment) with how implementers handled console.assert, I don't understand why you aren't willing to work with me on other mutual problems. I would have been extremely happy to get the feedback of the format specifiers you've given above.

As more and more developers handle both sides of the stack, and as one that has to do so, I'd expect that they'll prefer closer convergence, rather than the two objects drifting farther away.

@jsinmotion
Copy link
Author

@terinjokes the real problem, as evidenced in that thread and the response, is logical:

  • one browser makes a mistake
  • every other browser copies the mistake
  • node does the correct thing
  • because of the numbers (N browsers vs node), node is expected to copy the mistake

That is a logical problem whereby the numbers certainly point to a resolution in which node copies bugs from browsers. And that seems silly and I fervently disagree with that idea. The right thing is to push the browsers to fix their bugs. I liked the particular example of console.assert abecause no one in the browser development teams actually thought about consulting the Firebug documentation when ripping the bug. And if repeating the bug is the recommendation it's better to forge on and do what is sensible rather than going down the rabbit hole of repeating browser bugs.

@silverwind I originally commented that I supported implementing something paralleling the browser's %o %O for objects. This clashes with printf and as a result I am -1 for %o %O but still +1 to %d %i %f behavior aligning with printf

@sam-github
Copy link
Contributor

As more and more developers handle both sides of the stack, and as one that has to do so, I'd expect that they'll prefer closer convergence, rather than the two objects drifting farther away.

@terinjokes

console.log() in browsers is a debug tool for sticking messages in debug window that is viewable ONLY by developers.

console.log() in node is a way of printing to the terminal, the actual user, or to whever the stdout has been directed.

The fact that you can write some code that will call console.log in the browser and in the terminal doesn't mean that the code will have anything even vaguely like the same behaviour - node completely lacks a dev tools-like debug tool.

Are you personally writing code that uses console, and find their behaviours to be meaningfully equivalent? What kind of code?

@diegoug
Copy link

diegoug commented Jan 4, 2017

For me it is useful the chrome inspector attached to nodejs, to control the flow of the code, execute functions and fix problems.

I think you are missing things of usability, for example when there is an execution error, the console fails, together with node and does not recover, this should auto-load the inspector, something similar to what node-inspector does.

@sam-github
Copy link
Contributor

for example when there is an execution error, the console fails, together with node and does not recover

could you elaborate what you mean by this?

this should auto-load the inspector

and by this? how would a remote node micro service instance "load the inspector" on an uncaught exception?

@jsinmotion
Copy link
Author

@sam-github should you ever use console.log for printing data to stdout? The docs are confusing here:

https://nodejs.org/dist/latest-v7.x/docs/api/console.html

The console module provides a simple debugging console that is similar to the JavaScript console mechanism provided by web browsers.

I interpret that to mean console should only be used for debugging purposes and that process.stdout.write should always be favored for non-debugging output.

@diegoug
Copy link

diegoug commented Jan 4, 2017

@sam-github ok I'll be more specific, when I work the integration of the V8 Inspector for Node.js with the flag inspect, nodejs gives me a url in console to place in the browser and to be able to use the Chrome DevTools connected to Node.js, then when there is a code error in the code flow, nodejs is blocked showing an error in console and Chrome DevTools stop working just showing a console error, example of an intentional error:

Docker command:
/bin/sh -c "cd /opt/app; python manage.py runserver 0.0.0.0:${DJANGO_PORT}"

node console:

Debugger listening on port 8081.
Warning: This is an experimental feature and could change at any time.
To start debugging, open the following URL in Chrome:
    chrome-devtools://devtools/remote/serve_file/@60cd6e859b9f557d2312f5bf532f6aec5f284980/inspector.html?experiments=true&v8only=true&ws=127.0.0.1:8081/61186fa1-36e9-4ffb-b8ac-242066354b14
Debugger attached.
/opt/nodejs/server.js:14
var tal = cosa();
          ^

ReferenceError: cosa is not defined
    at Object.<anonymous> (/opt/nodejs/server.js:14:11)
    at Module._compile (module.js:570:32)
    at Object.Module._extensions..js (module.js:579:10)
    at Module.load (module.js:487:32)
    at tryModuleLoad (module.js:446:12)
    at Function.Module._load (module.js:438:3)
    at Timeout.Module.runMain [as _onTimeout] (module.js:604:10)
    at ontimeout (timers.js:365:14)
    at tryOnTimeout (timers.js:237:5)
    at Timer.listOnTimeout (timers.js:207:5)
Waiting for the debugger to disconnect...

Chrome DevTools:
Uncaught ReferenceError: cosa is not defined(…)

For a case of an error what I have to do is: restart nodejs, take the new url of the terminal and load it in the browser, this process tires a little, it would be very useful that nodejs will restart automatically, without disconnecting the web socket, starting again The flow, and if it has the flag debug-brk use it.

Another case is: if you make a code change, restart nodejs and the Chrome console is reloaded with the same or a new web socket

This with node-inspector + forever can be done, the problem with this library is that it is quite unstable, tends to break when changing node version or simply stops working for no apparent reason.

@sam-github
Copy link
Contributor

sam-github commented Jan 4, 2017

@diegoug That doesn't have anything to do with console, AFAICT, it looks like you are thinking that its the printing to stdout that is causing node to die, when node has already (tried) to die, actually, and the stdout messages are just its attempt to tell you why. I don't use debuggers for node, but it looks like whichever one you are using lacks a feature, the ability to trap uncaught exceptions, and keep node alive enough for the debug protocol to be used to inspect its near-death state

@jsinmotion That text is relatively recent compared to the existence of the console, introduced only in #4428, and I think you are reading too much into it, anyhow. Its not even particularly accurate. Node does not "provide a debugging console"... even if you use console.log() solely for debugging (and there are reams and reams of CLIs around that use it for formatted I/O, since streams don't have formatted I/O), its the terminal that offers the (incredibly rough) equivalent of a "debugging console", not node.

And anyway, those docs are in a state of disrepair.

I'm going to open a bug on this. EDIT: #10617

@Trott
Copy link
Member

Trott commented Jan 11, 2017

@silverwind The ctc-agenda has been removed. If you think this needs a decision from the CTC (and I'm guessing you do, but I don't know for sure), you may want to add it back and say so.

silverwind added a commit to silverwind/node that referenced this issue Mar 25, 2017
This change brings formatting specifiers available in `util.format` and
consequently, `console.*` closer to what is supported in all major
browsers. There is a breaking change with the `%d` specifier which
previously served a double purpose of formatting both integer and
floats. With this change, it will format only as integer.

- `%d` is being changed to format only as integer.
- `%i` is introduced as an alias to `%d`.
- `%f` is introduced to format floating point values.

When updating code, all instances of `%d` format strings that
were supplied with floats should be changed to the `%f` format string.

Fixes: nodejs#10292
@silverwind
Copy link
Contributor

%i and %f have been added. %d still formats as Number which is incompatible with browsers for floats, but there were concerns about backward compatibilty on that one.

%o looks like a candidate we could possibly support too, reopening for this purpose.

@silverwind silverwind reopened this Mar 29, 2017
@silverwind silverwind reopened this Mar 29, 2017
@silverwind silverwind changed the title console.log and util.format should support the %i integer conversion console.log and util.format formatting specifiers Mar 29, 2017
italoacasas pushed a commit to italoacasas/node that referenced this issue Apr 10, 2017
This change brings formatting specifiers available in `util.format` and
consequently, `console.*` closer to what is supported in all major
browsers.

- `%i` is introduced to format integer values.
- `%f` is introduced to format floating point values.

Fixes: nodejs#10292
PR-URL: nodejs#10308
Reviewed-By: James M Snell <[email protected]>
@Trott
Copy link
Member

Trott commented Jul 30, 2017

I'm going to close this and open a separate issue for %o.

@Trott Trott closed this as completed Jul 30, 2017
@Trott
Copy link
Member

Trott commented Jul 30, 2017

#14545

MylesBorins pushed a commit that referenced this issue Jan 16, 2018
This change brings formatting specifiers available in `util.format` and
consequently, `console.*` closer to what is supported in all major
browsers.

- `%i` is introduced to format integer values.
- `%f` is introduced to format floating point values.

Fixes: #10292
PR-URL: #10308
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
console Issues and PRs related to the console subsystem. feature request Issues that request new features to be added to Node.js. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging a pull request may close this issue.