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

Proposed by Martin Pool on 2010-07-09
Status: Rejected
Rejected by: Martin Pool on 2010-07-17
Proposed branch: lp:~mbp/bzr/progress
Merge into: lp:bzr
Diff against target: 559 lines (+184/-100)
10 files modified
NEWS (+22/-0)
bzrlib/branch.py (+4/-0)
bzrlib/errors.py (+16/-8)
bzrlib/help_topics/en/patterns.txt (+14/-4)
bzrlib/progress.py (+0/-22)
bzrlib/tests/blackbox/test_commit.py (+16/-0)
bzrlib/tests/test_errors.py (+34/-1)
bzrlib/tests/test_progress.py (+37/-4)
bzrlib/tests/test_ui.py (+4/-48)
bzrlib/ui/text.py (+37/-13)
To merge this branch: bzr merge lp:~mbp/bzr/progress
Reviewer Review Type Date Requested Status
John A Meinel 2010-07-09 Needs Fixing on 2010-07-09
bzr-core 2010-07-17 Pending
Review via email: mp+29605@code.launchpad.net

Commit message

Truncate progress messages rather than counters

Description of the change

* Truncate the description rather than the counter, so that you don't get misleading numbers.

* Put the spinner (or bar) between the transport activity and the current progress task, so we don't have two '|' on the line, one of them apparently stuck.

To post a comment you must log in.
John A Meinel (jameinel) wrote :

sent to pqm by email

John A Meinel (jameinel) wrote :
Download full text (3.4 KiB)

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/progress into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> #388266 many uses of DummyProgress are unnecessary
> https://bugs.launchpad.net/bugs/388266
>
>
> * Truncate the description rather than the counter, so that you don't get misleading numbers.
>
> * Put the spinner (or bar) between the transport activity and the current progress task, so we don't have two '|' on the line, one of them apparently stuck.
>

Failing in PQM with:

> ======================================================================
> FAIL: bzrlib.tests.test_ui.TestTextUIFactory.test_progress_note_clears
> ----------------------------------------------------------------------
> _StringException: Text attachment: log
> ------------
> 4446.457 Deprecated method called
> Called from:
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/test_ui.py", line 138, in test_progress_n
> ote_clears
> pb.note, 't')
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1389, in applyDeprecat
> ed
> call_warnings, result = self._capture_deprecation_warnings(a_callable,
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/__init__.py", line 1357, in _capture_depr
> ecation_warnings
> result = a_callable(*args, **kwargs)
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/symbol_versioning.py", line 132, in decorated_m
> ethod
> trace.mutter_callsite(4, "Deprecated method called")
> ------------
> Text attachment: traceback
> ------------
> Traceback (most recent call last):
> File "/usr/lib/python2.4/site-packages/testtools/runtest.py", line 128, in _run_user
> return fn(*args)
> File "/usr/lib/python2.4/site-packages/testtools/testcase.py", line 368, in _run_test_method
> testMethod()
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/test_ui.py", line 144, in test_progress_n
> ote_clears
> self.assertContainsRe(stderr.getvalue(), r'\r {10,}\r$')
> AssertionError: pattern "\r {10,}\r$" not found in
> """\
> """ 1/1
>
> ------------
>
>
> ======================================================================
> FAIL: bzrlib.tests.test_ui.TestTextUIFactory.test_text_factory_prompts_and_clears
> ----------------------------------------------------------------------
> _StringException: Text attachment: log
> ------------
>
> ------------
> Text attachment: traceback
> ------------
> Traceback (most recent call last):
> File "/usr/lib/python2.4/site-packages/testtools/runtest.py", line 128, in _run_user
> return fn(*args)
> File "/usr/lib/python2.4/site-packages/testtools/testcase.py", line 368, in _run_test_method
> testMethod()
> File "/home/pqm/bzr-pqm-workdir/home/+trunk/bzrlib/tests/test_ui.py", line 208, in test_text_facto
> ry_prompts_and_clears
> "foo *\r\r *\r*")
> *" not found in pattern "foo *
> """\
> what do you want? [y/n]: what do you want? [y/n]: """
>
> ------------
>
>
> ----------------------------------------------------------------------
> Ran 45 tests in 6.348s
>
> FAILED (failures=2)

