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

test: Fix chalk tagged template literals to use nested chalk tags #4265

Merged
merged 2 commits into from
Jul 9, 2019

Conversation

ExE-Boss
Copy link
Contributor

@ExE-Boss ExE-Boss commented Jun 7, 2019

Calls like:

chalk`{red  red }{red.bold  red & bold }{red red again }`

will always produce:

\e[31m red \e[39m\e[31m\e[1m red & bold \e[22m\e[39m\e[31m red again \e[39m

as Chalk is unable to know that it’s meant to be nested, whereas:

chalk`{red  red {bold  red & bold } red again }`

can be optimised to produce:

\e[31m red \e[1m red & bold \e[22m red again \e[39m

as Chalk knows that the bold style is a sub‑section of the whole red style (note that this isn’t yet fully implemented).

The second call is also a lot cleaner.

@ExE-Boss ExE-Boss requested a review from Elchi3 as a code owner June 7, 2019 20:40
@queengooborg queengooborg added the linter Issues or pull requests regarding the tests / linter of the JSON files. label Jun 7, 2019
Copy link
Contributor

@queengooborg queengooborg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for cherry-picking, this all looks good to me! 👍 I'll leave the final review for a repository owner. 😉

ExE-Boss added a commit to ExE-Boss/mdn-browser-compat-data that referenced this pull request Jun 10, 2019
Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, focused work and nice explanation of the change as well. Very well done, @ExE-Boss 👍

@Elchi3 Elchi3 merged commit 558093f into mdn:master Jul 9, 2019
@ExE-Boss ExE-Boss deleted the test/fix-chalk-tagged-template-literals branch July 10, 2019 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linter Issues or pull requests regarding the tests / linter of the JSON files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants