Merge lp:~vila/bzr/316357-SIGWINCH into lp:bzr

[Vincent Ladeuil]

[Vincent Ladeuil (vila)]

Due to popular demand (see https://bugs.launchpad.net/bugs/316357), I've put this patch up for review.

bzr will now listen to the SIGWINCH signal but it may reveal dormant bugs in parts of the code that don't yet handle EINTR.

Feedback and bug reports welcome: don't sit bored anymore watching the progress bar during those long test suite runs, shake your window size, that will shake the bzr code :)

[Martin Packman (gz)]

This change needs to be if-nix onlyed. SIGWINCH doesn't exist on windows, and this is not a neat hack there due to lack of size-shaking. Changes to the console window size are communicated using WINDOW_BUFFER_SIZE_RECORD from ReadConsoleInput, which isn't in the bazaar loop currently.

review: Needs Fixing

[Martin Pool (mbp)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

 vote needs-fixing

Aside from Martin.gz's comment it looks reasonable to me.

> Changes to the console window size are communicated using WINDOW_BUFFER_SIZE_RECORD from ReadConsoleInput, which isn't in the bazaar loop currently.

Maybe on Windows we should just call _win32_terminal_size every time
we want to know?

- - --
Martin <http://launchpad.net/~mbp/>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iEYEARECAAYFAksnQd8ACgkQPGPKP6Cz6It9uQCfTkNUg4vS6Pi4DogMGgjy7OBZ
MUsAn3bd9NBHDMRYk4GTXTwMx/llmE48
=doaZ
-----END PGP SIGNATURE-----

review: Needs Fixing

[Martin Packman (gz)]

> Maybe on Windows we should just call _win32_terminal_size every time
> we want to know?

That's perfectly acceptable for this kind of application, yes. (Or even just assuming it doesn't change - it's not very easy for the user to fiddle with after all).

Also, please make the signal import lazy, will save me doing that in one of my branches after this lands and breaks it.

=== modified file 'NEWS'
--- NEWS	2009-12-15 19:59:00 +0000
+++ NEWS	2009-12-16 10:53:22 +0000
@@ -141,6 +141,9 @@
 
 * Interactive merge doesn't leave branch locks behind.  (Aaron Bentley)
 
+* Listen to the SIGWINCH signal to update the terminal width.
+  (Vincent Ladeuil, #316357)
+
 * Lots of bugfixes for the test suite on Windows. We should once again
   have a test suite with no failures on Windows. (John Arbash Meinel)
 

=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py	2009-12-14 16:16:05 +0000
+++ bzrlib/osutils.py	2009-12-16 10:53:22 +0000
@@ -39,6 +39,7 @@
 from shutil import (
     rmtree,
     )
+import signal
 import subprocess
 import tempfile
 from tempfile import (
@@ -1427,6 +1428,20 @@
     _terminal_size = _ioctl_terminal_size
 
 
+def _terminal_size_changed(signum, frame):
+    """Set COLUMNS upon receiving a SIGnal for WINdow size CHange."""
+    width, height = _terminal_size(None, None)
+    if width is not None:
+        os.environ['COLUMNS'] = str(width)
+
+if sys.platform == 'win32':
+    # Martin (gz) mentioned WINDOW_BUFFER_SIZE_RECORD from ReadConsoleInput but
+    # I've no idea how to plug that in the current design -- vila 20091216
+    pass
+else:
+    signal.signal(signal.SIGWINCH, _terminal_size_changed)
+
+
 def supports_executable():
     return sys.platform != "win32"
 

=== modified file 'bzrlib/ui/text.py'
--- bzrlib/ui/text.py	2009-12-14 06:05:30 +0000
+++ bzrlib/ui/text.py	2009-12-16 10:53:22 +0000
@@ -248,9 +248,6 @@
         self._term_file = term_file
         # true when there's output on the screen we may need to clear
         self._have_output = False
-        # XXX: We could listen for SIGWINCH and update the terminal width...
-        # https://launchpad.net/bugs/316357
-        self._width = osutils.terminal_width()
         self._last_transport_msg = ''
         self._spin_pos = 0
         # time we last repainted the screen
@@ -267,9 +264,11 @@
 
     def _show_line(self, s):
         # sys.stderr.write("progress %r\n" % s)
-        if self._width is not None:
-            n = self._width - 1
-            s = '%-*.*s' % (n, n, s)
+        width = osutils.terminal_width()
+        if width is not None:
+            # we need one extra space for terminals that wrap on last char
+            width = width - 1
+            s = '%-*.*s' % (width, width, s)
         self._term_file.write('\r' + s + '\r')
 
     def clear(self):