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
wants to merge 28 commits into
base: master
Choose a base branch
from
Open

Allocate a lot less #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 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
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
1 change: 1 addition & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ features = ["Win32_Foundation", "Win32_System_Console"]
[dev-dependencies]
ansi_term = "0.12"
insta = "1"
itertools = "0.14.0"
rspec = "1"

[lints.rust]
Expand Down
226 changes: 179 additions & 47 deletions src/color.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::{borrow::Cow, cmp, env, str::FromStr};
use core::{cmp, fmt::Write};
use std::{borrow::Cow, convert::Into, env, str::FromStr};
use Color::{
AnsiColor, Black, Blue, BrightBlack, BrightBlue, BrightCyan, BrightGreen, BrightMagenta,
BrightRed, BrightWhite, BrightYellow, Cyan, Green, Magenta, Red, TrueColor, White, Yellow,
Expand Down Expand Up @@ -34,58 +35,120 @@ fn truecolor_support() -> bool {

#[allow(missing_docs)]
impl Color {
/// Converts the foreground [`Color`] into a [`&'static str`](str)
///
/// # Errors
///
/// If the color is a `TrueColor` or `AnsiColor`, it will return [`NotStaticColor`] as an Error
const fn to_fg_static_str(self) -> Option<&'static str> {
match self {
Self::Black => Some("30"),
Self::Red => Some("31"),
Self::Green => Some("32"),
Self::Yellow => Some("33"),
Self::Blue => Some("34"),
Self::Magenta => Some("35"),
Self::Cyan => Some("36"),
Self::White => Some("37"),
Self::BrightBlack => Some("90"),
Self::BrightRed => Some("91"),
Self::BrightGreen => Some("92"),
Self::BrightYellow => Some("93"),
Self::BrightBlue => Some("94"),
Self::BrightMagenta => Some("95"),
Self::BrightCyan => Some("96"),
Self::BrightWhite => Some("97"),
Self::TrueColor { .. } | Self::AnsiColor(..) => None,
}
}

/// Write [`to_fg_str`](Self::to_fg_str) to the given [`Formatter`](core::fmt::Formatter) without allocating
pub(crate) fn to_fg_write(self, f: &mut dyn core::fmt::Write) -> Result<(), core::fmt::Error> {
match self.to_fg_static_str() {
Some(s) => f.write_str(s),
None => match self {
Black | Red | Green | Yellow | Blue | Magenta | Cyan | White | BrightBlack
| BrightRed | BrightGreen | BrightYellow | BrightBlue | BrightMagenta
| BrightCyan | BrightWhite => unreachable!(),
AnsiColor(code) => write!(f, "38;5;{code}"),
TrueColor { r, g, b } if !truecolor_support() => Self::TrueColor { r, g, b }
.closest_color_euclidean()
.to_fg_write(f),
TrueColor { 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::AnsiColor(code) => format!("38;5;{code}").into(),
Self::TrueColor { r, g, b } => format!("38;2;{r};{g};{b}").into(),
self.to_fg_static_str().map_or_else(
|| {
// Not static, we can use the default formatter
let mut buf = String::new();
// We write into a String, we do not expect an error.
let _ = self.to_fg_write(&mut buf);
buf.into()
},
Into::into,
)
}

/// Converts the background [`Color`] into a [`&'static str`](str)
///
/// # Errors
///
/// If the color is a `TrueColor` or `AnsiColor`, it will return [`NotStaticColor`] as an Error
const fn to_bg_static_str(self) -> Option<&'static str> {
match self {
Self::Black => Some("40"),
Self::Red => Some("41"),
Self::Green => Some("42"),
Self::Yellow => Some("43"),
Self::Blue => Some("44"),
Self::Magenta => Some("45"),
Self::Cyan => Some("46"),
Self::White => Some("47"),
Self::BrightBlack => Some("100"),
Self::BrightRed => Some("101"),
Self::BrightGreen => Some("102"),
Self::BrightYellow => Some("103"),
Self::BrightBlue => Some("104"),
Self::BrightMagenta => Some("105"),
Self::BrightCyan => Some("106"),
Self::BrightWhite => Some("107"),
Self::TrueColor { .. } | Self::AnsiColor(..) => None,
}
}

/// Write [`to_bg_str`](Self::to_fg_str) to the given [`Formatter`](core::fmt::Formatter) without allocating
pub(crate) fn to_bg_write(self, f: &mut dyn Write) -> Result<(), core::fmt::Error> {
match self.to_bg_static_str() {
Some(s) => f.write_str(s),
None => match self {
Black | Red | Green | Yellow | Blue | Magenta | Cyan | White | BrightBlack
| BrightRed | BrightGreen | BrightYellow | BrightBlue | BrightMagenta
| BrightCyan | BrightWhite => unreachable!(),
AnsiColor(code) => write!(f, "48;5;{code}"),
TrueColor { r, g, b } if !truecolor_support() => Self::TrueColor { r, g, b }
.closest_color_euclidean()
.to_fg_write(f),
TrueColor { r, g, b } => write!(f, "48;2;{r};{g};{b}"),
},
}
}

#[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::AnsiColor(code) => format!("48;5;{code}").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(),
}
self.to_bg_static_str().map_or_else(
|| {
// Not static, we can use the default formatter
let mut buf = String::new();
// Writing into a String should be always valid.
let _ = self.to_bg_write(&mut buf);
buf.into()
},
Into::into,
)
}

/// Gets the closest plain color to the `TrueColor`
Expand All @@ -96,7 +159,7 @@ impl Color {
g: g1,
b: b1,
} => {
let colors = vec![
let colors = [
Black,
Red,
Green,
Expand Down Expand Up @@ -262,8 +325,77 @@ fn parse_hex(s: &str) -> Option<Color> {

#[cfg(test)]
mod tests {

pub use super::*;

#[test]
/// Test that `fmt` and `to_str` are the same
fn fmt_and_to_str_same() {
Comment on lines +331 to +333
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this test touches on something interesting. the to_*_fmt and to_*_str copy a lot of the same logic.
You've defined wrappers the implement Display here, but, TBH, I think this is a sign to de-duplicate the logic between the format and string methods. The string methods should call the format methods.

But that's probably for a separate PR. The reason I bring it up is that, if I'm reading this correctly, what this tests is "did we copy and paste the logic between the two functions?" I'm not a fan of that type of test. Rather a test like this:

#[test]
fn are_some() {
    // Do foo and bar return the same value?
    assert_eq!(x.foo(), x.bar());
}

I'd rather have tests like this:

#[test]
fn test_foo() {
    assert_eq!(x.foo(), some_expected_value);
}

#[test]
fn test_bar() {
    assert_eq!(x.bar(), some_expected_value);
}

That might look redundant, but redundancy is OK in tests IMO.

Copy link
Author

Choose a reason for hiding this comment

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

I reworked the code, so that there should be as little code duplication as possible.
The problem is, that this is a public interface. As such I cannot change it.

The requirement is, that formatting and the to_str method should always have the same output.
But we cannot write a static str, such that we cannot just call formatting from to_str.
We also want to support precision and advanced formatting. So we also cannot call to_str from the formatting code.
So, if I see it correctly, we have duplicate public interfaces. To check, that both interfaces behave the same, we just call them against each other. So in the end, I only need to test one of them to make sure both work.
The test is also there to protect in the future, if a change occurs, that this requirement is not forgotten.

I'd rather have tests like this:
The problem is, i have to duplicate almost each test case. If a change occurs, the chance of forgetting to update the other one is quite high.

Another option would be to just write a doc comment and hope that everyone reads it.
I think the solution right now should be okay.

use core::fmt::Display;
use itertools::Itertools;
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_write(f)
}
}
struct FmtBgWrapper(Color);
impl Display for FmtBgWrapper {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.0.to_bg_write(f)
}
}

// Actual test

let colors = [
Black,
Red,
Green,
Yellow,
Blue,
Magenta,
Cyan,
White,
BrightBlack,
BrightRed,
BrightGreen,
BrightYellow,
BrightBlue,
BrightMagenta,
BrightCyan,
BrightWhite,
]
.into_iter()
.chain(
// Iterator over TrueColors
// r g b
// 0 0 0
// 0 0 1
// 0 0 2
// 0 0 3
// 0 1 0
// ..
// 3 3 3
(0..4)
.combinations_with_replacement(3)
.map(|rgb| Color::TrueColor {
r: rgb[0],
g: rgb[1],
b: rgb[2],
}),
)
.chain((0..4).map(Color::AnsiColor));

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