Merge lp:~gz/bzr/2.5_text_progress_view_unicode_966934 into lp:bzr/2.5
Status: | Merged |
---|---|
Approved by: | Vincent Ladeuil |
Approved revision: | no longer in the source branch. |
Merged at revision: | 6497 |
Proposed branch: | lp:~gz/bzr/2.5_text_progress_view_unicode_966934 |
Merge into: | lp:bzr/2.5 |
Diff against target: |
241 lines (+80/-34) 5 files modified
bzrlib/progress.py (+7/-1) bzrlib/tests/test_progress.py (+40/-22) bzrlib/tests/test_ui.py (+20/-8) bzrlib/ui/text.py (+10/-3) doc/en/release-notes/bzr-2.5.txt (+3/-0) |
To merge this branch: | bzr merge lp:~gz/bzr/2.5_text_progress_view_unicode_966934 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Vincent Ladeuil | Approve | ||
Review via email: mp+99790@code.launchpad.net |
Commit message
Encode progress task messages when written to a terminal
Description of the change
Some of the progress descriptions in bzrlib.transform were wrapped in gettext() calls, but the underlying TextProgressView has no handling for non-ascii text. So, as these strings get translated, operations that display one of these updates will break trying to write unicode.
This branch changes the TextProgressView class to expect unicode, and accept encoding and errors parameters. The ui stuff in general could do with neater handling here, but this should do for now.
We do not seem to have any tests for the transform progress itself, or a mechanism for blackbox tests to assert on progress updates. For now there's a basic test checking that the progress output is indeed encoded as bytes. I have left worrying about the text width measurement being broken for later, but this may also break progress display for anyone with a fancy UTF-8 terminal.
As I wanted to change from using StringIO to cStringIO to get the failing test, I moved some classes that needed to subclass StringIO into the test module where they are used.
8 -from StringIO import StringIO
9 +from cStringIO import StringIO
Good, that was a bug in itself.
71 +class _TTYStringIO( StringIO) : (StringIO) :
78 +class _NonTTYStringIO
I don't think we want the leading '_' here. Those are helper classes (and
there is even "113 + TTYStringIO = _TTYStringIO" which seems even weirder.
142 + self._encoding = getattr(term_file, "encoding", "ascii")
Don't we want to default to utf8 instead ?
159 + # GZ 2012-03-28: Counting bytes is wrong for calculating width of
160 + # text but better than counting codepoints.
Can you just briefly add (in this comment) a description of what you think
should be done (or even better file a bug and just reference it there) ?
Just from reading the comment, I'm not sure if we should blame python for
not providing an easy way to do that or what *we* need to do there.
155 + def _show_line(self, u):
Worth a trivial docstring mentioning 'u' means: 'We expect unicode'