search

rust-lang / rustup

The Rust toolchain installer
https://rust-lang.github.io/rustup/
Apache License 2.0
6.13k stars 880 forks source link

Rustup output should end with a newline #2243

Open djmitche opened 4 years ago

djmitche commented 4 years ago

Problem Rustup's output ends with a  after its last newline.

But command-line tools are typically expected to end their output with a newline.

In a CLI context, this doesn't matter as that character sequence "resets" the color without producing visible output. But in a log-parsing context this does matter, as the next line has this weird escape-sequence prefix. This caused a (minor) bug here. The output looked like

info: (Bdownloading component 'rustfmt'
info: (Binstalling component 'rustfmt'
[taskcluster 2019-06-10 16:36:58.754Z] === Task Finished ===
[taskcluster 2019-06-10 16:36:58.755Z] Successful task run with exit code: 0 completed in 238.573 seconds
djmitche commented 4 years ago

(also I'm just the messenger here -- hopefully folks on that bug can chime in if more details are required)

kinnison commented 4 years ago

I imagine that this is related to how our terminal hilighting code works. It ought to not colourise its output if it's not outputting to a terminal. If your log gathering is using a pseudoterminal to force certain output then it will need to parse ANSI escapes etc in order to deal effectively with this kind of behaviour.

It's probable that you could adjust our output code to fix it, though really your log parser should be taught to deal with the escapes.

I'll leave this open because if someone wants to have a go at fixing it, I doubt we'd refuse a patch.

djmitche commented 4 years ago

Thanks for leaving this open. The display of the output in the browser does interpret ANSI (the above snippets are from the 'raw' view), but tools like grep don't, so I think this is a legitimate request.

rbtcollins commented 4 years ago

As @kinnison said, it's probably that this happens because you've got a tty attached to your stdout+stderr. Generally if you wish to be able to grep content, it shouldn't be an interactive terminal, but rather a non-interactive terminal.

Can you confirm that this is happening with a non-interactive terminal?

djmitche commented 4 years ago

It doesn't.

kinnison commented 4 years ago

On one hand therefore, I'd be prepared to close this 'wontfix' because really the behaviour is entirely as designed, and if you expect to grep logs from a pseudotty then you need a cleverer version of grep capable of ignoring tty control commands.

On the other hand, if it's possible to simply fix this so that the reset sequence is sent before the last newline then that's not onerous for us to maintain.

As such, I'd like to leave this open and welcome someone to contribute a fix.