Skip to content

Allocate a lot less #202

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

Open
tanculau wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Allocate a lot less #202

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7b06a7b
Replace compute_stlye with a Writer
tanculau Mar 26, 2025
1ad8d71
fmt
tanculau Mar 26, 2025
82fa2c2
Replace loop with insert_str
tanculau Mar 26, 2025
19faeba
Replaced escape_inner_reset_sequences
tanculau Mar 26, 2025
19eec9b
Fix tests
tanculau Mar 26, 2025
0c6f6c3
Cleanup
tanculau Mar 26, 2025
d540f3d
Change Helper Structs to Tuples
tanculau Mar 26, 2025
b217771
Padding without alloc
tanculau Mar 27, 2025
0ae0b78
Enhance formatting tests
tanculau Mar 27, 2025
57c53ea
Enhance formatting tests
tanculau Mar 27, 2025
ecd6355
Cleanup
tanculau Mar 27, 2025
098fa4f
Remove duplicate code
tanculau Mar 27, 2025
34eaf1a
Remove alloc in closest_color_euclidean
tanculau Mar 27, 2025
baf9453
Replace min_by with min_by_key
tanculau Mar 27, 2025
1e04c4d
Add comments
tanculau Mar 27, 2025
962a9ed
Refactor distance calculation in color comparison
tanculau Mar 27, 2025
d5cd420
Add tests to check if fmt and to_str are equal
tanculau Mar 27, 2025
3e7932b
Split up formatting test and added some
tanculau Mar 27, 2025
d2d4d93
Cleanup
tanculau Mar 27, 2025
383397e
Move tests to their modul
tanculau Mar 27, 2025
362f667
Typo
tanculau Mar 27, 2025
2cd3492
Enhance formatting, escape and color_fn tests
tanculau Mar 28, 2025
432ebe7
Remove allocations in Style
tanculau Apr 7, 2025
aae4077
Merge branch 'master' into less-alloc
tanculau Apr 7, 2025
2497bd0
Add doc comments & AnsiColor test
tanculau Apr 7, 2025
a122bea
Reworked to_static_str, Removed Helper structs, moved formatting in i…
tanculau Sep 5, 2025
5270a3c
Merge branch 'master' into less-alloc
tanculau Sep 5, 2025
a561d42
cargo fmt
tanculau Sep 5, 2025
b0778d0
Changed dyn to impl
tanculau Sep 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
209 changes: 157 additions & 52 deletions src/color.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{borrow::Cow, cmp, env, str::FromStr};
use std::{borrow::Cow, env, str::FromStr};
use Color::{
Black, Blue, BrightBlack, BrightBlue, BrightCyan, BrightGreen, BrightMagenta, BrightRed,
BrightWhite, BrightYellow, Cyan, Green, Magenta, Red, TrueColor, White, Yellow,
Expand Down Expand Up @@ -33,55 +33,88 @@ fn truecolor_support() -> bool {

#[allow(missing_docs)]
impl Color {
const fn to_fg_static_str(self) -> Result<&'static str, (u8, u8, u8)> {
match self {
Self::Black => Ok("30"),
Self::Red => Ok("31"),
Self::Green => Ok("32"),
Self::Yellow => Ok("33"),
Self::Blue => Ok("34"),
Self::Magenta => Ok("35"),
Self::Cyan => Ok("36"),
Self::White => Ok("37"),
Self::BrightBlack => Ok("90"),
Self::BrightRed => Ok("91"),
Self::BrightGreen => Ok("92"),
Self::BrightYellow => Ok("93"),
Self::BrightBlue => Ok("94"),
Self::BrightMagenta => Ok("95"),
Self::BrightCyan => Ok("96"),
Self::BrightWhite => Ok("97"),
Self::TrueColor { r, g, b } => Err((r, g, b)),
}
}

pub(crate) fn to_fg_fmt(self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
match self.to_fg_static_str() {
Ok(s) => f.write_str(s),
Err((r, g, b)) if !truecolor_support() => Self::TrueColor { r, g, b }
.closest_color_euclidean()
.to_fg_fmt(f),
Err((r, g, b)) => write!(f, "38;2;{r};{g};{b}"),
}
}

#[must_use]
pub fn to_fg_str(&self) -> Cow<'static, str> {
match *self {
Self::Black => "30".into(),
Self::Red => "31".into(),
Self::Green => "32".into(),
Self::Yellow => "33".into(),
Self::Blue => "34".into(),
Self::Magenta => "35".into(),
Self::Cyan => "36".into(),
Self::White => "37".into(),
Self::BrightBlack => "90".into(),
Self::BrightRed => "91".into(),
Self::BrightGreen => "92".into(),
Self::BrightYellow => "93".into(),
Self::BrightBlue => "94".into(),
Self::BrightMagenta => "95".into(),
Self::BrightCyan => "96".into(),
Self::BrightWhite => "97".into(),
Self::TrueColor { .. } if !truecolor_support() => {
self.closest_color_euclidean().to_fg_str()
}
Self::TrueColor { r, g, b } => format!("38;2;{r};{g};{b}").into(),
match self.to_fg_static_str() {
Ok(s) => s.into(),
Err((r, g, b)) if !truecolor_support() => Self::TrueColor { r, g, b }
.closest_color_euclidean()
.to_fg_str(),
Err((r, g, b)) => format!("38;2;{r};{g};{b}").into(),
}
}
const fn to_bg_static_str(self) -> Result<&'static str, (u8, u8, u8)> {
match self {
Self::Black => Ok("40"),
Self::Red => Ok("41"),
Self::Green => Ok("42"),
Self::Yellow => Ok("43"),
Self::Blue => Ok("44"),
Self::Magenta => Ok("45"),
Self::Cyan => Ok("46"),
Self::White => Ok("47"),
Self::BrightBlack => Ok("100"),
Self::BrightRed => Ok("101"),
Self::BrightGreen => Ok("102"),
Self::BrightYellow => Ok("103"),
Self::BrightBlue => Ok("104"),
Self::BrightMagenta => Ok("105"),
Self::BrightCyan => Ok("106"),
Self::BrightWhite => Ok("107"),
Self::TrueColor { r, g, b } => Err((r, g, b)),
}
}

pub(crate) fn to_bg_fmt(self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
match self.to_bg_static_str() {
Ok(s) => f.write_str(s),
Err((r, g, b)) if !truecolor_support() => Self::TrueColor { r, g, b }
.closest_color_euclidean()
.to_fg_fmt(f),
Err((r, g, b)) => write!(f, "48;2;{r};{g};{b}"),
Copy link
Collaborator

@spenserblack spenserblack Apr 7, 2025

Choose a reason for hiding this comment

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

It looks like you used Result as a clever way to "unwrap" the truecolor if it is one. Here's how I'm thinking it should be implemented instead, for clarity.

// Note that we're now borrowing, since we don't have a reason to take ownership and force a copy
const fn to_bg_static_str(&self) -> &str {
    Self::Black => "40",
    // Implement the other simple colors...
    // Panicking is OK because this is internal
    Self::TrueColor { r, g, b } => unreachable!("Shouldn't be called on True Color"),
}

pub(crate) fn to_bg_fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
    if let Self::TrueColor { r, g, b } = self {
        // Check for support, write value
        // Can possibly be split into a "write_true_color()"
        return Ok(());
    }
    // If we're here, we know the color can be converted to a static string
    f.write_str(self.to_bg_static_str())
}

Copy link
Author

@tanculau tanculau Apr 7, 2025

Choose a reason for hiding this comment

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

My intent was inspired by something like tokio's send method.

I try to convert the color in a static str. If it is possible, I return it, if not, I return rgb (since the color is not statically known).
The reason, I do not need a second match or any unreachable. I personally dislike the use of unreachable if it can be avoided easily.

Also, for borrowing the color, this is an internal api (so no stability needed) and Color is smaller than a pointer. So it is cheaper to hand over the color instead of an pointer. See this clippy::trivially_copy_pass_by_ref

}
}

