Code review comment for lp:~vila/bzr/353370-notty-no-term-width

Revision history for this message
John A Meinel (jameinel) wrote :

So vila and I discussed this a bit on IRC, I'll try to summarize my part.

1) I'm concerned about respecting $COLUMNS even when redirecting. For example doing "bzr log --line >file", I probably would expect it to not truncate any lines. Note that this differs from what people probably want when they do "bzr log --line | less".

2) In general, I feel like most of our "truncated output" goes to stderr rather than stdout. Which means that checking stdout is going to give incorrect results. (The only thing we could come up with was 'bzr log --line' that truncates to stdout, everything else is stuff like 'progress' that goes to stderr.)

Of course, the bug #353370 is *explicitly* about 'bzr log --line | less' getting truncated. It would indicate (to me) that we should be detecting the terminal width on the file-handle that we are going to be writing out the truncated data.

3) 256 rather than 65536 seems fine to me. The specific width when things aren't truncated doesn't matter as long as it is "big enough".

I suppose I'm a bit concerned as to what output we are generating that causes vila to see the log files balloon into GB. Perhaps the progress bars are being redirected to the output file? (And thus the 'clean the bar' step is generating 64kB each time. Though, I don't think we want progress output in our debug logs anyway...

« Back to merge proposal