Skip to content

Commit

Permalink
fix(wincon)!: Make styled output stateless
Browse files Browse the repository at this point in the history
To do this, we use cache the current colors on first check, rather than
re-querying it on every check.

This enables us to just have a simple `WinconStream::write_colored`
function.

This also fixes bugs with making sure that the stream is locked before
doing our various operations on it, avoiding race conditions.
  • Loading branch information
epage committed Sep 27, 2023
1 parent 250d1ff commit 229e0c1
Show file tree
Hide file tree
Showing 14 changed files with 222 additions and 491 deletions.
3 changes: 1 addition & 2 deletions crates/anstream/benches/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ fn stream(c: &mut Criterion) {
group.bench_function("WinconStream", |b| {
b.iter(|| {
let buffer = anstream::Buffer::with_capacity(content.len());
let mut stream =
anstream::WinconStream::new(anstyle_wincon::Console::new(buffer).unwrap());
let mut stream = anstream::WinconStream::new(buffer);

stream.write_all(content).unwrap();

Expand Down
5 changes: 2 additions & 3 deletions crates/anstream/src/auto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,8 @@ where
fn wincon(raw: S) -> Result<Self, S> {
#[cfg(all(windows, feature = "wincon"))]
{
let console = anstyle_wincon::Console::new(raw)?;
Ok(Self {
inner: StreamInner::Wincon(WinconStream::new(console)),
inner: StreamInner::Wincon(WinconStream::new(raw)),
})
}
#[cfg(not(all(windows, feature = "wincon")))]
Expand All @@ -120,7 +119,7 @@ where
StreamInner::PassThrough(w) => w,
StreamInner::Strip(w) => w.into_inner(),
#[cfg(all(windows, feature = "wincon"))]
StreamInner::Wincon(w) => w.into_inner().into_inner(),
StreamInner::Wincon(w) => w.into_inner(),
}
}

Expand Down
24 changes: 4 additions & 20 deletions crates/anstream/src/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,28 +50,12 @@ impl std::io::Write for Buffer {

#[cfg(all(windows, feature = "wincon"))]
impl anstyle_wincon::WinconStream for Buffer {
fn set_colors(
fn write_colored(
&mut self,
fg: Option<anstyle::AnsiColor>,
bg: Option<anstyle::AnsiColor>,
) -> std::io::Result<()> {
use std::io::Write as _;

if let Some(fg) = fg {
write!(self, "{}", fg.render_fg())?;
}
if let Some(bg) = bg {
write!(self, "{}", bg.render_bg())?;
}
if fg.is_none() && bg.is_none() {
write!(self, "{}", anstyle::Reset.render())?;
}
Ok(())
}

fn get_colors(
&self,
) -> std::io::Result<(Option<anstyle::AnsiColor>, Option<anstyle::AnsiColor>)> {
Ok((None, None))
data: &[u8],
) -> std::io::Result<usize> {
self.0.write_colored(fg, bg, data)
}
}
32 changes: 0 additions & 32 deletions crates/anstream/src/is_terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,38 +30,6 @@ impl IsTerminal for std::io::StderrLock<'static> {
}
}

#[cfg(all(windows, feature = "wincon"))]
impl IsTerminal for anstyle_wincon::Console<std::io::Stdout> {
#[inline]
fn is_terminal(&self) -> bool {
self.is_terminal()
}
}

#[cfg(all(windows, feature = "wincon"))]
impl IsTerminal for anstyle_wincon::Console<std::io::StdoutLock<'static>> {
#[inline]
fn is_terminal(&self) -> bool {
self.is_terminal()
}
}

#[cfg(all(windows, feature = "wincon"))]
impl IsTerminal for anstyle_wincon::Console<std::io::Stderr> {
#[inline]
fn is_terminal(&self) -> bool {
self.is_terminal()
}
}

#[cfg(all(windows, feature = "wincon"))]
impl IsTerminal for anstyle_wincon::Console<std::io::StderrLock<'static>> {
#[inline]
fn is_terminal(&self) -> bool {
self.is_terminal()
}
}

impl IsTerminal for std::fs::File {
#[inline]
fn is_terminal(&self) -> bool {
Expand Down
20 changes: 0 additions & 20 deletions crates/anstream/src/lockable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,3 @@ impl Lockable for std::io::Stderr {
(&self).lock()
}
}

#[cfg(all(windows, feature = "wincon"))]
impl Lockable for anstyle_wincon::Console<std::io::Stdout> {
type Locked = anstyle_wincon::Console<std::io::StdoutLock<'static>>;

#[inline]
fn lock(self) -> Self::Locked {
self.lock()
}
}

#[cfg(all(windows, feature = "wincon"))]
impl Lockable for anstyle_wincon::Console<std::io::Stderr> {
type Locked = anstyle_wincon::Console<std::io::StderrLock<'static>>;

#[inline]
fn lock(self) -> Self::Locked {
self.lock()
}
}
34 changes: 16 additions & 18 deletions crates/anstream/src/wincon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub struct WinconStream<S>
where
S: RawStream,
{
console: anstyle_wincon::Console<S>,
raw: S,
// `WinconBytes` is especially large compared to other variants of `AutoStream`, so boxing it
// here so `AutoStream` doesn't have to discard one allocation and create another one when
// calling `AutoStream::lock`
Expand All @@ -23,24 +23,22 @@ where
{
/// Only pass printable data to the inner `Write`
#[inline]
pub fn new(console: anstyle_wincon::Console<S>) -> Self {
pub fn new(raw: S) -> Self {
Self {
console,
raw,
state: Box::default(),
}
}

/// Get the wrapped [`RawStream`]
#[inline]
pub fn into_inner(self) -> anstyle_wincon::Console<S> {
self.console
pub fn into_inner(self) -> S {
self.raw
}

#[inline]
pub fn is_terminal(&self) -> bool {
// HACK: We can't get the console's stream to check but if there is a console, it likely is
// a terminal
true
self.raw.is_terminal()
}
}

Expand All @@ -62,7 +60,7 @@ where
for (style, printable) in self.state.extract_next(buf) {
let fg = style.get_fg_color().and_then(cap_wincon_color);
let bg = style.get_bg_color().and_then(cap_wincon_color);
let written = self.console.write(fg, bg, printable.as_bytes())?;
let written = self.raw.write_colored(fg, bg, printable.as_bytes())?;
let possible = printable.len();
if possible != written {
// HACK: Unsupported atm
Expand All @@ -73,7 +71,7 @@ where
}
#[inline]
fn flush(&mut self) -> std::io::Result<()> {
self.console.flush()
self.raw.flush()
}
}

Expand All @@ -83,7 +81,7 @@ impl Lockable for WinconStream<std::io::Stdout> {
#[inline]
fn lock(self) -> Self::Locked {
Self::Locked {
console: self.console.lock(),
raw: self.raw.lock(),
state: self.state,
}
}
Expand All @@ -95,7 +93,7 @@ impl Lockable for WinconStream<std::io::Stderr> {
#[inline]
fn lock(self) -> Self::Locked {
Self::Locked {
console: self.console.lock(),
raw: self.raw.lock(),
state: self.state,
}
}
Expand All @@ -120,9 +118,9 @@ mod test {
#[cfg_attr(miri, ignore)] // See https://github.com/AltSysrq/proptest/issues/253
fn write_all_no_escapes(s in "\\PC*") {
let buffer = crate::Buffer::new();
let mut stream = WinconStream::new(anstyle_wincon::Console::new(buffer).unwrap());
let mut stream = WinconStream::new(buffer);
stream.write_all(s.as_bytes()).unwrap();
let buffer = stream.into_inner().into_inner();
let buffer = stream.into_inner();
let actual = std::str::from_utf8(buffer.as_ref()).unwrap();
assert_eq!(s, actual);
}
Expand All @@ -131,11 +129,11 @@ mod test {
#[cfg_attr(miri, ignore)] // See https://github.com/AltSysrq/proptest/issues/253
fn write_byte_no_escapes(s in "\\PC*") {
let buffer = crate::Buffer::new();
let mut stream = WinconStream::new(anstyle_wincon::Console::new(buffer).unwrap());
let mut stream = WinconStream::new(buffer);
for byte in s.as_bytes() {
stream.write_all(&[*byte]).unwrap();
}
let buffer = stream.into_inner().into_inner();
let buffer = stream.into_inner();
let actual = std::str::from_utf8(buffer.as_ref()).unwrap();
assert_eq!(s, actual);
}
Expand All @@ -144,15 +142,15 @@ mod test {
#[cfg_attr(miri, ignore)] // See https://github.com/AltSysrq/proptest/issues/253
fn write_all_random(s in any::<Vec<u8>>()) {
let buffer = crate::Buffer::new();
let mut stream = WinconStream::new(anstyle_wincon::Console::new(buffer).unwrap());
let mut stream = WinconStream::new(buffer);
stream.write_all(s.as_slice()).unwrap();
}

#[test]
#[cfg_attr(miri, ignore)] // See https://github.com/AltSysrq/proptest/issues/253
fn write_byte_random(s in any::<Vec<u8>>()) {
let buffer = crate::Buffer::new();
let mut stream = WinconStream::new(anstyle_wincon::Console::new(buffer).unwrap());
let mut stream = WinconStream::new(buffer);
for byte in s.as_slice() {
stream.write_all(&[*byte]).unwrap();
}
Expand Down
17 changes: 9 additions & 8 deletions crates/anstyle-wincon/examples/dump.rs
Original file line number Diff line number Diff line change
@@ -1,32 +1,33 @@
use anstyle_wincon::WinconStream as _;

fn main() -> Result<(), lexopt::Error> {
let args = Args::parse()?;
let stdout = std::io::stdout();
let mut stdout = anstyle_wincon::Console::new(stdout.lock())
.map_err(|_err| lexopt::Error::from("could not open `stdout` for color control"))?;
let mut stdout = stdout.lock();

for fixed in 0..16 {
let style = style(fixed, args.layer, args.effects);
let _ = print_number(&mut stdout, fixed, style);
if fixed == 7 || fixed == 15 {
let _ = stdout.write(None, None, &b"\n"[..]);
let _ = stdout.write_colored(None, None, &b"\n"[..]);
}
}

for r in 0..6 {
let _ = stdout.write(None, None, &b"\n"[..]);
let _ = stdout.write_colored(None, None, &b"\n"[..]);
for g in 0..6 {
for b in 0..6 {
let fixed = r * 36 + g * 6 + b + 16;
let style = style(fixed, args.layer, args.effects);
let _ = print_number(&mut stdout, fixed, style);
}
let _ = stdout.write(None, None, &b"\n"[..]);
let _ = stdout.write_colored(None, None, &b"\n"[..]);
}
}

for c in 0..24 {
if 0 == c % 8 {
let _ = stdout.write(None, None, &b"\n"[..]);
let _ = stdout.write_colored(None, None, &b"\n"[..]);
}
let fixed = 232 + c;
let style = style(fixed, args.layer, args.effects);
Expand All @@ -46,7 +47,7 @@ fn style(fixed: u8, layer: Layer, effects: anstyle::Effects) -> anstyle::Style {
}

fn print_number(
stdout: &mut anstyle_wincon::Console<std::io::StdoutLock<'static>>,
stdout: &mut std::io::StdoutLock<'static>,
fixed: u8,
style: anstyle::Style,
) -> std::io::Result<()> {
Expand All @@ -62,7 +63,7 @@ fn print_number(
});

stdout
.write(fg, bg, format!("{:>4}", fixed).as_bytes())
.write_colored(fg, bg, format!("{:>4}", fixed).as_bytes())
.map(|_| ())
}

Expand Down
7 changes: 4 additions & 3 deletions crates/anstyle-wincon/examples/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ fn main() {

#[cfg(windows)]
fn main() -> Result<(), lexopt::Error> {
use anstyle_wincon::WinconStream as _;

let args = Args::parse()?;
let stdout = std::io::stdout();
let mut stdout = anstyle_wincon::Console::new(stdout.lock())
.map_err(|_err| lexopt::Error::from("could not open `stdout` for color control"))?;
let mut stdout = stdout.lock();

let fg = args.fg.and_then(|c| c.into_ansi());
let bg = args.bg.and_then(|c| c.into_ansi());

let _ = stdout.write(fg, bg, "".as_bytes());
let _ = stdout.write_colored(fg, bg, "".as_bytes());

std::mem::forget(stdout);

Expand Down
25 changes: 25 additions & 0 deletions crates/anstyle-wincon/src/ansi.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//! Low-level ANSI-styling
/// Write ANSI colored text to the stream
pub fn write_colored<S: std::io::Write>(
stream: &mut S,
fg: Option<anstyle::AnsiColor>,
bg: Option<anstyle::AnsiColor>,
data: &[u8],
) -> std::io::Result<usize> {
let non_default = fg.is_some() || bg.is_some();

if non_default {
if let Some(fg) = fg {
write!(stream, "{}", fg.render_fg())?;
}
if let Some(bg) = bg {
write!(stream, "{}", bg.render_bg())?;
}
}
let written = stream.write(data)?;
if non_default {
write!(stream, "{}", anstyle::Reset.render())?;
}
Ok(written)
}
Loading

0 comments on commit 229e0c1

Please sign in to comment.