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
1=== modified file 'bzrlib/osutils.py'
2--- bzrlib/osutils.py 2010-02-11 16:32:32 +0000
3+++ bzrlib/osutils.py 2010-02-15 04:34:13 +0000
4@@ -1440,12 +1440,21 @@
5 if width is not None:
6 os.environ['COLUMNS'] = str(width)
7
8-if sys.platform == 'win32':
9- # Martin (gz) mentioned WINDOW_BUFFER_SIZE_RECORD from ReadConsoleInput but
10- # I've no idea how to plug that in the current design -- vila 20091216
11- pass
12-else:
13- signal.signal(signal.SIGWINCH, _terminal_size_changed)
14+
15+_registered_sigwinch = False
16+
17+def watch_sigwinch():
18+ """Register for SIGWINCH, once and only once."""
19+ global _registered_sigwinch
20+ if not _registered_sigwinch:
21+ if sys.platform == 'win32':
22+ # Martin (gz) mentioned WINDOW_BUFFER_SIZE_RECORD from
23+ # ReadConsoleInput but I've no idea how to plug that in
24+ # the current design -- vila 20091216
25+ pass
26+ else:
27+ signal.signal(signal.SIGWINCH, _terminal_size_changed)
28+ _registered_sigwinch = True
29
30
31 def supports_executable():
32
33=== modified file 'bzrlib/ui/text.py'
34--- bzrlib/ui/text.py 2010-02-10 17:52:08 +0000
35+++ bzrlib/ui/text.py 2010-02-15 04:34:13 +0000
36@@ -37,6 +37,8 @@
37
38 """)
39
40+from bzrlib.osutils import watch_sigwinch
41+
42 from bzrlib.ui import (
43 UIFactory,
44 NullProgressView,
45@@ -60,7 +62,9 @@
46 self.stderr = stderr
47 # paints progress, network activity, etc
48 self._progress_view = self.make_progress_view()
49-
50+ # hook up the signals to watch for terminal size changes
51+ watch_sigwinch()
52+
53 def be_quiet(self, state):
54 if state and not self._quiet:
55 self.clear_term()