Check isatty == false before coloring

[3Hren]

Hi!

I've found this library as an excellent solution to colorize the logging output. However, when the stdout/stderr is redirected somewhere else (./a.out > log.txt for example) the final output is still mangled with terminal special characters as an opposite expected behavior.

It's a common practice in Unix to check isatty function before trying to colorize, see man for more. Moreover the isatty function is associated with terminal as like as this crate, so it may be useful to add it.

I can implement this functionality if you agree. Thoughts?

[Stebalien]

Sounds useful. However, we'd probably want to return an error when the user attempts to create the terminal instead of not colorizing.

[iliekturtles]

Color output may be desired when not attached to a tty (ex, cmd --colored always | less -R). Also keep Windows in mind and give some way to fall back to WinConsole when TerminfoTerminal wont work as expected.

[Stebalien]

Color output may be desired when not attached to a tty (ex, cmd --colored always | less -R).

Good point. However, this is a rare edge case and doesn't even work in WinConsole.

Also keep Windows in mind and give some way to fall back to WinConsole when TerminfoTerminal wont work as expected.

We already do this.

[keeperofdakeys]

A similar issue turned up in another library that uses this project. Perhaps the Terminal object returned by stdout() should have a isatty flag/attribute that can be checked. This would allow the caller to check both if the terminal supports colours, and whether it's being output directly to the terminal.

[Stebalien]

This could require a significant change to the API (although that might not be a bad idea) because there's no way to get a file descriptor/file handle from an Stdout/Stderr object (or Write implementations in general). It might be a good idea to restructure the API such that true Terminals can only be constructed using special stdout()/stderr() functions. If you want to wrap an existing writer, you have to manually construct a TerminfoTerminal. As it stands, the ability to manually construct a WinConsole doesn't even make sense.

[retep998]

Also keep Windows in mind and give some way to fall back to WinConsole when TerminfoTerminal wont work as expected.

In my opinion WinConsole should be the first option tried on Windows. If GetConsoleMode thinks it is a Windows console we should just go with that. Only if that fails should you attempt to fall back to TerminfoTerminal.

[Stebalien]

@retep998 are there any terminals that support both? Unfortunately, the windows console API does not support bold, underline, or italics so I'd rather use terminfo if supported.

[retep998]

In order to support the Windows console API you need to be the windows console. If you are the Windows console then you are incapable of displaying any sort of styling that the Windows console does not support, unless you do incredibly hacky things hooking into the Windows console. I personally don't know of any terminals that are capable of supporting the Windows console API but with the ability to display additional styling.

[tbu-]

@retep998 I read somewhere that Windows 10 consoles support ANSI escape sequences. Would trying terminfo first be fine now?

[retep998]

@tbu- No. The Windows 10 console is still not a terminfo terminal, it is just the Windows console with optional ANSI escape sequence support. Instead you'd first detect whether it is a Windows console, and you'd attempt to set the console mode to enable ANSI escape sequences. If that works, then you can use ANSI escape sequences. If you can't set that console mode but it still is a Windows console then your only option is standard windows console functions

[lukaslueg]

Please don't automagic isatty(). It's the implementor's responsibility to call that function (by means this library should provide) and decide himself if colorbeautification is called for. People may want colors in redirected output; that's why tools like ls and grep have --color switches that go on/off and auto, auto being on if isatty() is true.