Code review comment for lp:~gz/bzr/2.5_text_progress_view_unicode_966934

Revision history for this message
Vincent Ladeuil (vila) wrote :

8 -from StringIO import StringIO
9 +from cStringIO import StringIO

Good, that was a bug in itself.

71 +class _TTYStringIO(StringIO):
78 +class _NonTTYStringIO(StringIO):

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'

review: Needs Fixing

« Back to merge proposal