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.

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

"reset" doesn't seem to work #1

Closed
ehuss opened this issue Jun 8, 2019 · 7 comments
Closed

"reset" doesn't seem to work #1

ehuss opened this issue Jun 8, 2019 · 7 comments

Comments

@ehuss
Copy link

ehuss commented Jun 8, 2019

The reset sequence ESC [ 0 m doesn't seem to do anything.

Given this string:

"\u{001b}[38;5;14mI am colored\u{001b}[0mI am after reset\n"

Results in just a single color on win7/8:

image

I would submit a PR, but I can't quite figure out what the reset code is trying to do. It sets self.reset, but then does nothing with it. It also doesn't reset expected if it is already reset, which I don't understand.

@mati865
Copy link

mati865 commented Oct 3, 2019

I only can access Linux and Windows 10, can it be reproduced there?

I'm wondering if calling self.spec.clear() somewhere in

fwdansi/src/lib.rs

Lines 156 to 160 in 08466a4

(0, State::Normal) => {
if mem::replace(&mut self.reset, true) {
return false;
}
}
would fix it.

@ehuss
Copy link
Author

ehuss commented Oct 3, 2019

fwdansi doesn't have any platform-specific code, so it should be reproducible by inspecting the output. Here's an example using tests.rs. This fails because Element::Reset is not the same as an empty ColorSpec.

#[test]
fn reset_test() {
    let mut collector = ColorCollector(Vec::new());
    write_ansi(&mut collector, b"\x1b[38;5;14mI am colored\x1b[0mI am reset\n").unwrap();
    let mut cs = ColorSpec::new();
    cs.set_fg(Some(Color::Ansi256(14)));
    assert_eq!(collector.0, &[
        Element::ColorSpec(cs),
        Element::Text(b"I am colored".to_vec()),
        Element::Reset,
        Element::Text(b"I am reset\n".to_vec()),
    ]);

}

#[derive(Debug)]
struct ColorCollector(Vec<Element>);

impl WriteColor for ColorCollector {
    fn supports_color(&self) -> bool { true }
    fn set_color(&mut self, spec: &ColorSpec) -> std::io::Result<()> {
        self.0.push(Element::ColorSpec(spec.clone()));
        Ok(())
    }
    fn reset(&mut self) -> std::io::Result<()> {
        self.0.push(Element::Reset);
        Ok(())
    }
}
impl std::io::Write for ColorCollector {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        let len = buf.len();
        self.0.push(Element::Text(Vec::from(buf)));
        Ok(len)
    }
    fn flush(&mut self) -> std::io::Result<()> {
        Ok(())
    }
}

What's interesting is that the termcolor::Ansi writer handles this fine (it resets before each color). It's only the windows console code which ignores empty ColorSpecs.

cc @BurntSushi, is this maybe a bug in termcolor, too? What I'm seeing is that windows console behaves differently from Ansi. The Ansi writer resets the color for each color command, but the windows writer is accumulative. That is, the following:

    use termcolor::*;
    use std::io::Write;
    let mut stdout = StandardStream::stdout(ColorChoice::Always);
    stdout.set_color(ColorSpec::new().set_fg(Some(Color::Green))).unwrap();
    writeln!(&mut stdout, "green text!").unwrap();
    stdout.set_color(ColorSpec::new().set_bg(Some(Color::Green))).unwrap();
    writeln!(&mut stdout, "green back").unwrap();
    stdout.reset().unwrap();

Results in the ANSI:

image

Whereas on Windows console, it is:

image

Notice that on ANSI, the green background has white text, but in Windows it is green on green. Should this function be more careful about resetting the fg/bg?

@BurntSushi
Copy link

AFAIK, there is no way to reset the console like there is in ANSI. In any case, on modern versions of Windows, the console writer isn't used. ANSI is used instead.

@ehuss
Copy link
Author

ehuss commented Oct 3, 2019

For context, this is trying to fix it so that Windows 7/8 continues to work with Cargo/rustc, since those are still technically tier 1 platforms. fwdansi should be calling reset, but it doesn't (it calls set_color with a ColorSpec with no fg/bg).

I'm not sure what you mean by not being able to reset the console. If I call stdout.reset() in between those two lines, it does what I would expect. Is there some reason write_console can't call console.reset() like the ANSI version does?

@BurntSushi
Copy link

The caveat I'm referring to is here, where there is no actual "reset" API in the console APIs. Resetting is a library implemented feature: https://docs.rs/wincolor/1.0.2/x86_64-pc-windows-msvc/wincolor/struct.Console.html#method.reset

I will be AFK for the next few days so I won't be able to look into this in detail.

@mati865
Copy link

mati865 commented Oct 27, 2019

Ping @BurntSushi were you able to look at this issue?

@BurntSushi
Copy link

No. I have a pretty big backlog right now, and this is pretty low on my list. I would recommend that someone else investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants