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

Accept true color escape sequences #18

Merged
merged 4 commits into from
Jan 25, 2019
Merged

Conversation

piranna
Copy link
Contributor

@piranna piranna commented May 15, 2018

The slice only pick 4 bytes, not enough for longer escape sequences. This change increase it to 18, allowing to accept escape sequences for true color definitions. I have not used an unbounded limit to don't check the regular expression against probably very long strings.

Fix #17

The slice only pick 4 bytes, not enough for longer escape sequences. This change increase it to 18, allowing to accept escape sequences for true color definitions. I have not used an unbounded limit to don't check the regular expression against probably very long strings.

Fix chalk#17
@sindresorhus
Copy link
Member

Can you add a test?

@sindresorhus
Copy link
Member

Ping :)

@viddo
Copy link

viddo commented Nov 26, 2018

Just ran into the same problem, and found this PR while troubleshooting the code. I can confirm this fix working by modifying the installed source in node_modules. 👍

@sindresorhus while a test is missing for the code change, it seems like the last CI test failure was due to some xo offenders unrelated to this PR?

@piranna
Copy link
Contributor Author

piranna commented Nov 29, 2018

I have just merged the test provided by @viddo.

test.js Outdated Show resolved Hide resolved
Co-Authored-By: piranna <piranna@gmail.com>
@sindresorhus sindresorhus merged commit f4f3cc3 into chalk:master Jan 25, 2019
@piranna
Copy link
Contributor Author

piranna commented Jan 26, 2019

Thank you :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants