ANSI escape codes bug

[danielchatfield]

It is possible that I am just looking at this wrong but I'm 90% sure this code is wrong.

fmt = '  \u001b[9' + c + 'm' + name + ' '
  + '\u001b[3' + c + 'm\u001b[90m'
  + fmt + '\u001b[3' + c + 'm'
  + ' +' + humanize(ms) + '\u001b[0m';

What this is doing is making the output pretty colours.

c holds the number corresponding to a colour (https://en.wikipedia.org/wiki/ANSI_escape_code#Colors)

Since the 9c codes aren't actually standard (I haven't come across something which doesn't implement them though) it is understandable to include the equivalent 3c code before the 9c one as if the 'Intense' colour spec hasn't been implemented then it will display the 'non-intense' version instead. This is what I think the above code is meant to be doing but it isn't since the 3c escape code is in the wrong place (if a system didn't implement the intense spec then the message would be colored and the name would not be which is the opposite of what it should be).

Another improvement would be to use hexadecimal e.g. \u001b[36m -> \x1b[36m as it is smaller.

This is my suggestion:

fmt = '  \x1b[3' + c + 'm\x1b[9' + c + 'm' + name + ' '
  + '\x1b[30m\x1b[90m'
  + fmt + '\x1b[3' + c + 'm'
  + ' +' + humanize(ms) + '\x1b[0m';

I wanted to get some feedback before submitting a pull request.

[TooTallNate]

I'm not sure if this defensive output is necessary. On top of the \x... string syntax doesn't work with strict mode, so what we have right now seems just fine to me. Thanks for the suggestion, but closing.

[danielchatfield]

On top of the \x... string syntax doesn't work with strict mode, so what we have right now seems just fine to me.

The \x syntax does work with strict mode, what you currently have (octals) does not (another reason to change to this syntax).

[danielchatfield]

The bug that I was initially describing has since been fixed (the current code is slightly different) but as per my comment above you are mistaken about what will and won't work in strict mode.

[TooTallNate]

@danielchatfield I think we're both partially right (and wrong). Both /u0001 and \x01 syntax work fine in strict mode.

It's \251 syntax that doesn't work :)

[TooTallNate]

http://jslinterrors.com/dont-use-octal-a-use-instead

[TooTallNate]

But considering that the \u0001 syntax is what Node.js itself is using, I think I'm not gonna change it at this point.

[danielchatfield]

:+1:

[matthewmueller]

We just came across something that doesn't implement \u001b[9: https://papertrailapp.com/ :-D

Any chance we could switch this to the standard coloring?

[TooTallNate]

It should be literally the same bytes being written to the socket though. What is the standard coding you speak of?

[matthewmueller]

Oh hmm... Haven't looked too much into it, but I was referring to @danielchatfield's original post:

Since the 9c codes aren't actually standard (I haven't come across something which doesn't implement them though) it is understandable to include the equivalent 3c code before the 9c one as if the 'Intense' colour spec hasn't been implemented then it will display the 'non-intense' version instead

[danielchatfield]

If you look at the "SGR (Select Graphic Rendition) parameters" section of https://en.wikipedia.org/wiki/ANSI_escape_code you can see that the 90-97 parameters are not standard and some things (e.g. papertrailapp) have not implemented them. It is thus sensible to do the following:

[standard colour (30-37)][high intensity colour(90-97)]text[reset]

So that if something doesn't support the 90-97 parameters it will still be coloured but with the non-intense version.