medikoo / cli-color

Colors and formatting for the console
ISC License
676 stars 34 forks source link

Allow "color inside color". #17

Closed rentalhost closed 9 years ago

rentalhost commented 9 years ago

Example:

cli.whiteBright("Hello " + cli.blackBright("World") + "!");

Turns to:

<whiteBright>Hello </whiteBright><blackBright>World</blackBright><whiteBright>!</whiteBright>

Currently it turns to:

<whiteBright>Hello </whiteBright><blackBright>World</blackBright><white>!</white>

image

medikoo commented 9 years ago

@rentalhost What cli-color does is:

[SWITCH FG TO white:bright]Hello [SWITCH FG TO black:bright]World[SWITCH FG TO default]![SWITCH FG TO default]

This is also how ansi coloring works, and it's not how HTML works, and there's no instruction like SWITCH FG TO PREVIOUS available, so we can make it somehow work as in HTML.

So solution is not to nest same types of instructions. What you want should be configured as:

cli.whiteBright("Hello ") + cli.blackBright("World") + cli.whiteBright("!");
rentalhost commented 9 years ago

Yes, I do know.

But imagine that you have a function like:

function printMessage(message) {
    process.stdout.write(cli.blackBright("[hh:ii:ss] ") + cli.whiteBright("Message: " + message + "!!!") + "\n");
}

If you call like printMessage("your message"), it'll print like:

<bb>[05:23:00] </bb><wb>Message: your message!!!</wb>\n

But if you call it like printMessage(cli.cyan("Hello World")), it'll do:

<bb>[05:23:00] </bb><wb>Message: </wb><c>your message</c><w>!!!</w>\n

It'll use "white" instead of maintain the previous color "before close tag".

ANSI probably will not support it, but string can be parsed to detect "back to previous color", maybe?

I imagine something like, every method (like white, whiteBright) set an internal stack, if it goes to end, it pop and back to previous color stack.

- white(): stack "white", print "whiteAnsi" and message; pop "white";
- white(): stack "white", print "whiteAnsi" and message;
    - whiteBright(): stack "whiteBright", print "whiteBrightAnsi" and message; pop "whiteBright";
    - pop "white";
medikoo commented 9 years ago

@rentalhost again, please understand ansi is not xml-like and doesn't work like you describe. There's no nesting possible, there's no "back to previous color" instruction available.

rentalhost commented 9 years ago

Yes, I know it.

But the result string can be checked if color was applied until last character, based on "the end code". For instance (I spaced to better read):

cli.blackBright("abc") == \x1B[90m  abc  \x1B[39m

So the \x1B[39m is kind of the "end of last fore color" (and \x1B[49m is of background color). If the outer color detect the start of a new color, it need close and start the new color, expect it end, and write outer color again... For instance:

cli.blackBright("123") == \x1B[90m  123 \x1B[39m
cli.whiteBright("ABC") == \x1B[97m  ABC  \x1B[39m
                          ^ starts here  ^ ends here

If I do:

cli.whiteBright("ABC" + cli.blackBright("123") + "!!!")

Currently I will have:

\x1B[97m  ABC \x1B[90m  123 \x1B[39m !!!  \x1B[39m
^ wB start    ^ bB start    ^ bB end      ^ wB end, useless

But it whiteBright close it BEFORE of next color, and restart AFTER the end of this color, the result will be:

\x1B[97m  ABC \x1B[39m  \x1B[90m  123  \x1B[39m  \x1B[97m  !!!  \x1B[39m
^ wB start    ^ wB end  ^ bB start     ^ bB end  ^ wB restart   ^ wB end

How can it be done: when the whiteBright() have been called, it will starts with the whiteBright code only, without print message. We need search indexOf the start of a new color on message first. If not found, so write message and end of color (single color); else, write message until start of next color, and write end of current color, copy message until the end of the last color, and, if exists more message, restart current color, and close it again.

I do a live example: http://jsfiddle.net/d7x639zc/

medikoo commented 9 years ago

@rentalhost it makes sense. I'll take into consideration. I just wonder whether just appending another color start after each \x1B[39m is safe for all possible cases. It needs to work so there's no side effects, and so deep nesting work as expected. It may not be as straightforward as it seems.

I'll reopen issue for now

StreetStrider commented 9 years ago

Hello, everyone. Couple of days ago I was thinking about this issue and its practical applicability. And now I'm facing the issue in which this feature can helps.

Consider system where you need to build strings from chucks come from different sources. Result string must support coloring, so, any chunk must support it too. If chunks are just concatenated then everything is ok. But if result string has its own styling too, it will be overriden by the styles of first chunk in it.

If say in common, any utility where strings can be styled both by system and by the API user nested styles must be supported, for the sake of composability.

I'm not sure how this feature should be correctly implemented, maybe it will has gross impact on current codebase. If we can't do it, then ok. However, I want to help, if help is required or participate in discussion.

StreetStrider commented 9 years ago

The prototype I'm working on is console-ultimate, in design, fully-customizable colored console (with other features, exept coloring, too). Here's the example. Every logging method can has prefix option: a custom string which will be prefixed to method's each output. Also, every method can also has color option: a cli-color function (or a composition) or any formatting function which applied to whole method's output. So when prefix has cli-color customizations it overwrites color customizations.

I understand, this is how ansi works, but I can wrap whole result in some cli-color function, which would resolve nested styles.

rentalhost commented 9 years ago

@StreetStrider I don't know if I understand you very well... Can you post more examples about what you talking about?

StreetStrider commented 9 years ago

@rentalhost Take a look at this code:

var
    join = [].join,
    colr = require('cli-color');

var console =
{
    prefix: ' * ',
    color: colr.blue,

    log: function log ()
    {
        var output = join.call(arguments, ' ');
        output = console.prefix + output + '\n'
        output = console.color(output);
        process.stdout.write(output);
    }
}

/* run output */
console.log('blue value'); /* ok */

/* reconfigure styles and run output */
console.prefix = colr.red(' * ');
console.log('red prefix, blue value'); /* brokes */

/* now user want to add colored chunk */
console.prefix = ' * ';
console.log(colr.red('red value'), 'blue value'); /* brokes */

This is simplified version of what I'm trying to achieve with console-ultimate. The example's output brokes because of nested styles are not possible.

rentalhost commented 9 years ago

First example: current version (work ok because you use a single color):

[blue] * blue value\n[/blue]

Second example: current version (fail because cli-color don't stop and restart after inside color, what was suggested to fix on this issue):

[blue][red] * [/red][white]red prefix, blue value[/white][/blue]

The "blue" color was stopped by "red" prefix incorrectly, and not was restarted after end of "red" color. The correct will be:

[red] * [/red][blue]red prefix, blue value[/blue]

Third example: current version (same case):

[blue] * [red]red value[/red][white] blue value[/white][/blue]

The "blue" again was not closed properly and not was restarted after "red" color. Correct will be:

[blue] * [/blue][red]red value[/red][blue] blue value[/blue]
StreetStrider commented 9 years ago

@rentalhost, yes, you've got it correct. The second and the third examples fail.

rentalhost commented 9 years ago

If @medikoo don't try fix it, I'll try create a PR on this weekend to fix it. At least to work in dev, your development will can help fix possible issues.

medikoo commented 9 years ago

Fix for that would be controversial, as it will require parsing and tokenizing every input string, and that will definitely affect performance. I'm just not sure how serious concern that would be, it might be a minor assuming there's no use case to log large strings sequentially in fraction of seconds. It will also require a major version bump

rentalhost commented 9 years ago

I'll make some tests about performance. But I guess that it'll not affect a lot because are, in general, simple operations (except by regular expression). 100.000/second is a good time? I'll try do something in this time, if possible.

StreetStrider commented 9 years ago

@medikoo. Hmm, I think you're right, it cannot be done in current approach without expensive parsing-tokenizing process. So, sadly, this is not the thing cli-color should (and can) solve.

My original idea was in creating special «context» in which colorizing functions works different:

clc.context(function (clc) {
  return clc.red('red text ' + clc.blue('blue text') + 'red text again');
});

But it seems that it also requires parsing. And it does not solves problem when styled string comes from user. If you have fully-controlled string it can be constructed without context, otherwise it has external chunks and they use unmodified functions, so context does not work properly.

I was trying to make these functions work lazily (lisp-style), so we can apply format at final stage at once (like eval for colorizing), but it failed.

rentalhost commented 9 years ago

It'll not be expensive, because the "parsing" process will occur only if message contain some color definition (basically, if found \x1B). The .context() can confuse more than solve. cli-color need support that as native feature, not as extension, in my mind.

With a native method, if colored message comes from user, it'll be solved naturally (unless you can prove otherwise, what would be good for the study).

StreetStrider commented 9 years ago

if message contain some color definition

But this would happen in the majority of cases, because we use cli-color for colors, isn't we?

You disliked the .context(), that's ok. It's not the perfect solution. I just was trying to limit spreading of unobvious behavior, which is nesting styles. For users who don't use context everything just remains the same. For them this wouldn't be a breaking change even.

rentalhost commented 9 years ago

On really, not. For example: cli.red(" * ") don't will have \x1B because it have only a message that will be printed in red (this process will add \x1B, but only after the check).

But if you nest colors, it'll happen;

.red(" * ");                        # <red> * </red>
.blue("Test " + .red(" * ") + "!"); # <blue>Test </blue><red> * </red><white>!</white> (current)
.blue("Test " + .red(" * ") + "!"); # <blue>Test </blue><red> * </red><blue>!</blue> (fixed)

When you call cli.blue(), you expect that your message turns blue (expect by the nested color red, that you should expect that "only" this part turns red).

medikoo commented 9 years ago

It'll not be expensive, because the "parsing" process will occur only if message contain some color definition (basically, if found \x1B).

True, because first we may just check for existence of control string, and tokenize only if we find to be there, and that will happen only in rare custom cases. In light of that, it seems as good feature to add

StreetStrider commented 9 years ago

It is great we can do this. So, major will be incremented?

rentalhost commented 9 years ago

I vote to only increase the minor version. Major version generally mean "breaking change" or a rewrite. As in this case don't will happen a breaking change, so not need increase major.

StreetStrider commented 9 years ago

@rentalhost, actually this is a breaking change, due to some people can rely on «broken» straightforward behavior. This chance is low, but technically this is a compability breaker. Unless, we'll implement it with feature-gate or some kind of context. In that case it is a minor.

rentalhost commented 9 years ago

@StreetStrider well... it sounds like a "bug fix" for me, not like a feature, because if you call blue(message) do you expect that your message be blue. But the developer really can use it incorrectly, and he "accepted" that the after the nested color back to white, instead of continues in blue color (as is implicit).

Another possibility is, in 0.4, turn required you turn on this feature before you use. So, if you are only updating from 0.3 to 0.4, you will not be affected. But on 1.0 it can be turn default option.

medikoo commented 9 years ago

@StreetStrider It's a new feature (not a bugfix), that changes some behaviour, on which some programs may have relied on. It requires major bump.

rentalhost commented 9 years ago

@medikoo (off-topic) Do you plan to rewrite all code to 1.0? Not that I think that it is necessary, it is only to know.

medikoo commented 9 years ago

@rentalhost I don't think so. I think codebase will remain about same. Still it'll be good to jump into 1.x, using 0.x seems deprecated idea.

One idea I had is to make project cross environment, so e.g. it can also work in browser console (some do support coloring), but I didn't yet give it a real thought, in the end it might appear as overkill

rentalhost commented 9 years ago

Cool! Seems like Firefox and Chrome supports CSS style in console.log(). I don't know about that. http://stackoverflow.com/questions/7505623/colors-in-javascript-console

And I think that is a good idea cross-env support.

The only detail is that perhaps most of the plugins go not use both features at the same time (for instance, a cross-env plugin that use console to display something).

rentalhost commented 9 years ago

I have some news... I'm working on a lot of features that I suggest here, but I found some specifics problems.

First, check it here

It implements:

Things to do:

About parser changes (index.js):

About a nested style problem:

I found a problem that need be discussed. If I have it:

clc.bgBlue('blue ' + clc.bgRed('red ' + clc.whiteBright('white ') + 'red ') + 'blue');

What will be the foreground color after the whiteBright? Currently the parser maintain the last color, and reset it only on end of string. So the last red and blue will be whiteBright and not white (default color).

My question is: what is the right behavior in this case?

I think that in this case, the ideal would be to go back to the previous color, but as nothing has been defined before, should use the default color (white), and not maintain the last used color.

StreetStrider commented 9 years ago

I think that I broked the spaces, because I use 4 spaces instead of tab, and I noted just now that you use tabs, right?

Add to config:

"draw_white_space": "all",

And you'll never miss such things.

As for your question. The color stops affecting at this point:

... clc.whiteBright('white ')
                            ^ 

and, as you mentioned, nothing was defined before. I think if we have color ending and no upper-level colors we should clear it (is it possible?). white is not the default value. There's black-on-white terminals, and, potentially, any other combinations.

rentalhost commented 9 years ago

Clear, in case, is do a reset like ESC[39;m right? If yes, I'm working on it.

medikoo commented 9 years ago

Addressed and published as v1.0.0