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

Proposed by Vincent Ladeuil
Status: Merged
Merged at revision: not available
Proposed branch: lp:~vila/bzr/316357-SIGWINCH
Merge into: lp:bzr
Diff against target: 75 lines (+23/-6)
3 files modified
NEWS (+3/-0)
bzrlib/osutils.py (+15/-0)
bzrlib/ui/text.py (+5/-6)
To merge this branch: bzr merge lp:~vila/bzr/316357-SIGWINCH
Reviewer Review Type Date Requested Status
Martin Pool Needs Fixing
Martin Packman (community) Needs Fixing
Review via email: mp+15939@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

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 :)

Revision history for this message
Martin Packman (gz) wrote :

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
Revision history for this message
Martin Pool (mbp) wrote :

-----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
Revision history for this message
Martin Packman (gz) wrote :

> 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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'NEWS'
--- NEWS 2009-12-15 19:59:00 +0000
+++ NEWS 2009-12-16 10:53:22 +0000
@@ -141,6 +141,9 @@
141141
142* Interactive merge doesn't leave branch locks behind. (Aaron Bentley)142* Interactive merge doesn't leave branch locks behind. (Aaron Bentley)
143143
144* Listen to the SIGWINCH signal to update the terminal width.
145 (Vincent Ladeuil, #316357)
146
144* Lots of bugfixes for the test suite on Windows. We should once again147* Lots of bugfixes for the test suite on Windows. We should once again
145 have a test suite with no failures on Windows. (John Arbash Meinel)148 have a test suite with no failures on Windows. (John Arbash Meinel)
146149
147150
=== 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 @@
39from shutil import (39from shutil import (
40 rmtree,40 rmtree,
41 )41 )
42import signal
42import subprocess43import subprocess
43import tempfile44import tempfile
44from tempfile import (45from tempfile import (
@@ -1427,6 +1428,20 @@
1427 _terminal_size = _ioctl_terminal_size1428 _terminal_size = _ioctl_terminal_size
14281429
14291430
1431def _terminal_size_changed(signum, frame):
1432 """Set COLUMNS upon receiving a SIGnal for WINdow size CHange."""
1433 width, height = _terminal_size(None, None)
1434 if width is not None:
1435 os.environ['COLUMNS'] = str(width)
1436
1437if sys.platform == 'win32':
1438 # Martin (gz) mentioned WINDOW_BUFFER_SIZE_RECORD from ReadConsoleInput but
1439 # I've no idea how to plug that in the current design -- vila 20091216
1440 pass
1441else:
1442 signal.signal(signal.SIGWINCH, _terminal_size_changed)
1443
1444
1430def supports_executable():1445def supports_executable():
1431 return sys.platform != "win32"1446 return sys.platform != "win32"
14321447
14331448
=== 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 @@
248 self._term_file = term_file248 self._term_file = term_file
249 # true when there's output on the screen we may need to clear249 # true when there's output on the screen we may need to clear
250 self._have_output = False250 self._have_output = False
251 # XXX: We could listen for SIGWINCH and update the terminal width...
252 # https://launchpad.net/bugs/316357
253 self._width = osutils.terminal_width()
254 self._last_transport_msg = ''251 self._last_transport_msg = ''
255 self._spin_pos = 0252 self._spin_pos = 0
256 # time we last repainted the screen253 # time we last repainted the screen
@@ -267,9 +264,11 @@
267264
268 def _show_line(self, s):265 def _show_line(self, s):
269 # sys.stderr.write("progress %r\n" % s)266 # sys.stderr.write("progress %r\n" % s)
270 if self._width is not None:267 width = osutils.terminal_width()
271 n = self._width - 1268 if width is not None:
272 s = '%-*.*s' % (n, n, s)269 # we need one extra space for terminals that wrap on last char
270 width = width - 1
271 s = '%-*.*s' % (width, width, s)
273 self._term_file.write('\r' + s + '\r')272 self._term_file.write('\r' + s + '\r')
274273
275 def clear(self):274 def clear(self):