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

--color never not supported for error output #353

Closed
andreastt opened this Issue Feb 9, 2017 · 11 comments

Comments

Projects
None yet
4 participants
@andreastt

andreastt commented Feb 9, 2017

My shell does not support colours and consequently I set the environmental variable TERM=dumb to instruct programs never to colourise output.

However, even when I pass --color never to rg, the error output includes colour escape codes as can be seen below:

% rg --color never -asd
�[1;31merror:�[0m Found argument '�[33m-d�[0m' which wasn't expected, or isn't valid in this context

USAGE:
    
    rg [OPTIONS] <pattern> [<path> ...]
    rg [OPTIONS] [-e PATTERN | -f FILE ]... [<path> ...]
    rg [OPTIONS] --files [<path> ...]
    rg [OPTIONS] --type-list

For more information try �[32m--help�[0m
@kbknapp

This comment has been minimized.

Contributor

kbknapp commented Feb 9, 2017

@andreastt that's from claps (rg's CLI parser) error message, not rg's.

I think there's a hacky way to workaround this...but it's hacky. You'd essentially be parsing the CLI twice in the case of a CLI error and then double checking that --color never. An abbriviated version is something like:

let mut app = build_cli();
let args: Args = match app.get_matches_safe() {
    Err(e) => {
        if (env::args().find(|arg| arg == "--color").is_some() && env::arg().find(|arg| arg == "never")) || env::args().find(|arg| arg == "--color=never") {
            app.setting(AppSettings::ColorNever).get_matches();
        } 
        e.exit();
    },
    Ok(m) => m.into(),
};

There's probably a far better way to do all that, but I'm going a lack of sleep and 8 plane rides 😜

@BurntSushi

This comment has been minimized.

Owner

BurntSushi commented Feb 9, 2017

@kbknapp Yeah, probably the "right" way here is for clap to recognize TERM=dumb, which should mean "no colors." (It's pretty standard in my experience, and termcolor will Do The Right Thing.)

@kbknapp

This comment has been minimized.

Contributor

kbknapp commented Feb 9, 2017

@andreastt if you wouldn't mind, could you file an issue in claps repo too? This is something that I'd like to at least attempt to address on that side of things. Actually if you could just post a comment in clap-rs/clap#836 as a reminder to me, that's fine!

@kbknapp

This comment has been minimized.

Contributor

kbknapp commented Feb 9, 2017

@BurntSushi yep, once I get some time to sit down and actually implement clap-rs/clap#836 that's my exact plan 😉

@BurntSushi

This comment has been minimized.

Owner

BurntSushi commented Feb 9, 2017

@kbknapp While you're at it, note that the atty crate now supports MSYS terminals and native Windows consoles. I've wondered whether termcolor itself should use the atty crate directly.

@andreastt

This comment has been minimized.

andreastt commented Feb 9, 2017

Filed clap-rs/clap#847 with clap.

@BurntSushi

This comment has been minimized.

Owner

BurntSushi commented May 8, 2017

If someone wants to fix this using @kbknapp's hack, then I'd be open to it. It might be hard to write a regression test for it though.

@nateozem

This comment has been minimized.

nateozem commented May 13, 2017

Sure, I'll take a stab at it.

@nateozem

This comment has been minimized.

nateozem commented May 17, 2017

There's a PR for clap-rs to comply with terminfo when TERM=dumb (clap-rs/clap#963).
Anything else needs to be done? Still prefer to be colorless if --color never?

@BurntSushi

This comment has been minimized.

Owner

BurntSushi commented May 17, 2017

@nateozem Let's try to get away with TERM=dumb support instead of introducing a hack.

@BurntSushi

This comment has been minimized.

Owner

BurntSushi commented Oct 22, 2017

It looks like the clap PR was merged so I'm marking this as fixed. (You still can't use --color never, but TERM=dumb should be respected.)

@BurntSushi BurntSushi closed this Oct 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment