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