John
=:->

-----BEGIN P...

Read more...

John A Meinel (jameinel) wrote :

Failing in PQM as sent.

review: Needs Fixing
Martin Pool (mbp) wrote :

thanks for submitting it, I'll look into those failures

--
Martin

John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> thanks for submitting it, I'll look into those failures
>

Yeah, I was trying to get it into 2.2b4, since it is sort of an api
change (at least a visual one to the user). It is probably ok to land
between 2.2b4 and 2.2rc1, but I had hoped to not have to think about
that. :)

Speaking of which MergeIntoMerger... should that be pushed back to 2.3?

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkw5ifAACgkQJdeBCYSNAANHgwCgkEu4oRPpIMpf/sW4yjP+8lPx
e6oAnjwZjH63WvldPJKvpVU646KJC7LP
=CMbD
-----END PGP SIGNATURE-----

Martin Pool (mbp) wrote :

On 11 July 2010 11:10, John A Meinel <email address hidden> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Martin Pool wrote:
>> thanks for submitting it, I'll look into those failures
>>
>
> Yeah, I was trying to get it into 2.2b4, since it is sort of an api
> change (at least a visual one to the user). It is probably ok to land
> between 2.2b4 and 2.2rc1, but I had hoped to not have to think about
> that. :)

I think fairly small cosmetic ui changes are ok, certainly before the
2.2.0 release.

> Speaking of which MergeIntoMerger... should that be pushed back to 2.3?

From my memory of the type of changes, it seems like it may break some
plugins. Perhaps if we test the likely suspects and find that it
doesn't...

--
Martin

Unmerged revisions

5809. By Martin Pool on 2011-04-21

