Node: doc: confusing language in util.format() docs

[q42jaap]

https://github.com/nodejs/node/commit/214d02040e1c46b8cac47451bf2f0e2fc15a3e0f

The intention to speed up util.format breaks the following case:

util.format('%%')

prior to this commit, this returned '%', now it returns '%%'.

Imho, this is a bug and should be reverted.

[q42jaap]

This works correctly in node 0.12
broken in node 4 and up

[evanlucas]

to me, if util.format('%%') is supposed to return %, then util.format('%s') would return s. I don't really think the previous return value is intuitive. At this point, I'm not sure changing it back would be worth it. Is there a scenario where this is causing a problem for you?

[mscdex]

FWIW it's only an issue when you don't pass any additional arguments to format().

[addaleax]

The current behaviour also matches the one that browsers have in console.log, fwiw.

@evanlucas The idea is been that if strings like %s are used as placeholders, then %% is the way to escape a percent sign. It’s been that way since printf() was invented, and that’s what most languages do, so I would suggest to not deviate in Node. (edit: maybe I misunderstood you – I assumed you were talking about the formatting function of util.format for multiple arguments)

[aqrln]

If the first argument is not a format string then util.format() returns a string that is the concatenation of all arguments separated by spaces. Each argument is converted to a string using util.inspect().

(https://nodejs.org/dist/latest-v7.x/docs/api/util.html#util_util_format_format_args)

I'd rather we clarify it here that if a string is the only argument, it cannot be a format string by definition, so it is returned as is.

[evanlucas]

@addaleax I probably should have clarified. I was thinking only in the case of a single argument.

[addaleax]

Yeah, I realized too late that I probably misunderstood you. :smile:

[aqrln]

So how about making it a good first contribution documentation issue (https://github.com/nodejs/node/issues/12362#issuecomment-293580572)?

[q42jaap]

I still think, that util.format should behave more like printf in C than console.log in javascript.

[q42jaap]

The case is winston logging a sql query with wildcards WHERE column LIKE '%string_with_s%' which I had to escape to WHERE column LIKE '%%string_with_s%%'.
winston calls util.format.apply(null, args), which in my case uses just 1 parameter.

Given that there wasn't a testcase allowing the performance improvement to break code unnoticed, I'd still opt for printf behavior over console.log.

[q42jaap]

#include <stdio.h>

int main() {
  printf("%%\n");
}

will print % not %%.

[bnoordhuis]

What printf() does is irrelevant though, util.format() is used to implement console.log() and that prints '%%' in browsers and node. Moving to close.

[q42jaap]

@bnoordhuis the docs mention printf specifically... So it is definitely not irrelevant. Furthermore, the bug was unintentionally introduced in given commit. There should have been a testcase.
Other than the mentioning of printf, the docs lay out a clear path for how tokens are processed. %% is a token that does not consume an argument and just prints %.

This is just wrong!

[bnoordhuis]

I'm willing to concede the docs are wrong (didn't check) but the behavior itself brings node closer to browsers. I see no reason to roll that back. "It's not like printf" is not a compelling argument to me.

[Fishrock123]

I guess this is a docs issue?

[aqrln]

Yeah, definitely a docs issue IMO. I think the current behavior is correct given that util.format() is just a backend for console.log() in the first place and it has never been a printf() counterpart (that falls outside the scope of Node core and it's quite feasible to do npm install printf or other userland package, I believe there should be a plenty of them). Not to mention that it's better to be incompatible with EOL v0.12 than with modern Node.js versions that have been released for the past two years.

[tarunbatra]

Hey, I'd like to help with this! 🙋
Just need to know if we'd want to remove the reference to printf, apart from documenting the single argument scenario?

[aqrln]

@tarunbatra great, go for it! You can ping me here, in #node-dev on Freenode or via direct messages on Twitter (username is the same) if you need any help.

I'd say printf() references are fine as long as it is clear that when the first argument is considered to be a format string, it has printf-like-ish (though far from what printf can do) format, nothing more. If it occurs to be hard to formulate it concisely, I'm fine with removing references to printf() completely and saying just "format string" instead of "printf-like format string". I think I would even prefer it :)

Open a PR though, and reviewers will help find the best wording :)

[tarunbatra]

Thanx @aqrln! I created #12374.

I skipped the changes to printf() thinking that it's reference helps devs from other backgrounds to get instantly familiar with the method. Will change it if you feel so.