#[must_use]
pub fn to_bg_str(&self) -> Cow<'static, str> {
match *self {
Self::Black => "40".into(),
Self::Red => "41".into(),
Self::Green => "42".into(),
Self::Yellow => "43".into(),
Self::Blue => "44".into(),
Self::Magenta => "45".into(),
Self::Cyan => "46".into(),
Self::White => "47".into(),
Self::BrightBlack => "100".into(),
Self::BrightRed => "101".into(),
Self::BrightGreen => "102".into(),
Self::BrightYellow => "103".into(),
Self::BrightBlue => "104".into(),
Self::BrightMagenta => "105".into(),
Self::BrightCyan => "106".into(),
Self::BrightWhite => "107".into(),
Self::TrueColor { .. } if !truecolor_support() => {
self.closest_color_euclidean().to_bg_str()
}
Self::TrueColor { r, g, b } => format!("48;2;{r};{g};{b}").into(),
match self.to_bg_static_str() {
Ok(s) => s.into(),
Err((r, g, b)) if !truecolor_support() => Self::TrueColor { r, g, b }
.closest_color_euclidean()
.to_fg_str(),
Err((r, g, b)) => format!("48;2;{r};{g};{b}").into(),
}
}

Expand All @@ -93,7 +126,7 @@ impl Color {
g: g1,
b: b1,
} => {
let colors = vec![
let colors = [
Black,
Red,
Green,
Expand All @@ -115,19 +148,16 @@ impl Color {
.map(|c| (c, c.into_truecolor()));
let distances = colors.map(|(c_original, c)| {
if let TrueColor { r, g, b } = c {
let rd = cmp::max(r, r1) - cmp::min(r, r1);
let gd = cmp::max(g, g1) - cmp::min(g, g1);
let bd = cmp::max(b, b1) - cmp::min(b, b1);
let rd: u32 = rd.into();
let gd: u32 = gd.into();
let bd: u32 = bd.into();
let distance = rd.pow(2) + gd.pow(2) + bd.pow(2);
fn distance(a: u8, b: u8) -> u32 {
u32::from(a.abs_diff(b)).pow(2)
}
let distance = distance(r, r1) + distance(g, g1) + distance(b, b1);
(c_original, distance)
} else {
unimplemented!("{:?} not a TrueColor", c)
}
});
distances.min_by(|(_, d1), (_, d2)| d1.cmp(d2)).unwrap().0
distances.min_by_key(|(_, distance)| *distance).unwrap().0
Copy link
Collaborator

@spenserblack spenserblack Sep 5, 2025

Choose a reason for hiding this comment

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

This is a nice way to clean up the code, but, TBH, I find this unrelated to the intent of this PR (allocating less). Unless this refactor somehow helps with this PR, I'd rather it be in a separate PR. I prefer to keep unrelated changes out of PRs so that they're more reviewable. Let me know if I'm wrong, and if you feel this belongs in this PR.

Copy link
Author

@tanculau tanculau Sep 5, 2025

Choose a reason for hiding this comment

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

Removed the change.
I just added it, since I was already refactoring the code.

}
c => c,
}
Expand Down Expand Up @@ -258,8 +288,83 @@ fn parse_hex(s: &str) -> Option<Color> {

#[cfg(test)]
mod tests {

pub use super::*;

#[test]
fn fmt_and_to_str_same() {
use core::fmt::Display;
use Color::*;

// Helper structs to call the method
struct FmtFgWrapper(Color);
impl Display for FmtFgWrapper {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.to_fg_fmt(f)
}
}
struct FmtBgWrapper(Color);
impl Display for FmtBgWrapper {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.to_bg_fmt(f)
}
}

// Actual test
let colors = &[
Black,
Red,
Green,
Yellow,
Blue,
Magenta,
Cyan,
White,
BrightBlack,
BrightRed,
BrightGreen,
BrightYellow,
BrightBlue,
BrightMagenta,
BrightCyan,
BrightWhite,
TrueColor { r: 0, g: 0, b: 0 },
TrueColor {
r: 255,
g: 255,
b: 255,
},
TrueColor {
r: 126,
g: 127,
b: 128,
},
TrueColor { r: 255, g: 0, b: 0 },
TrueColor {
r: 255,
g: 255,
b: 0,
},
TrueColor { r: 0, g: 255, b: 0 },
TrueColor {
r: 0,
g: 255,
b: 255,
},
TrueColor { r: 0, g: 0, b: 255 },
TrueColor {
r: 255,
g: 0,
b: 255,
},
];

for color in colors {
assert_eq!(color.to_fg_str(), FmtFgWrapper(*color).to_string());
assert_eq!(color.to_bg_str(), FmtBgWrapper(*color).to_string());
}
}

mod from_str {
pub use super::*;

Expand Down
Loading
Loading