Code review comment for lp:~spiv/bzr/sigwinch-without-signalmodule-583941

Revision history for this message
Andrew Bennetts (spiv) wrote :

Martin Pool wrote:
> On 24 May 2010 16:02, Andrew Bennetts <email address hidden> wrote:
>
> > However, we want to get notified when the terminal size changes,
> > which means either asking the OS every time we want to know the
> > terminal size (perhaps not so bad?), or side-stepping the 'signal'
> > module in Python.  This patch takes the latter route.
>
> Perhaps we should try the former route? (I'm kind of sorry to say
> that after you've already done this, but on the other hand history
> suggests this may not be the final fix, and it's not all that small.)
>
> I suppose the only drawback is that it may slow us down if we do it
> really often. On the other hand I think the progress code already
> tries to throttle updates to some extent. Could you try something
> with timeit to see just how expensive it would be?

I'll take a look. I can't easily time the different code path taken by
win32, but obviously SIGWINCH never applied there anyway.

I know the _ioctl_terminal_size isn't going to be über-efficient, being
an ioctl and struct packing and unpacking, but as you say we probably
don't do it often.

« Back to merge proposal