Bazaar

Merge lp:~statik/bzr/bzr-less-signal into lp:bzr

  1. bzr-less-signal
  2. Merge into bzr.dev
Proposed by Elliot Murphy
Status: Merged
Approved by: Martin Pool
Approved revision: not available
Merge reported by: Martin Pool
Merged at revision: not available
Proposed branch: lp:~statik/bzr/bzr-less-signal
Merge into: lp:bzr
Diff against target: 55 lines (+20/-7)
2 files modified
bzrlib/osutils.py (+15/-6)
bzrlib/ui/text.py (+5/-1)
To merge this branch: bzr merge lp:~statik/bzr/bzr-less-signal
Reviewer Review Type Date Requested Status
Martin Pool Approve
Robert Collins (community) Approve
Review via email: mp+19316@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Elliot Murphy (statik) wrote :

This moves the wiring up of the terminal change signal into the constructor for TextUIFactory instead of running during module import of osutils.

I wasn't sure where to add tests. I'd love it if this branch could be merged before bzr 2.1 final is out, it's for a bug I'm hitting in bzr 2.1.0rc2.

Revision history for this message
Elliot Murphy (statik) wrote :

On the bug, robert mentioned "we'd still want to only do it once, other than that I agree that there is no need to do it on import." I don't know enough about bzr to know whether TextUIFactory constructor runs only once during a run of bzr, or whether this needs some additional work to be sure the signal is only wired once.

Revision history for this message
Robert Collins (lifeless) wrote :

On Mon, 2010-02-15 at 04:14 +0000, Elliot Murphy wrote:
> On the bug, robert mentioned "we'd still want to only do it once, other than that I agree that there is no need to do it on import." I don't know enough about bzr to know whether TextUIFactory constructor runs only once during a run of bzr, or whether this needs some additional work to be sure the signal is only wired once.

Additional work will be required - doing it in the constructor isn't
really going to work well - signals can't cope if e.g. two TextUI
objects exist.

rather you need to keep this at module scope (and I'd keep it in
osutils).

Have a function the UI constructor can call, which checks a global to
see if its already done its registration.

-Rob

Revision history for this message
Robert Collins (lifeless) wrote :

 review: needsfixing

review: Needs Fixing
lp:~statik/bzr/bzr-less-signal updated
5037. By Elliot Murphy

Refactored based on feedback from Robert.

Revision history for this message
Elliot Murphy (statik) wrote :

That makes sense, I believe I've refactored it correctly now.

Revision history for this message
Robert Collins (lifeless) wrote :

Looks good to me know.

-Rob

review: Approve
Revision history for this message
Martin Pool (mbp) wrote :

as discussed in bug 512989 I'll merge this to 2.1 too.

review: Approve
Revision history for this message
Martin Pool (mbp) wrote :

Now merged to 2.1.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py 2010-02-11 16:32:32 +0000
+++ bzrlib/osutils.py 2010-02-15 04:34:13 +0000
@@ -1440,12 +1440,21 @@
1440 if width is not None:1440 if width is not None:
1441 os.environ['COLUMNS'] = str(width)1441 os.environ['COLUMNS'] = str(width)
14421442
1443if sys.platform == 'win32':1443
1444 # Martin (gz) mentioned WINDOW_BUFFER_SIZE_RECORD from ReadConsoleInput but1444_registered_sigwinch = False
1445 # I've no idea how to plug that in the current design -- vila 200912161445
1446 pass1446def watch_sigwinch():
1447else:1447 """Register for SIGWINCH, once and only once."""
1448 signal.signal(signal.SIGWINCH, _terminal_size_changed)1448 global _registered_sigwinch
1449 if not _registered_sigwinch:
1450 if sys.platform == 'win32':
1451 # Martin (gz) mentioned WINDOW_BUFFER_SIZE_RECORD from
1452 # ReadConsoleInput but I've no idea how to plug that in
1453 # the current design -- vila 20091216
1454 pass
1455 else:
1456 signal.signal(signal.SIGWINCH, _terminal_size_changed)
1457 _registered_sigwinch = True
14491458
14501459
1451def supports_executable():1460def supports_executable():
14521461
=== modified file 'bzrlib/ui/text.py'
--- bzrlib/ui/text.py 2010-02-10 17:52:08 +0000
+++ bzrlib/ui/text.py 2010-02-15 04:34:13 +0000
@@ -37,6 +37,8 @@
3737
38""")38""")
3939
40from bzrlib.osutils import watch_sigwinch
41
40from bzrlib.ui import (42from bzrlib.ui import (
41 UIFactory,43 UIFactory,
42 NullProgressView,44 NullProgressView,
@@ -60,7 +62,9 @@
60 self.stderr = stderr62 self.stderr = stderr
61 # paints progress, network activity, etc63 # paints progress, network activity, etc
62 self._progress_view = self.make_progress_view()64 self._progress_view = self.make_progress_view()
63 65 # hook up the signals to watch for terminal size changes
66 watch_sigwinch()
67
64 def be_quiet(self, state):68 def be_quiet(self, state):
65 if state and not self._quiet:69 if state and not self._quiet:
66 self.clear_term()70 self.clear_term()