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

Remove support for hsv, hwb, ansi and css keyword colors and add .rgbToAnsi256(), .hexToRgb() and .hexToAnsi256() #65

Merged
merged 18 commits into from
Dec 1, 2020

Conversation

Richienb
Copy link
Contributor

@Richienb Richienb commented Nov 21, 2020

Remove color-convert in favour of rgb, hex and ansi256 formats and add .rgbToAnsi256(), .hexToRgb() and .hexToAnsi256().

Related to chalk/chalk#300

Signed-off-by: Richie Bendall <[email protected]>
@Qix-
Copy link
Member

Qix- commented Nov 21, 2020

Let's confirm the gameplan before merging this, but this looks like an OK start. I will have some requests about the API though since this is going to be a breaking change anyway - namely that we probably shouldn't follow the from_module.to_model() design anymore and come up with something a bit more ergonomic.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Member

The PR title should reflect the user-facing changes, not an internal implementation detail.

@sindresorhus
Copy link
Member

I will have some requests about the API though since this is going to be a breaking change anyway - namely that we probably shouldn't follow the from_module.to_model() design anymore and come up with something a bit more ergonomic.

What do you have in mind?

@Richienb Richienb changed the title Remove color-convert Remove support for hsv, hwb, ansi and css keyword colours Nov 22, 2020
Richienb and others added 5 commits November 22, 2020 22:34
@Qix-
Copy link
Member

Qix- commented Nov 23, 2020

What do you have in mind?

In tight loops the double object lookup could add overhead. We could get away with simpler conversion functions at the flat level, e.g. ansi256toTruecolor().

Also, we don't need ansi2ansi anymore, for example - those were just API fillers if I recall.

@sindresorhus
Copy link
Member

In tight loops the double object lookup could add overhead. We could get away with simpler conversion functions at the flat level, e.g. ansi256toTruecolor().

👍🏻

@Richienb Richienb changed the title Remove support for hsv, hwb, ansi and css keyword colours Remove support for hsv, hwb, ansi and css keyword colours and add top-level color conversion functions Nov 24, 2020
@Richienb Richienb changed the title Remove support for hsv, hwb, ansi and css keyword colours and add top-level color conversion functions Remove support for hsv, hwb, ansi and css keyword colours, accept an array for ansi16m() and add top-level color conversion functions Nov 24, 2020
@Richienb Richienb changed the title Remove support for hsv, hwb, ansi and css keyword colours, accept an array for ansi16m() and add top-level color conversion functions Remove support for hsv, hwb, ansi and css keyword colours, only accept an array for ansi16m() and add top-level color conversion functions Nov 24, 2020
@Richienb Richienb changed the title Remove support for hsv, hwb, ansi and css keyword colours, only accept an array for ansi16m() and add top-level color conversion functions Remove support for hsv, hwb, ansi and css keyword colours, only accept an array for ansi16m() and add .rgbToAnsi256(), .hexToRgb() and .hexToAnsi256() Nov 24, 2020
@Richienb
Copy link
Contributor Author

We should drop support for Node.js 8 because it doesn't support named regex capture groups.

@Qix-
Copy link
Member

Qix- commented Nov 24, 2020

Sure; node 8 is EOL so that's fine.

readme.md Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.d.ts Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
Richienb and others added 2 commits November 26, 2020 02:02
Co-authored-by: Sindre Sorhus <[email protected]>
@Richienb Richienb changed the title Remove support for hsv, hwb, ansi and css keyword colours, only accept an array for ansi16m() and add .rgbToAnsi256(), .hexToRgb() and .hexToAnsi256() Remove support for hsv, hwb, ansi and css keyword colours and add .rgbToAnsi256(), .hexToRgb() and .hexToAnsi256() Nov 25, 2020
Signed-off-by: Richie Bendall <[email protected]>
Signed-off-by: Richie Bendall <[email protected]>
@Richienb
Copy link
Contributor Author

The build is now failing because tsd uses Typescript 3.9 whereas labelled tuple elements were added in Typescript 4.0.

@sindresorhus
Copy link
Member

That's ok. A new tsd version will be out in a couple of days.

@sindresorhus sindresorhus changed the title Remove support for hsv, hwb, ansi and css keyword colours and add .rgbToAnsi256(), .hexToRgb() and .hexToAnsi256() Remove support for hsv, hwb, ansi and css keyword colors and add .rgbToAnsi256(), .hexToRgb() and .hexToAnsi256() Dec 1, 2020
@sindresorhus sindresorhus merged commit 9150f61 into chalk:master Dec 1, 2020
@sindresorhus
Copy link
Member

Thanks for working on this 🙌🏻

@sindresorhus
Copy link
Member

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