Cows. Cows everywhere.

[shanecelis]

Great project. I saw an opportunity to alloc less. This PR changes input from a String to a Cow<'a, str> and the implementation for &'a str to Into<Cow<'a, str>> which will cover &str as before and String and Cows and makes the necessary changes to support that. No usage change for the end user, just broader support and less allocs.

As an aside, I added print statements to the dynamic_colors example.

[kpcyrd]

Suggested change

	#[allow(unused_mut)]
	#[allow(unused_assignments)]
	let mut tty = false;
	#[cfg(feature = "tty")]
	{
	tty = atty::is(atty::Stream::Stdout);
	}
	let tty = if cfg!(feature = "tty") {
	atty::is(atty::Stream::Stdout)
	} else {
	false
	};

[shanecelis]

Good idea. I like this better than mine but it doesn't compile. This compiles:

        let tty = if cfg!(feature = "tty") {
            #[cfg(feature = "tty")]
            {
            atty::is(atty::Stream::Stdout)
            }
            #[cfg(not(feature = "tty"))]
            false
        } else {
            false
        };

But we can clean it up further to this:

        let tty = {
            #[cfg(feature = "tty")]
            {
                atty::is(atty::Stream::Stdout)
            }
            #[cfg(not(feature = "tty"))]
            false
        };

And sorry there are some commits that I didn't mean to put into this PR.

[kpcyrd]

You could also use

use std::io::{self, IsTerminal};
let tty = io::stdout().is_terminal();

that was very recently introduced in the Rust 1.70.0 std library.

[shanecelis]

Could use that and drop the atty dependency.

[shanecelis]

I reverted the changes that weren't meant to go into this PR (removed a struct called ColoredStrings). And I found that tests weren't working when I varied between "cargo test", "cargo test --all-features", and "cargo test --no-default-features". It seemed like the path of least resistance was to change the negating feature "no-color" to a default feature of "color", which is what Cargo recommends probably so things like "all-features" will work. I apologize for if this is intrusive or presumptuous.

[hwittenborn]

Thanks for the PR @shanecelis! I've been invited on as a contributor to overlook some things, and I'll get this PR look at once I get a bit more familiar with the codebase. I just need to get a better grasp on how everything's working internally, and then I'll be able to look over this code and give it a good review.

[hwittenborn]

Hey @shanecelis, there's plans for a v3 of colored where #139 would be implemented, and because of that I'm probably going to hold off on getting this merged.

Once that issue gets implemented, it should avoid any allocations entirely, which this PR seems to aim for but not entirely achieve. I do like the idea of allocating less, but I'm not positive it'll make much of a difference in the actual speed/memory usage of a program. Would you have any metrics for that?

[kurtlawrence]

#154 intends to expose the internals of ColoredString, with the intention of having mutability of the underlying string.

A common pattern seems to be having the colouring and styling orthogonal to the backing string.

I think a good way forward would be to create a generic structure which is agnostic to the backing string, such as

struct Colored<T> {
    input: T,
    fgcolor: Option<Color>,
    bgcolor: Option<Color>,
    style: style::Style,
}

And then implement the required conversions, dereferencing, and formatting agnostic to the backing type.