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

Revision history for this message
Vincent Ladeuil (vila) wrote :

>>>>> "martin" == Martin Pool <email address hidden> writes:

    martin> I think this is ok to land as it is, but eventually the priority we want is
    martin> 1- BZR_COLUMNS
    martin> 2- TIOCGWINSZ if possible (or the equivalent for win32)
    martin> 3- COLUMNS otherwise
    martin> 4- otherwise None

Damn, I didn't notice that earlier.

Well, I tried that but that's wrong, at least under emacs. That
means at least in one case COLUMNS is more correct than
TIOCGWINSZ (I *think* the rationale is that you can have COLUMNS
smaller than the window size because you use only part of that
window or bigger because the window has scroll bars).

So I reverted to my first proposal and align the tests so they
pass on PQM where things are a bit different too.

You

    martin> also fwiw
    martin> http://www.ohse.de/uwe/software/resize.c.html
    martin> suggests checking TIOCGSIZE too

Hmm. Right it also seem to ignore LINES and COLUMNS to get to the
real terminal size, confirming that COLUMNS is used to override
TIOCGWINSZ, not used as a fallback.

I'll revisit that anyway if anybody raise concerns and include
that while handling the resize window event.

     Vincent

« Back to merge proposal