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

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

So arguably, we need to change the command a bit, and add a flag that says whether we want to check stderr or stdout. So that progress bars stay the right width even when redirecting, but 'bzr log --line' gives the wide info.

Note that some of the motivation was also because of pagers. And "bzr log --line | less" goes to a terminal, even though "bzr log --line > file" does not. AFAIK there is no way to determine in process whether stdout is a pipe rather than a file. And even if there was, what do you do for "bzr log --line | tee file" ? As that goes both to the terminal and to a file. (Note this is even worse for getting the file encoding right on windows, as default file content encoding != default terminal encoding != filename encoding. So doing "bzr log > file && gvim file you would want content encoding, but bzr log | less you would want terminal encoding." Probably one of the few reasons to spawn our own pager, as then we know it is still going to a term...)

Anyway, I feel that the current patch is 'good enough'. It allows BZR_COLUMNS if people really need to tweak the width. It respects COLUMNS if available, and still uses TIOCGWINSZ.

review: Approve

« Back to merge proposal