Merge lp:~mbp/bzr/progress into lp:bzr

Proposed by Martin Pool
Status: Merged
Approved by: Martin Pool
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~mbp/bzr/progress
Merge into: lp:bzr
Diff against target: 81 lines (+24/-1)
3 files modified
NEWS (+6/-0)
bzrlib/tests/test_progress.py (+13/-0)
bzrlib/ui/text.py (+5/-1)
To merge this branch: bzr merge lp:~mbp/bzr/progress
Reviewer Review Type Date Requested Status
Andrew Bennetts Approve
Review via email: mp+15997@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Martin Pool (mbp) wrote :

This changes the progress bar to be just a spinner plus transport info and text. I think it looks better, and it allays complaints that the bar does not correspond to overall progress.

Revision history for this message
Andrew Bennetts (spiv) wrote :

Interesting that there was no test for the default progress showing a bar. Anyway, this patch seems fine. I'm sure we'll get plenty of feedback from people once its in trunk, hopefully positive! :)

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

There are tests for the default, that's why I need to poke in view.show_bar=True to make them keep passing.

lp:~mbp/bzr/progress updated
4890. By Canonical.com Patch Queue Manager <email address hidden>

(mbp) turn off progress bar; just show a spinner

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2009-12-10 21:49:20 +0000
3+++ NEWS 2009-12-11 03:54:15 +0000
4@@ -112,6 +112,12 @@
5 'file://localhost/' as well as 'file:///' URLs on POSIX. (Michael
6 Hudson)
7
8+* The progress bar now shows only a spinner and per-operation counts,
9+ not an overall progress bar. The previous bar was often not correlated
10+ with real overall operation progress, either because the operations take
11+ nonlinear time, or because at the start of the operation Bazaar couldn't
12+ estimate how much work there was to do. (Martin Pool)
13+
14 Documentation
15 *************
16
17
18=== modified file 'bzrlib/tests/test_progress.py'
19--- bzrlib/tests/test_progress.py 2009-06-19 07:28:41 +0000
20+++ bzrlib/tests/test_progress.py 2009-12-11 03:54:15 +0000
21@@ -70,9 +70,20 @@
22 task.total_cnt = total
23 return task
24
25+ def test_render_progress_no_bar(self):
26+ """The default view now has a spinner but no bar."""
27+ out, view = self.make_view()
28+ # view.enable_bar = False
29+ task = self.make_task(None, view, 'reticulating splines', 5, 20)
30+ view.show_progress(task)
31+ self.assertEqual(
32+'\r/ reticulating splines 5/20 \r'
33+ , out.getvalue())
34+
35 def test_render_progress_easy(self):
36 """Just one task and one quarter done"""
37 out, view = self.make_view()
38+ view.enable_bar = True
39 task = self.make_task(None, view, 'reticulating splines', 5, 20)
40 view.show_progress(task)
41 self.assertEqual(
42@@ -85,6 +96,7 @@
43 task = self.make_task(None, view, 'reticulating splines', 0, 2)
44 task2 = self.make_task(task, view, 'stage2', 1, 2)
45 view.show_progress(task2)
46+ view.enable_bar = True
47 # so we're in the first half of the main task, and half way through
48 # that
49 self.assertEqual(
50@@ -100,6 +112,7 @@
51 def test_render_progress_sub_nested(self):
52 """Intermediate tasks don't mess up calculation."""
53 out, view = self.make_view()
54+ view.enable_bar = True
55 task_a = ProgressTask(None, progress_view=view)
56 task_a.update('a', 0, 2)
57 task_b = ProgressTask(task_a, progress_view=view)
58
59=== modified file 'bzrlib/ui/text.py'
60--- bzrlib/ui/text.py 2009-12-10 16:54:28 +0000
61+++ bzrlib/ui/text.py 2009-12-11 03:54:15 +0000
62@@ -261,6 +261,9 @@
63 self._total_byte_count = 0
64 self._bytes_since_update = 0
65 self._fraction = 0
66+ # force the progress bar to be off, as at the moment it doesn't
67+ # correspond reliably to overall command progress
68+ self.enable_bar = False
69
70 def _show_line(self, s):
71 # sys.stderr.write("progress %r\n" % s)
72@@ -276,7 +279,8 @@
73
74 def _render_bar(self):
75 # return a string for the progress bar itself
76- if (self._last_task is None) or self._last_task.show_bar:
77+ if self.enable_bar and (
78+ (self._last_task is None) or self._last_task.show_bar):
79 # If there's no task object, we show space for the bar anyhow.
80 # That's because most invocations of bzr will end showing progress
81 # at some point, though perhaps only after doing some initial IO.