Just testing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-07-13 21:25:03 +0000
3+++ NEWS 2010-07-17 16:48:48 +0000
4@@ -45,6 +45,12 @@
5 Compatibility Breaks
6 ********************
7
8+* BzrError subclasses no longer support the name "message" to be used
9+ as an argument for __init__ or in _fmt format specification as this
10+ breaks in some Python versions. errors.LockError.__init__ argument
11+ is now named "msg" instead of earlier "message".
12+ (Parth Malwankar, #603461)
13+
14 New Features
15 ************
16
17@@ -58,21 +64,37 @@
18 * Don't traceback trying to unversion children files of an already
19 unversioned directory. (Vincent Ladeuil, #494221)
20
21+* Progress bars prefer to truncate the text message rather than the
22+ counters. The spinner is shown between the network transfer indicator
23+ and the progress message. (Martin Pool)
24+
25+* Recursive binding for checkouts is now detected by bzr. A clear error
26+ message is shown to the user. (Parth Malwankar, #405192)
27+
28 Improvements
29 ************
30
31 Documentation
32 *************
33
34+* ``bzr help patterns`` now explains case insensitive patterns and
35+ points to Python regular expression documentation.
36+ (Parth Malwankar, #594386)
37+
38 API Changes
39 ***********
40
41+* Delete ``ProgressTask.note``, which was deprecated in 2.1.
42+
43 Internals
44 *********
45
46 Testing
47 *******
48
49+* Unit test added to ensure that "message" is not uses as a format variable
50+ name in BzrError subclasses as this conflicts with some Python versions.
51+ (Parth Malwankar, #603461)
52
53 bzr 2.2b4
54 #########
55
56=== modified file 'bzrlib/branch.py'
57--- bzrlib/branch.py 2010-06-30 08:34:11 +0000
58+++ bzrlib/branch.py 2010-07-17 16:48:48 +0000
59@@ -246,9 +246,13 @@
60 if not local and not config.has_explicit_nickname():
61 try:
62 master = self.get_master_branch(possible_transports)
63+ if master and self.user_url == master.user_url:
64+ raise errors.RecursiveBind(self.user_url)
65 if master is not None:
66 # return the master branch value
67 return master.nick
68+ except errors.RecursiveBind, e:
69+ raise e
70 except errors.BzrError, e:
71 # Silently fall back to local implicit nick if the master is
72 # unavailable
73
74=== modified file 'bzrlib/errors.py'
75--- bzrlib/errors.py 2010-07-09 16:16:11 +0000
76+++ bzrlib/errors.py 2010-07-17 16:48:48 +0000
77@@ -947,11 +947,8 @@
78 # original exception is available as e.original_error
79 #
80 # New code should prefer to raise specific subclasses
81- def __init__(self, message):
82- # Python 2.5 uses a slot for StandardError.message,
83- # so use a different variable name. We now work around this in
84- # BzrError.__str__, but this member name is kept for compatability.
85- self.msg = message
86+ def __init__(self, msg):
87+ self.msg = msg
88
89
90 class LockActive(LockError):
91@@ -1378,12 +1375,12 @@
92
93 class WeaveParentMismatch(WeaveError):
94
95- _fmt = "Parents are mismatched between two revisions. %(message)s"
96+ _fmt = "Parents are mismatched between two revisions. %(msg)s"
97
98
99 class WeaveInvalidChecksum(WeaveError):
100
101- _fmt = "Text did not match it's checksum: %(message)s"
102+ _fmt = "Text did not match it's checksum: %(msg)s"
103
104
105 class WeaveTextDiffers(WeaveError):
106@@ -1437,7 +1434,7 @@
107
108 class VersionedFileInvalidChecksum(VersionedFileError):
109
110- _fmt = "Text did not match its checksum: %(message)s"
111+ _fmt = "Text did not match its checksum: %(msg)s"
112
113
114 class KnitError(InternalBzrError):
115@@ -3149,12 +3146,14 @@
116 def __init__(self, bzrdir):
117 self.bzrdir = bzrdir
118
119+
120 class NoWhoami(BzrError):
121
122 _fmt = ('Unable to determine your name.\n'
123 "Please, set your name with the 'whoami' command.\n"
124 'E.g. bzr whoami "Your Name <name@example.com>"')
125
126+
127 class InvalidPattern(BzrError):
128
129 _fmt = ('Invalid pattern(s) found. %(msg)s')
130@@ -3162,3 +3161,12 @@
131 def __init__(self, msg):
132 self.msg = msg
133
134+
135+class RecursiveBind(BzrError):
136+
137+ _fmt = ('Branch "%(branch_url)s" appears to be bound to itself. '
138+ 'Please use `bzr unbind` to fix.')
139+
140+ def __init__(self, branch_url):
141+ self.branch_url = branch_url
142+
143
144=== modified file 'bzrlib/help_topics/en/patterns.txt'
145--- bzrlib/help_topics/en/patterns.txt 2010-01-02 07:36:36 +0000
146+++ bzrlib/help_topics/en/patterns.txt 2010-07-17 16:48:48 +0000
147@@ -10,7 +10,7 @@
148 slash or is a regular expression, it is compared to the whole path
149 from the branch root. Otherwise, it is compared to only the last
150 component of the path. To match a file only in the root directory,
151-prepend './'. Patterns specifying absolute paths are not allowed.
152+prepend ``./``. Patterns specifying absolute paths are not allowed.
153
154 Patterns may include globbing wildcards such as::
155
156@@ -19,10 +19,20 @@
157 /**/ - Matches 0 or more directories in a path
158 [a-z] - Matches a single character from within a group of characters
159
160-Patterns may also be Python regular expressions. Regular expression
161-patterns are identified by a 'RE:' prefix followed by the regular
162+Patterns may also be `Python regular expressions`_. Regular expression
163+patterns are identified by a ``RE:`` prefix followed by the regular
164 expression. Regular expression patterns may not include named or
165 numbered groups.
166
167-Ignore patterns may be prefixed with '!', which means that a filename
168+Case insensitive ignore patterns can be specified with regular expressions
169+by using the ``i`` (for ignore case) flag in the pattern.
170+
171+For example, a case insensitive match for ``foo`` may be specified as::
172+
173+ RE:(?i)foo
174+
175+Ignore patterns may be prefixed with ``!``, which means that a filename
176 matched by that pattern will not be ignored.
177+
178+.. _Python regular expressions: http://docs.python.org/library/re.html
179+
180
181=== modified file 'bzrlib/progress.py'
182--- bzrlib/progress.py 2010-04-30 11:03:59 +0000
183+++ bzrlib/progress.py 2010-07-17 16:48:48 +0000
184@@ -152,19 +152,6 @@
185 own_fraction = 0.0
186 return self._parent_task._overall_completion_fraction(own_fraction)
187
188- @deprecated_method(deprecated_in((2, 1, 0)))
189- def note(self, fmt_string, *args):
190- """Record a note without disrupting the progress bar.
191-
192- Deprecated: use ui_factory.note() instead or bzrlib.trace. Note that
193- ui_factory.note takes just one string as the argument, not a format
194- string and arguments.
195- """
196- if args:
197- self.ui_factory.note(fmt_string % args)
198- else:
199- self.ui_factory.note(fmt_string)
200-
201 def clear(self):
202 # TODO: deprecate this method; the model object shouldn't be concerned
203 # with whether it's shown or not. Most callers use this because they
204@@ -220,12 +207,6 @@
205 self.clear()
206 self._stack.return_pb(self)
207
208- def note(self, fmt_string, *args, **kwargs):
209- """Record a note without disrupting the progress bar."""
210- self.clear()
211- self.to_messages_file.write(fmt_string % args)
212- self.to_messages_file.write('\n')
213-
214
215 class DummyProgress(object):
216 """Progress-bar standin that does nothing.
217@@ -248,9 +229,6 @@
218 def clear(self):
219 pass
220
221- def note(self, fmt_string, *args, **kwargs):
222- """See _BaseProgressBar.note()."""
223-
224 def child_progress(self, **kwargs):
225 return DummyProgress(**kwargs)
226
227
228=== modified file 'bzrlib/tests/blackbox/test_commit.py'
229--- bzrlib/tests/blackbox/test_commit.py 2010-06-28 02:41:22 +0000
230+++ bzrlib/tests/blackbox/test_commit.py 2010-07-17 16:48:48 +0000
231@@ -22,6 +22,7 @@
232 import sys
233
234 from bzrlib import (
235+ bzrdir,
236 osutils,
237 ignores,
238 msgeditor,
239@@ -757,3 +758,18 @@
240 osutils.set_or_unset_env('BZR_EMAIL', None)
241 out, err = self.run_bzr(['commit', '-m', 'initial'], 3)
242 self.assertContainsRe(err, 'Unable to determine your name')
243+
244+ def test_commit_recursive_checkout(self):
245+ """Ensure that a commit to a recursive checkout fails cleanly.
246+ """
247+ self.run_bzr(['init', 'test_branch'])
248+ self.run_bzr(['checkout', 'test_branch', 'test_checkout'])
249+ os.chdir('test_checkout')
250+ self.run_bzr(['bind', '.']) # bind to self
251+ open('foo.txt', 'w').write('hello')
252+ self.run_bzr(['add'])
253+ out, err = self.run_bzr(['commit', '-m', 'addedfoo'], 3)
254+ self.assertEqual(out, '')
255+ self.assertContainsRe(err,
256+ 'Branch.*test_checkout.*appears to be bound to itself')
257+
258
259=== modified file 'bzrlib/tests/test_errors.py'
260--- bzrlib/tests/test_errors.py 2010-07-09 16:16:11 +0000
261+++ bzrlib/tests/test_errors.py 2010-07-17 16:48:48 +0000
262@@ -16,6 +16,8 @@
263
264 """Tests for the formatting and construction of errors."""
265
266+import inspect
267+import re
268 import socket
269 import sys
270
271@@ -26,11 +28,36 @@
272 symbol_versioning,
273 urlutils,
274 )
275-from bzrlib.tests import TestCase, TestCaseWithTransport
276+from bzrlib.tests import TestCase, TestCaseWithTransport, TestSkipped
277
278
279 class TestErrors(TestCaseWithTransport):
280
281+ def test_no_arg_named_message(self):
282+ """Ensure the __init__ and _fmt in errors do not have "message" arg.
283+
284+ This test fails if __init__ or _fmt in errors has an argument
285+ named "message" as this can cause errors in some Python versions.
286+ Python 2.5 uses a slot for StandardError.message.
287+ See bug #603461
288+ """
289+ fmt_pattern = re.compile("%\(message\)[sir]")
290+ subclasses_present = getattr(errors.BzrError, '__subclasses__', None)
291+ if not subclasses_present:
292+ raise TestSkipped('__subclasses__ attribute required for classes. '
293+ 'Requires Python 2.5 or later.')
294+ for c in errors.BzrError.__subclasses__():
295+ init = getattr(c, '__init__', None)
296+ fmt = getattr(c, '_fmt', None)
297+ if init:
298+ args = inspect.getargspec(init)[0]
299+ self.assertFalse('message' in args,
300+ ('Argument name "message" not allowed for '
301+ '"errors.%s.__init__"' % c.__name__))
302+ if fmt and fmt_pattern.search(fmt):
303+ self.assertFalse(True, ('"message" not allowed in '
304+ '"errors.%s._fmt"' % c.__name__))
305+
306 def test_bad_filename_encoding(self):
307 error = errors.BadFilenameEncoding('bad/filen\xe5me', 'UTF-8')
308 self.assertEqualDiff(
309@@ -660,6 +687,12 @@
310 self.assertEqualDiff("Invalid pattern(s) found. Bad pattern msg.",
311 str(error))
312
313+ def test_recursive_bind(self):
314+ error = errors.RecursiveBind('foo_bar_branch')
315+ msg = ('Branch "foo_bar_branch" appears to be bound to itself. '
316+ 'Please use `bzr unbind` to fix.')
317+ self.assertEqualDiff(msg, str(error))
318+
319
320 class PassThroughError(errors.BzrError):
321
322
323=== modified file 'bzrlib/tests/test_progress.py'
324--- bzrlib/tests/test_progress.py 2010-02-17 17:11:16 +0000
325+++ bzrlib/tests/test_progress.py 2010-07-17 16:48:48 +0000
326@@ -58,7 +58,7 @@
327 def make_view(self):
328 out = StringIO()
329 view = TextProgressView(out)
330- view._width = 80
331+ view._avail_width = lambda: 79
332 return out, view
333
334 def make_task(self, parent_task, view, msg, curr, total):
335@@ -100,13 +100,13 @@
336 # so we're in the first half of the main task, and half way through
337 # that
338 self.assertEqual(
339-r'[####- ] reticulating splines:stage2 1/2'
340+'[####- ] reticulating splines:stage2 1/2 '
341 , view._render_line())
342 # if the nested task is complete, then we're all the way through the
343 # first half of the overall work
344 task2.update('stage2', 2, 2)
345 self.assertEqual(
346-r'[#########\ ] reticulating splines:stage2 2/2'
347+'[#########\ ] reticulating splines:stage2 2/2 '
348 , view._render_line())
349
350 def test_render_progress_sub_nested(self):
351@@ -123,5 +123,38 @@
352 # progress indication, just a label; and the bottom one is half done,
353 # so the overall fraction is 1/4
354 self.assertEqual(
355- r'[####| ] a:b:c 1/2'
356+'[####| ] a:b:c 1/2 '
357 , view._render_line())
358+
359+ def test_render_truncated(self):
360+ # when the bar is too long for the terminal, we prefer not to truncate
361+ # the counters because they might be interesting, and because
362+ # truncating the numbers might be misleading
363+ out, view = self.make_view()
364+ task_a = ProgressTask(None, progress_view=view)
365+ task_a.update('start_' + 'a' * 200 + '_end', 2000, 5000)
366+ line = view._render_line()
367+ self.assertEqual(
368+'- start_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.. 2000/5000',
369+ line)
370+ self.assertEqual(len(line), 79)
371+
372+
373+ def test_render_with_activity(self):
374+ # if the progress view has activity, it's shown before the spinner
375+ out, view = self.make_view()
376+ task_a = ProgressTask(None, progress_view=view)
377+ view._last_transport_msg = ' 123kB 100kB/s '
378+ line = view._render_line()
379+ self.assertEqual(
380+' 123kB 100kB/s / ',
381+ line)
382+ self.assertEqual(len(line), 79)
383+
384+ task_a.update('start_' + 'a' * 200 + '_end', 2000, 5000)
385+ view._last_transport_msg = ' 123kB 100kB/s '
386+ line = view._render_line()
387+ self.assertEqual(
388+' 123kB 100kB/s \\ start_aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.. 2000/5000',
389+ line)
390+ self.assertEqual(len(line), 79)
391
392=== modified file 'bzrlib/tests/test_ui.py'
393--- bzrlib/tests/test_ui.py 2010-06-21 22:29:38 +0000
394+++ bzrlib/tests/test_ui.py 2010-07-17 16:48:48 +0000
395@@ -100,51 +100,6 @@
396 finally:
397 pb.finished()
398
399- def test_progress_note(self):
400- stderr = tests.StringIOWrapper()
401- stdout = tests.StringIOWrapper()
402- ui_factory = _mod_ui_text.TextUIFactory(stdin=tests.StringIOWrapper(''),
403- stderr=stderr,
404- stdout=stdout)
405- pb = ui_factory.nested_progress_bar()
406- try:
407- result = self.applyDeprecated(deprecated_in((2, 1, 0)),
408- pb.note,
409- 't')
410- self.assertEqual(None, result)
411- self.assertEqual("t\n", stdout.getvalue())
412- # Since there was no update() call, there should be no clear() call
413- self.failIf(re.search(r'^\r {10,}\r$',
414- stderr.getvalue()) is not None,
415- 'We cleared the stderr without anything to put there')
416- finally:
417- pb.finished()
418-
419- def test_progress_note_clears(self):
420- stderr = test_progress._TTYStringIO()
421- stdout = test_progress._TTYStringIO()
422- # so that we get a TextProgressBar
423- os.environ['TERM'] = 'xterm'
424- ui_factory = _mod_ui_text.TextUIFactory(
425- stdin=tests.StringIOWrapper(''),
426- stdout=stdout, stderr=stderr)
427- self.assertIsInstance(ui_factory._progress_view,
428- _mod_ui_text.TextProgressView)
429- pb = ui_factory.nested_progress_bar()
430- try:
431- # Create a progress update that isn't throttled
432- pb.update('x', 1, 1)
433- result = self.applyDeprecated(deprecated_in((2, 1, 0)),
434- pb.note, 't')
435- self.assertEqual(None, result)
436- self.assertEqual("t\n", stdout.getvalue())
437- # the exact contents will depend on the terminal width and we don't
438- # care about that right now - but you're probably running it on at
439- # least a 10-character wide terminal :)
440- self.assertContainsRe(stderr.getvalue(), r'\r {10,}\r$')
441- finally:
442- pb.finished()
443-
444 def test_text_ui_get_boolean(self):
445 stdin = tests.StringIOWrapper("y\n" # True
446 "n\n" # False
447@@ -193,6 +148,7 @@
448 factory = _mod_ui_text.TextUIFactory(
449 stdin=tests.StringIOWrapper("yada\ny\n"),
450 stdout=out, stderr=out)
451+ factory._avail_width = lambda: 79
452 pb = factory.nested_progress_bar()
453 pb.show_bar = False
454 pb.show_spinner = False
455@@ -204,9 +160,9 @@
456 factory.get_boolean,
457 "what do you want"))
458 output = out.getvalue()
459- self.assertContainsRe(factory.stdout.getvalue(),
460- "foo *\r\r *\r*")
461- self.assertContainsRe(factory.stdout.getvalue(),
462+ self.assertContainsRe(output,
463+ "| foo *\r\r *\r*")
464+ self.assertContainsRe(output,
465 r"what do you want\? \[y/n\]: what do you want\? \[y/n\]: ")
466 # stdin should have been totally consumed
467 self.assertEqual('', factory.stdin.readline())
468
469=== modified file 'bzrlib/ui/text.py'
470--- bzrlib/ui/text.py 2010-06-21 01:30:45 +0000
471+++ bzrlib/ui/text.py 2010-07-17 16:48:48 +0000
472@@ -300,13 +300,15 @@
473 # correspond reliably to overall command progress
474 self.enable_bar = False
475
476+ def _avail_width(self):
477+ # we need one extra space for terminals that wrap on last char
478+ w = osutils.terminal_width()
479+ if w is None:
480+ return w
481+ else:
482+ return w - 1
483+
484 def _show_line(self, s):
485- # sys.stderr.write("progress %r\n" % s)
486- width = osutils.terminal_width()
487- if width is not None:
488- # we need one extra space for terminals that wrap on last char
489- width = width - 1
490- s = '%-*.*s' % (width, width, s)
491 self._term_file.write('\r' + s + '\r')
492
493 def clear(self):
494@@ -348,6 +350,10 @@
495 return ''
496
497 def _format_task(self, task):
498+ """Format task-specific parts of progress bar.
499+
500+ :returns: (text_part, counter_part) both unicode strings.
501+ """
502 if not task.show_count:
503 s = ''
504 elif task.current_cnt is not None and task.total_cnt is not None:
505@@ -363,21 +369,39 @@
506 t = t._parent_task
507 if t.msg:
508 m = t.msg + ':' + m
509- return m + s
510+ return m, s
511
512 def _render_line(self):
513 bar_string = self._render_bar()
514 if self._last_task:
515- task_msg = self._format_task(self._last_task)
516+ task_part, counter_part = self._format_task(self._last_task)
517 else:
518- task_msg = ''
519+ task_part = counter_part = ''
520 if self._last_task and not self._last_task.show_transport_activity:
521 trans = ''
522 else:
523 trans = self._last_transport_msg
524- if trans:
525- trans += ' | '
526- return (bar_string + trans + task_msg)
527+ # the bar separates the transport activity from the message, so even
528+ # if there's no bar or spinner, we must show something if both those
529+ # fields are present
530+ if (task_part or trans) and not bar_string:
531+ bar_string = '| '
532+ # preferentially truncate the task message if we don't have enough
533+ # space
534+ avail_width = self._avail_width()
535+ if avail_width is not None:
536+ # if terminal avail_width is unknown, don't truncate
537+ current_len = len(bar_string) + len(trans) + len(task_part) + len(counter_part)
538+ gap = current_len - avail_width
539+ if gap > 0:
540+ task_part = task_part[:-gap-2] + '..'
541+ s = trans + bar_string + task_part + counter_part
542+ if avail_width is not None:
543+ if len(s) < avail_width:
544+ s = s.ljust(avail_width)
545+ elif len(s) > avail_width:
546+ s = s[:avail_width]
547+ return s
548
549 def _repaint(self):
550 s = self._render_line()
551@@ -439,7 +463,7 @@
552 rate = (self._bytes_since_update
553 / (now - self._transport_update_time))
554 # using base-10 units (see HACKING.txt).
555- msg = ("%6dkB %5dkB/s" %
556+ msg = ("%6dkB %5dkB/s " %
557 (self._total_byte_count / 1000, int(rate) / 1000,))
558 self._transport_update_time = now
559 self._last_repaint = now