chalk / slice-ansi

Slice a string with ANSI escape codes
MIT License
47 stars 21 forks source link

Get all ansi escape sequences #25

Closed TiagoDanin closed 4 years ago

TiagoDanin commented 5 years ago

Summary:

Fix

Closes #14 Closes #22 Closes #24

TiagoDanin commented 5 years ago

@sindresorhus Could you review this PR ?

Qix- commented 5 years ago

Replace default end code 39m ~> 0m (Work with "true color", background & foreground)q

Why? 0m also resets all other modes, which is not the SE functionality. 39 works with truecolor just fine. If you want to reset the background, also specify 49.

TiagoDanin commented 5 years ago

@Qix- Only returns 0m in case you don't find the end code. https://github.com/chalk/slice-ansi/blob/71f382bc0bd13c1fa1f0a4139ece40f2d3ce63e5/index.js#L63

Qix- commented 5 years ago

@TiagoDanin You can change it to just m. 0m is redundant.

TiagoDanin commented 5 years ago

Did you see the diff? All and True Color is closed with correct end codes (test)

If ansiStyles.codes.get(parseInt(ansiColor, 10)) fails, the return will be 0m (Previously 39m).

Qix- commented 5 years ago

But my point is 39m works with true color.

TiagoDanin commented 5 years ago

Examples:

A comparison with 39m vs 0m

39m 0m
Finalize only foreground color Finalize all colors
Do not work with background Work with background
Do not work with style modifiers Work with style modifiers

NOTE¹: There is background in true color (39m is not valid). NOTE²: 0m is best option in case of failures

Qix- commented 5 years ago

I'm quite lost as to what you mean.

39m is "reset foreground color". It works with the 38;2;r;g;bm code just fine. The same for 48;2;...m and 49m.

What is it you're claiming does not work?

TiagoDanin commented 5 years ago

What is it you're claiming does not work?

Nothing! See the tests

Old

(39m will only work with foreground).

In https://github.com/chalk/slice-ansi/blob/865b0d68a309d4ade7f5100b417ab5f00ed53af8/index.js#L51 if the ansiStyles.codes.get(parseInt(escapeCode, 10)) returns undefined.

in https://github.com/chalk/slice-ansi/blob/865b0d68a309d4ade7f5100b417ab5f00ed53af8/index.js#L51 wrapAnsi is calling with value END_CODE (END_CODE === 39).

New

(0m work with all)

In https://github.com/chalk/slice-ansi/blob/71f382bc0bd13c1fa1f0a4139ece40f2d3ce63e5/index.js#L57, if the ansiStyles.codes.get(parseInt(ansiColor, 10)) returns undefined.

in https://github.com/chalk/slice-ansi/blob/71f382bc0bd13c1fa1f0a4139ece40f2d3ce63e5/index.js#L67 wrapAnsi is calling with value 0

sindresorhus commented 5 years ago

NOTE²: 0m is best option in case of failures

What are "failures"? When would that occur. I also don't see any added tests for when 0m is used. Can you add that?

sindresorhus commented 4 years ago

Nice work! :)

sindresorhus commented 4 years ago

@TiagoDanin Since you're deep into ansi escape codes now, maybe you wanna also do https://github.com/chalk/wrap-ansi/issues/35 ;)