Bazaar

Merge lp://staging/~mbp/bzr/339385-set-progress-bar into lp://staging/~bzr/bzr/trunk-old

  1. 339385-set-progress-bar
  2. Merge into trunk-old
Proposed by Martin Pool
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~mbp/bzr/339385-set-progress-bar
Merge into: lp://staging/~bzr/bzr/trunk-old
Diff against target: 204 lines
To merge this branch: bzr merge lp://staging/~mbp/bzr/339385-set-progress-bar
Reviewer Review Type Date Requested Status
Robert Collins (community) Needs Fixing
Review via email: mp+7534@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

This fixes this annoying bug that support for turning off progress bars regressed.

It also removes an over-specific error message; I doubt users really want bzr to fail if it won't support their progress bar preference, and it's even less likely code will want to specifically catch that case.

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

This looks fine.
However dropping the exception is an API break; please update the
minimum api version in __init__.py accordingly.

 review needsfixing

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-06-17 02:02:44 +0000
3+++ NEWS 2009-06-17 08:35:26 +0000
4@@ -23,6 +23,10 @@
5 * ``Branch.set_append_revisions_only`` now works with branches on a smart
6 server. (Andrew Bennetts, #365865)
7
8+* Progress bars are now suppressed again when the environment variable
9+ ``BZR_PROGRESS_BAR`` is set to ``none``.
10+ (Martin Pool, #339385)
11+
12 Internals
13 *********
14
15@@ -53,6 +57,16 @@
16 * Minor clarifications to the help for End-Of-Line conversions.
17 (Ian Clatworthy)
18
19+API Changes
20+***********
21+
22+* Removed overspecific error class ``InvalidProgressBarType``.
23+ (Martin Pool)
24+
25+* The method ``ProgressView._show_transport_activity`` is now
26+ ``show_transport_activity`` because it's part of the contract between
27+ this class and the UI. (Martin Pool)
28+
29
30 bzr 1.16rc1 "It's yesterday in California" 2009-06-11
31 #####################################################
32
33=== modified file 'bzrlib/__init__.py'
34--- bzrlib/__init__.py 2009-06-12 08:38:52 +0000
35+++ bzrlib/__init__.py 2009-06-17 08:35:26 +0000
36@@ -53,7 +53,7 @@
37 version_info = (1, 17, 0, 'dev', 0)
38
39 # API compatibility version: bzrlib is currently API compatible with 1.15.
40-api_minimum_version = (1, 15, 0)
41+api_minimum_version = (1, 17, 0)
42
43 def _format_version_tuple(version_info):
44 """Turn a version number 2, 3 or 5-tuple into a short string.
45
46=== modified file 'bzrlib/errors.py'
47--- bzrlib/errors.py 2009-06-16 12:22:20 +0000
48+++ bzrlib/errors.py 2009-06-17 08:35:26 +0000
49@@ -2172,15 +2172,6 @@
50 _fmt = "Cannot perform local-only commits on unbound branches."
51
52
53-class InvalidProgressBarType(BzrError):
54-
55- _fmt = ("Environment variable BZR_PROGRESS_BAR='%(bar_type)s"
56- " is not a supported type Select one of: %(valid_types)s")
57-
58- def __init__(self, bar_type, valid_types):
59- BzrError.__init__(self, bar_type=bar_type, valid_types=valid_types)
60-
61-
62 class UnsupportedOperation(BzrError):
63
64 _fmt = ("The method %(mname)s is not supported on"
65
66=== modified file 'bzrlib/tests/blackbox/test_diff.py'
67--- bzrlib/tests/blackbox/test_diff.py 2009-03-23 14:59:43 +0000
68+++ bzrlib/tests/blackbox/test_diff.py 2009-06-17 08:35:26 +0000
69@@ -360,18 +360,11 @@
70 # subprocess.py that we had to workaround).
71 # However, if 'diff' may not be available
72 self.make_example_branch()
73- orig_progress = os.environ.get('BZR_PROGRESS_BAR')
74- try:
75- os.environ['BZR_PROGRESS_BAR'] = 'none'
76- out, err = self.run_bzr_subprocess('diff -r 1 --diff-options -ub',
77- universal_newlines=True,
78- retcode=None)
79- finally:
80- if orig_progress is None:
81- del os.environ['BZR_PROGRESS_BAR']
82- else:
83- os.environ['BZR_PROGRESS_BAR'] = orig_progress
84-
85+ # this will be automatically restored by the base bzr test class
86+ os.environ['BZR_PROGRESS_BAR'] = 'none'
87+ out, err = self.run_bzr_subprocess('diff -r 1 --diff-options -ub',
88+ universal_newlines=True,
89+ retcode=None)
90 if 'Diff is not installed on this machine' in err:
91 raise TestSkipped("No external 'diff' is available")
92 self.assertEqual('', err)
93
94=== modified file 'bzrlib/tests/test_ui.py'
95--- bzrlib/tests/test_ui.py 2009-06-10 03:56:49 +0000
96+++ bzrlib/tests/test_ui.py 2009-06-17 08:35:26 +0000
97@@ -39,6 +39,7 @@
98 SilentUIFactory,
99 )
100 from bzrlib.ui.text import (
101+ NullProgressView,
102 TextProgressView,
103 TextUIFactory,
104 )
105@@ -103,6 +104,25 @@
106 finally:
107 pb.finished()
108
109+ def test_progress_construction(self):
110+ """TextUIFactory constructs the right progress view.
111+ """
112+ os.environ['BZR_PROGRESS_BAR'] = 'none'
113+ self.assertIsInstance(TextUIFactory()._progress_view,
114+ NullProgressView)
115+
116+ os.environ['BZR_PROGRESS_BAR'] = 'text'
117+ self.assertIsInstance(TextUIFactory()._progress_view,
118+ TextProgressView)
119+
120+ os.environ['BZR_PROGRESS_BAR'] = 'text'
121+ self.assertIsInstance(TextUIFactory()._progress_view,
122+ TextProgressView)
123+
124+ del os.environ['BZR_PROGRESS_BAR']
125+ self.assertIsInstance(TextUIFactory()._progress_view,
126+ TextProgressView)
127+
128 def test_progress_note(self):
129 stderr = StringIO()
130 stdout = StringIO()
131
132=== modified file 'bzrlib/ui/text.py'
133--- bzrlib/ui/text.py 2009-04-10 19:37:20 +0000
134+++ bzrlib/ui/text.py 2009-06-17 08:35:26 +0000
135@@ -19,6 +19,7 @@
136 """Text UI, write output to the console.
137 """
138
139+import os
140 import sys
141 import time
142 import warnings
143@@ -56,8 +57,8 @@
144 symbol_versioning.warn(symbol_versioning.deprecated_in((1, 11, 0))
145 % "bar_type parameter")
146 # paints progress, network activity, etc
147- self._progress_view = TextProgressView(self.stderr)
148-
149+ self._progress_view = self._make_progress_view()
150+
151 def clear_term(self):
152 """Prepare the terminal for output.
153
154@@ -69,6 +70,12 @@
155 # to clear it. We might need to separately check for the case of
156 self._progress_view.clear()
157
158+ def _make_progress_view(self):
159+ if os.environ.get('BZR_PROGRESS_BAR') in ('text', None, ''):
160+ return TextProgressView(self.stderr)
161+ else:
162+ return NullProgressView()
163+
164 def note(self, msg):
165 """Write an already-formatted message, clearing the progress bar if necessary."""
166 self.clear_term()
167@@ -80,7 +87,7 @@
168 This may update a progress bar, spinner, or similar display.
169 By default it does nothing.
170 """
171- self._progress_view._show_transport_activity(transport,
172+ self._progress_view.show_transport_activity(transport,
173 direction, byte_count)
174
175 def _progress_updated(self, task):
176@@ -98,6 +105,19 @@
177 self._progress_view.clear()
178
179
180+class NullProgressView(object):
181+ """Soak up and ignore progress information."""
182+
183+ def clear(self):
184+ pass
185+
186+ def show_progress(self, task):
187+ pass
188+
189+ def show_transport_activity(self, transport, direction, byte_count):
190+ pass
191+
192+
193 class TextProgressView(object):
194 """Display of progress bar and other information on a tty.
195
196@@ -216,7 +236,7 @@
197 self._last_repaint = now
198 self._repaint()
199
200- def _show_transport_activity(self, transport, direction, byte_count):
201+ def show_transport_activity(self, transport, direction, byte_count):
202 """Called by transports via the ui_factory, as they do IO.
203
204 This may update a progress bar, spinner, or similar display.