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.

Sign up for GitHub

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

Jump to bottom

Add support for TrueColor #78

Merged
merged 2 commits into from
Jul 14, 2020
Merged

Add support for TrueColor #78

merged 2 commits into from
Jul 14, 2020

Conversation

MichaelAquilina
Copy link
Collaborator

@MichaelAquilina MichaelAquilina commented May 18, 2020 •
edited
Loading

Add support for specifying true colors

@kurtlawrence

@MichaelAquilina MichaelAquilina force-pushed the feat/truecolor_support branch 4 times, most recently from 4696b93 to 89953c5 Compare May 18, 2020 19:52
@MichaelAquilina
Copy link
Collaborator Author

Any chance this could get looked at @kurtlawrence?

@kurtlawrence
Copy link
Collaborator

Hi @MichaelAquilina,

Sorry for the slow response, I plan on getting some time to review all the PRs in the repo in the next month or so.

I had a cursory look into the PR, changing to_bg_str and to_fg_str to return String would introduce unnecessary allocations. I would ask that the return type be changed to Cow<str>.

This PR is similar to what @tforgione in #6 has done, with similar issues surrounding the allocations. When I get to review this PR I will be looking into the forks and consider the best way forward that:

  1. Minimises API changes, and
  2. Maintains the least overhead

I understand that many people would like this feature and I appreciate their paitence.

@tforgione tforgione mentioned this pull request May 23, 2020
Open
mackwic
mackwic requested changes May 26, 2020
View reviewed changes
Copy link
Collaborator

@mackwic mackwic left a comment

Choose a reason for hiding this comment

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

It also needs a major version bump, but nice changes ! 👍 :)

src/color.rs Outdated Show resolved Hide resolved
@kurtlawrence kurtlawrence merged commit 18b5ff8 into colored-rs:master Jul 14, 2020
@MichaelAquilina MichaelAquilina deleted the feat/truecolor_support branch July 14, 2020 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mackwic mackwic mackwic requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MichaelAquilina @kurtlawrence @mackwic