Merge lp:~songofacandy/bzr/i18n-utextwrap into lp:bzr

[methane]

Add support for double width characters to textwrap.

I18n part 1
textwrap supports double width characters.

[Vincent Ladeuil (vila)]

Haaa, far better :) Thanks a lot !

The tests are a good start but wrap probably deserve more of them given its internal complexity and fill should probably receive some too. Finally one test demonstrating that the width is correctly handled would help (the idea is to make it easier later to debug specific areas of the code by having tests that already exercise it. That makes far easier to debug the code when you didn't write it yourself).

  ./bzr selftest -s bt.test_utext --coverage utext

will help you ensuring a minimal coverage (look into utext/bzrlib.utextwrap.cover).

I just did, so if indeed 'fill' is not tested, there is only a few spots that aren't in 'wrap' so you're really close already. (Forgive my ignorance, but is 'Good Morning' already on multiple lines in Japanese ?).

I'll put the review into 'work in progress', don't forget to set it back to 'NeedsReview' when needed.

review: Needs Fixing

[Alexander Belchenko (bialix)]

On Windows osutils.terminal_width() returns the real terminal width, but
we need (osutils.terminal_width()-1) otherwise user will see extra blank
line when line filled up to that limit.

[Vincent Ladeuil (vila)]

> On Windows osutils.terminal_width() returns the real terminal width, but
> we need (osutils.terminal_width()-1) otherwise user will see extra blank
> line when line filled up to that limit.

Right. We need tests for that or we'll keep having this discussion. In any case, the windows implementation of osutils.terminal_width should take that into account so this proposal should have to care about it.

Do we have a bug filed for that ?

[methane (songofacandy)]

Thank you for your review, Vincent.

> Haaa, far better :) Thanks a lot !
>
> The tests are a good start but wrap probably deserve more of them given its
> internal complexity and fill should probably receive some too. Finally one
> test demonstrating that the width is correctly handled would help (the idea is
> to make it easier later to debug specific areas of the code by having tests
> that already exercise it. That makes far easier to debug the code when you
> didn't write it yourself).
>
> ./bzr selftest -s bt.test_utext --coverage utext
> will help you ensuring a minimal coverage (look into
> utext/bzrlib.utextwrap.cover).
>
> I just did, so if indeed 'fill' is not tested, there is only a few spots that
> aren't in 'wrap' so you're really close already.

Okey, I've added many tests and fixed minor bug found by the test.
I make UTextWrap's private methods compatible to TextWrap and
run test_textwrap on UTextWrap to ensure no regression with
single byte texts.

> (Forgive my ignorance, but is
> 'Good Morning' already on multiple lines in Japanese ?).

No, it is 4 grams word. O-HA-YO-U and usually on one line.
But we don't use word separator like whiteespace and
can split any place. (There are some exception but handling
these exception is bit hard.)

> I'll put the review into 'work in progress', don't forget to set it back to
> 'NeedsReview' when needed.

I've got ready.

[Alexander Belchenko (bialix)]

Vincent Ladeuil пишет:
>> On Windows osutils.terminal_width() returns the real terminal width, but
>> we need (osutils.terminal_width()-1) otherwise user will see extra blank
>> line when line filled up to that limit.
>
> Right. We need tests for that or we'll keep having this discussion. In any case, the windows implementation of osutils.terminal_width should take that into account so this proposal should have to care about it.
>
> Do we have a bug filed for that ?

I don't think so but sometimes we talked about this issue with you in
IRC, cause it affects selftest output.

--
All the dude wanted was his rug back

[Vincent Ladeuil (vila)]

> I don't think so but sometimes we talked about this issue with you in
> IRC, cause it affects selftest output.

Yeah I remember the discussions, but as I have never been able to reproduce the issue myself, filing a bug with a recipe will help (hint, hint, nudge, nudge ;)

[Alexander Belchenko (bialix)]

> > I don't think so but sometimes we talked about this issue with you in
> > IRC, cause it affects selftest output.
>
> Yeah I remember the discussions, but as I have never been able to reproduce
> the issue myself, filing a bug with a recipe will help (hint, hint, nudge,
> nudge ;)

Well, if you're asking then I'm doing :-)

[Vincent Ladeuil (vila)]

> Okey, I've added many tests and fixed minor bug found by the test.

Thanks and yeah for tests finding bugs \o/

> I make UTextWrap's private methods compatible to TextWrap and
> run test_textwrap on UTextWrap to ensure no regression with
> single byte texts.

Excellent, also I just noticed a couple of issues with the way you write your tests.

The rule is that we do assert(expected, actual). While this may not be true for every test, that's what we aim for.

We also aim at making tests as focused as possible, so some tests are worth splitting.

Finally, overriding symbols as you do while reusing the python tests leave them overriden after the tests are run, not a big deal here but just in case, it's better to use overrideAttr to ensure they are restored.

I've made those changes in lp:~vila/bzr/i18n-utextwrap , you may merge that if you agree with them and put the proposal into 'NeedsReview' again for final approval.

>
> > (Forgive my ignorance, but is
> > 'Good Morning' already on multiple lines in Japanese ?).
>
> No, it is 4 grams word. O-HA-YO-U and usually on one line.
> But we don't use word separator like whiteespace

Doh ! Of course ! I never thought about that...

> and
> can split any place. (There are some exception but handling
> these exception is bit hard.)

Ok, we can deal with that later if needed.

review: Needs Fixing

[methane (songofacandy)]

I've seen your fixes and merged it. Thank you.

[Vincent Ladeuil (vila)]

mgz said he'll do a second review, I'm fine with the proposal myself.

review: Approve

[Martin Packman (gz)]

Good work tackling this, reading TR14 again reminds me how not-fun all the details are. I think your overall approach of trying extend the existing stdlib functionality just enough for bzr is sensible. We currently have en, es, ja, and ru docs so there isn't any need to deal with esoteric rules for other languages yet.

Various notes and nitpicks on the diff follow. The main thing that needs fixing is that wrapping for Russian (and Greek) will follow east asian style with this change rather than western, and width measurement will be wrong in those cases. The easiest way to fix this is debatable, I'd be tempted to try and use the language as an input to change the text wrapping behaviour, and possibly the codepage for the width measurement.

general
=======

I find the tests a little hard to follow, but they look like a realy good start. Currently only hiragana and basic latin are covered, which is why the cyrillic and greek problems aren't apparent.

+# Copyright (C) 2011 Canonical Ltd

For bzrlib.utextwrap this line possibly wants the dates and names from textwrap.py as you note later that some code comes from that module.

+ if self.drop_whitespace and chunks[-1].strip() == '' and lines:

The `drop_whitespace` param is 2.6 or later only:

<http://docs.python.org/library/textwrap.html#textwrap.TextWrapper.drop_whitespace>

This is likely a feature bzr doesn't need, but it's not hard to add a little version check, pull <lp:~gz/bzr/tweak_utextwrap> for a fix.

It looks like the `expand_tabs` param won't work correctly for ideographic text as the `_munge_whitespace` method isn't overridden and it uses `unicode.expandtabs` which does the wrong measurement for tab stops. This probably also a feature bzr doesn't need, but the limitation should be documented.

str vs. unicode
===============

Is there any reason to support non-unicode strings in this class? Is is a requirement for your neat trick of running the Python textwrap tests against this module too?

+ if isinstance(s, str):
+ return len(s)
+ assert isinstance(s, unicode)

The two blocks like this in the private `_width` and `_cut` functions would ideally not be needed, the public entrance points should ensure the string is unicode. Currently, they look like they'll just silently do the wrong thing for non-ascii byte strings.

+ def _split(self, text):
+ chunks = textwrap.TextWrapper._split(self, unicode(text))

This is also a private method so shouldn't need to convert the argument, the callers should just use the right object.

+ def wrap(self, text):
+ # ensure text is unicode
+ return textwrap.TextWrapper.wrap(self, unicode(text))

In the public interface this is a reasonable way to support ascii-only byte strings and a convenience for callers, other parts of bzrlib do a similar thing. The main issue is that it's easy for callers to get away with doing the wrong thing, but that's not much of a concern here as it should be obvious when non-english text is passed.

east_asian_width
================

The main problem comes from this logic:

+ w += (c in 'FWA' and 2) or 1
...
+ w += (_eawidth(c) in 'FWA' and 2) or 1
...
+ ...

[Martin Packman (gz)]

review: Needs Fixing

[John A Meinel (jameinel)]

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

On 5/7/2011 11:28 PM, Martin [gz] wrote:
> Review: Needs Fixing
>

I just want us to remember the rules for reviewing:

 Is it better than what we have, and doesn't reduce test coverage.

Nowhere in there is "Is it perfect".

I don't understand enough here to be able to say where things are at.
But if the current code is an improvement over our existing logic, it
can be landed into core, *and improved from there*, rather than stalling
the work at the review stage.

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

iEYEARECAAYFAk3GS7oACgkQJdeBCYSNAAP75ACeL6TyvVYtwsfo8cc6CACpPFya
t4oAoKZo3y3voqNyjWsd3PcwqZkCtVSC
=Rs3O
-----END PGP SIGNATURE-----

[methane (songofacandy)]

> +# Copyright (C) 2011 Canonical Ltd
>
> For bzrlib.utextwrap this line possibly wants the dates and names from
> textwrap.py as you note later that some code comes from that module.

I've copied Copyright lines from textwrap.

> + if self.drop_whitespace and chunks[-1].strip() == '' and lines:
>
> The `drop_whitespace` param is 2.6 or later only:
>
> <http://docs.python.org/library/textwrap.html#textwrap.TextWrapper.drop_whites
> pace>
>
> This is likely a feature bzr doesn't need, but it's not hard to add a little
> version check, pull <lp:~gz/bzr/tweak_utextwrap> for a fix.

Thank you. I've merged it.

> It looks like the `expand_tabs` param won't work correctly for ideographic
> text as the `_munge_whitespace` method isn't overridden and it uses
> `unicode.expandtabs` which does the wrong measurement for tab stops. This
> probably also a feature bzr doesn't need, but the limitation should be
> documented.

Could you tell me what is the problem of `unicode.expandtabs`?
I haven't heard about that.

> str vs. unicode
> ===============
>
> Is there any reason to support non-unicode strings in this class? Is is a
> requirement for your neat trick of running the Python textwrap tests against
> this module too?
>
> + if isinstance(s, str):
> + return len(s)
> + assert isinstance(s, unicode)
>
> The two blocks like this in the private `_width` and `_cut` functions would
> ideally not be needed, the public entrance points should ensure the string is
> unicode. Currently, they look like they'll just silently do the wrong thing
> for non-ascii byte strings.

I've removed `if isinstance(...`.

>
> + def _split(self, text):
> + chunks = textwrap.TextWrapper._split(self, unicode(text))
>
> This is also a private method so shouldn't need to convert the argument, the
> callers should just use the right object.

This private method is tested by test_textwrap and it passes byte string.
`unicode(text)` uses default encoding that is 'ascii' on most platform and
raises UnicodeDecodeError when we passes non-ascii string.

>
> + def wrap(self, text):
> + # ensure text is unicode
> + return textwrap.TextWrapper.wrap(self, unicode(text))
>
> In the public interface this is a reasonable way to support ascii-only byte
> strings and a convenience for callers, other parts of bzrlib do a similar
> thing. The main issue is that it's easy for callers to get away with doing the
> wrong thing, but that's not much of a concern here as it should be obvious
> when non-english text is passed.
>
>
> east_asian_width
> ================
>
> The main problem comes from this logic:
>
> + w += (c in 'FWA' and 2) or 1
> ...
> + w += (_eawidth(c) in 'FWA' and 2) or 1
> ...
> + if _eawidth(char) in 'FWA':
>
> Firstly, it's a bad thing that a similar-but different line appears three
> times. That's a sign a helper is needed.

I've split it.

>
> For width measurement, `in 'FWA'` isn't a bad check in an ideographic context,
> I have nearly the same logic in a script I use to work out indents in EUC-JP
> text with a `eawidth_map = {"F":2,"H":1,"W":2,"Na":1,"A":2,"N":1}` mapping.
>
> However...

[Martin Packman (gz)]

> Nowhere in there is "Is it perfect".

Yes, there's a conflict between "writing everything I think of when I read through the code" and "being clear about what changes are needed so the code can land".

Avoiding a regression in how Russian is handled versus the default TextWrapper class is the main thing, but until there's some Russian help text written that's not actually fatal.

[Martin Packman (gz)]

Thanks for following up on this!

> Could you tell me what is the problem of `unicode.expandtabs`?
> I haven't heard about that.

It gets the tabulation wrong in this context, eg:

>>> s = u"English\ten\n日本語\tja"
>>> print s
English en
日本語 ja
>>> print s.expandtabs()
English en
日本語 ja

The help text we're dealing with doesn't use tabs for tables, so just noting the `expand_tabs` param may give surprising results is fine.

> > + if isinstance(s, str):
> > + return len(s)
> > + assert isinstance(s, unicode)
...
>
> I've removed `if isinstance(...`.

One thing I forgot to mention is that we can't use `assert` either, try `bzr selftest -s bt.test_source`, in these two cases just removing that statement should be fine.

> This private method is tested by test_textwrap and it passes byte string.
> `unicode(text)` uses default encoding that is 'ascii' on most platform and
> raises UnicodeDecodeError when we passes non-ascii string.

This is fine then.

> I think we can use i18n for it. For example, i18n module may be able to
> provide a
> function like:
>
> def get_ambiguous_width():
> if _installed_language in ('cn', 'ja', 'kr'):
> return 2
> else:
> return 1
>
> For now, I've stopped using utextwrap in bzrlib.help module.

That's the kind of thing I was thinking of, I'm not sure if gettext really makes it easy to do this. Passing the actual language to UTextWrapper would also allow it do vary the wrapping behaviour in other language-specific contexts. This isn't something that needs doing in this merge proposal.

> Splitting between combined character or surrogate pair may cause a problem.
> I'll try to make more clever line breaking if the problem happens.
> But if the problem happen, users can show English help (we shoud provide
> that way) so nothing goes worse than now.

Yes, this was just some extra thoughts and not any changes that needed doing now.

[methane (songofacandy)]

Thank you for teaching me.

I've added some docstring about limitations.
But I'm not good English writer. I hope someone rewrites docs written by me.

[John C Barstow (jbowtie)]

On Sun, May 8, 2011 at 9:24 AM, Martin [gz] <email address hidden> wrote:
> We currently have en, es, ja, and ru docs so there isn't any need to deal with esoteric rules for other languages yet.

In general i18n terms, the only thing really missing to round out our
sample is a bidirectional language like Arabic (ar), which would
expose any major weaknesses left in the logic. But I'm more than happy
to see this land.

[Martin Packman (gz)]

> One thing I forgot to mention is that we can't use `assert` either, try `bzr
> selftest -s bt.test_source`, in these two cases just removing that statement
> should be fine.

The asserts need removing before PQM will let this branch land.

[methane (songofacandy)]

> > One thing I forgot to mention is that we can't use `assert` either, try `bzr
> > selftest -s bt.test_source`, in these two cases just removing that statement
> > should be fine.
>
> The asserts need removing before PQM will let this branch land.

I've removed the asserts.

[Vincent Ladeuil (vila)]

@gz: Are you ok to land this now ?

[Martin Packman (gz)]

+ assert chunk # TextWrapper._split removes empty chunk

Need to remove that one too. Run `./bzr selftest -s bt.test_source` to make sure you've got them all.

With that done this should be fine to land, need to try it with some real translations to improve further probably.

[methane (songofacandy)]

> + assert chunk # TextWrapper._split removes empty chunk
>
> Need to remove that one too. Run `./bzr selftest -s bt.test_source` to make
> sure you've got them all.
>

Sorry, I removed last assert.
And run test_source then fix minor issue reported by the test.

[Martin Packman (gz)]

Okay, nearly there. Two tests from textwrapper that use `fix_sentence_endings=True` fail because of this code in the textwrap module which puts bytestrings in the chunks list:

    if chunks[i+1] == " " and pat.search(chunks[i]):
        chunks[i+1] = " "

Resulting in errors like:

Traceback (most recent call last):
  ...
  File "...\lib\test\test_textwrap.py", line 102, in test_fix_sentence_endings
    self.check(wrapper.wrap(text), expect)
  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 216, in wrap
    return textwrap.TextWrapper.wrap(self, unicode(text))
  File "...\lib\textwrap.py", line 321, in wrap
    return self._wrap_chunks(chunks)
  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 172, in _wrap_chunks
    l = self._width(chunks[-1])
  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 95, in _width
    return sum(charwidth(c) for c in s)
  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 95, in <genexpr>
    return sum(charwidth(c) for c in s)
  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 86, in _unicode_char_width
    return (_eawidth(uc) in self._east_asian_doublewidth and 2) or 1
TypeError: must be unicode, not str

Annoying. That option is pretty bogus anyway, I'd not mind just not supporting it. Otherwise _fix_sentence_endings needs overriding so it doesn't mix up types in the chunks list.

[methane (songofacandy)]

On Sat, May 14, 2011 at 10:04 PM, Martin [gz] <email address hidden> wrote:
> Okay, nearly there. Two tests from textwrapper that use `fix_sentence_endings=True` fail because of this code in the textwrap module which puts bytestrings in the chunks list:
>
>    if chunks[i+1] == " " and pat.search(chunks[i]):
>        chunks[i+1] = "  "
>
> Resulting in errors like:
>
> Traceback (most recent call last):
>  ...
>  File "...\lib\test\test_textwrap.py", line 102, in test_fix_sentence_endings
>    self.check(wrapper.wrap(text), expect)
>  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 216, in wrap
>    return textwrap.TextWrapper.wrap(self, unicode(text))
>  File "...\lib\textwrap.py", line 321, in wrap
>    return self._wrap_chunks(chunks)
>  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 172, in _wrap_chunks
>    l = self._width(chunks[-1])
>  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 95, in _width
>    return sum(charwidth(c) for c in s)
>  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 95, in <genexpr>
>    return sum(charwidth(c) for c in s)
>  File "...\i18n-utextwrap\bzrlib\utextwrap.py", line 86, in _unicode_char_width
>    return (_eawidth(uc) in self._east_asian_doublewidth and 2) or 1
> TypeError: must be unicode, not str
>
> Annoying. That option is pretty bogus anyway, I'd not mind just not supporting it. Otherwise _fix_sentence_endings needs overriding so it doesn't mix up types in the chunks list.

I've overrided the _fix_sentence_endings.

Why I miss the error is I used another Python environment when
removing cast to unicode.
To avoid this, I've added empty test case that raises TestSkipped when
test.test_textwrap
is unavailable.

> --
> https://code.launchpad.net/~songofacandy/bzr/i18n-utextwrap/+merge/59950
> You are the owner of lp:~songofacandy/bzr/i18n-utextwrap.
>

--
INADA Naoki  <email address hidden>

[Martin Packman (gz)]

Tests pass, looks time to try it out for real.

review: Approve

[Vincent Ladeuil (vila)]

sent to pqm by email

=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	2011-05-13 12:51:05 +0000
+++ bzrlib/tests/__init__.py	2011-05-14 15:01:51 +0000
@@ -3899,6 +3899,7 @@
         'bzrlib.tests.test_upgrade',
         'bzrlib.tests.test_upgrade_stacked',
         'bzrlib.tests.test_urlutils',
+        'bzrlib.tests.test_utextwrap',
         'bzrlib.tests.test_version',
         'bzrlib.tests.test_version_info',
         'bzrlib.tests.test_versionedfile',
 
=== added file 'bzrlib/tests/test_utextwrap.py'
--- bzrlib/tests/test_utextwrap.py	1970-01-01 00:00:00 +0000
+++ bzrlib/tests/test_utextwrap.py	2011-05-14 15:01:51 +0000
@@ -0,0 +1,207 @@
+# Copyright (C) 2011 Canonical Ltd
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+#
+
+"""Tests of the bzrlib.utextwrap."""
+
+from bzrlib import tests, utextwrap
+from bzrlib.tests import TestSkipped
+
+# Japanese "Good morning".
+# Each character have double width. So total 8 width on console.
+_str_D = u'\u304a\u306f\u3088\u3046'
+
+_str_S = u"hello"
+
+# Combine single width characters and double width characters.
+_str_SD = _str_S + _str_D
+_str_DS = _str_D + _str_S
+
+class TestUTextWrap(tests.TestCase):
+
+    def check_width(self, text, expected_width):
+        w = utextwrap.UTextWrapper()
+        self.assertEqual(
+                w._width(text),
+                expected_width,
+                "Width of %r should be %d" % (text, expected_width))
+
+    def test_width(self):
+        self.check_width(_str_D, 8)
+        self.check_width(_str_SD, 13)
+
+    def check_cut(self, text, width, pos):
+        w = utextwrap.UTextWrapper()
+        self.assertEqual((text[:pos], text[pos:]), w._cut(text, width))
+
+    def test_cut(self):
+        s = _str_SD
+        self.check_cut(s, 0, 0)
+        self.check_cut(s, 1, 1)
+        self.check_cut(s, 5, 5)
+        self.check_cut(s, 6, 5)
+        self.check_cut(s, 7, 6)
+        self.check_cut(s, 12, 8)
+        self.check_cut(s, 13, 9)
+        self.check_cut(s, 14, 9)
+        self.check_cut(u'A'*5, 3, 3)
+
+    def test_split(self):
+        w = utextwrap.UTextWrapper()
+        self.assertEqual(list(_str_D), w._split(_str_D))
+        self.assertEqual([_str_S]+list(_str_D), w._split(_str_SD))
+        self.assertEqual(list(_str_D)+[_str_S], w._split(_str_DS))
+
+    def test_wrap(self):
+        self.assertEqual(list(_str_D), utextwrap.wrap(_str_D, 1))
+        self.assertEqual(list(_str_D), utextwrap.wrap(_str_D, 2))
+        self.assertEqual(list(_str_D), utextwrap.wrap(_str_D, 3))
+        self.assertEqual(list(_str_D),
+                         utextwrap.wrap(_str_D, 3, break_long_words=False))
+
+class TestUTextFill(tests.TestCase):
+
+    def test_fill_simple(self):
+        # Test only can call fill() because it's just '\n'.join(wrap(text)).
+        self.assertEqual("%s\n%s" % (_str_D[:2], _str_D[2:]),
+                         utextwrap.fill(_str_D, 4))
+
+    def test_fill_with_breaks(self):
+        # Demonstrate complicated case.
+        text = u"spam ham egg spamhamegg" + _str_D + u" spam" + _str_D*2
+        self.assertEqual(u'\n'.join(["spam ham",
+                                     "egg spam",
+                                     "hamegg" + _str_D[0],
+                                     _str_D[1:],
+                                     "spam" + _str_D[:2],
+                                     _str_D[2:]+_str_D[:2],
+                                     _str_D[2:]]),
+                         utextwrap.fill(text, 8))
+
+    def test_fill_without_breaks(self):
+        text = u"spam ham egg spamhamegg" + _str_D + u" spam" + _str_D*2
+        self.assertEqual(u'\n'.join(["spam ham",
+                                     "egg",
+                                     "spamhamegg", 
+                                     # border between single width and double
+                                     # width.
+                                     _str_D,
+                                     "spam" + _str_D[:2],
+                                     _str_D[2:]+_str_D[:2],
+                                     _str_D[2:]]),
+                         utextwrap.fill(text, 8, break_long_words=False))
+
+    def test_fill_indent_with_breaks(self):
+        w = utextwrap.UTextWrapper(8, initial_indent=' '*4,
+                                   subsequent_indent=' '*4)
+        self.assertEqual(u'\n'.join(["    hell",
+                                     "    o" + _str_D[0],
+                                     "    " + _str_D[1:3],
+                                     "    " + _str_D[3]
+                                     ]),
+                         w.fill(_str_SD))
+
+    def test_fill_indent_without_breaks(self):
+        w = utextwrap.UTextWrapper(8, initial_indent=' '*4,
+                                   subsequent_indent=' '*4)
+        w.break_long_words = False
+        self.assertEqual(u'\n'.join(["    hello",
+                                     "    " + _str_D[:2],
+                                     "    " + _str_D[2:],
+                                     ]),
+                         w.fill(_str_SD))
+
+    def test_fill_indent_without_breaks_with_fixed_width(self):
+        w = utextwrap.UTextWrapper(8, initial_indent=' '*4,
+                                   subsequent_indent=' '*4)
+        w.break_long_words = False
+        w.width = 3
+        self.assertEqual(u'\n'.join(["    hello",
+                                     "    " + _str_D[0],
+                                     "    " + _str_D[1],
+                                     "    " + _str_D[2],
+                                     "    " + _str_D[3],
+                                     ]),
+                         w.fill(_str_SD))
+
+class TestUTextWrapAmbiWidth(tests.TestCase):
+    _cyrill_char = u"\u0410" # east_asian_width() == 'A'
+
+    def test_ambiwidth1(self):
+        w = utextwrap.UTextWrapper(4, ambiguous_width=1)
+        s = self._cyrill_char*8
+        self.assertEqual([self._cyrill_char*4]*2, w.wrap(s))
+
+    def test_ambiwidth2(self):
+        w = utextwrap.UTextWrapper(4, ambiguous_width=2)
+        s = self._cyrill_char*8
+        self.assertEqual([self._cyrill_char*2]*4, w.wrap(s))
+
+
+# Regression test with Python's test_textwrap
+# Note that some distribution including Ubuntu doesn't install
+# Python's test suite.
+try:
+    from test import test_textwrap
+
+    def override_textwrap_symbols(testcase):
+        # Override the symbols imported by test_textwrap so it uses our own
+        # replacements.
+        testcase.overrideAttr(test_textwrap, 'TextWrapper',
+                              utextwrap.UTextWrapper)
+        testcase.overrideAttr(test_textwrap, 'wrap', utextwrap.wrap)
+        testcase.overrideAttr(test_textwrap, 'fill', utextwrap.fill)
+
+
+    def setup_both(testcase, base_class, reused_class):
+        super(base_class, testcase).setUp()
+        override_textwrap_symbols(testcase)
+        reused_class.setUp(testcase)
+
+
+    class TestWrap(tests.TestCase, test_textwrap.WrapTestCase):
+
+        def setUp(self):
+            setup_both(self, TestWrap, test_textwrap.WrapTestCase)
+
+
+    class TestLongWord(tests.TestCase, test_textwrap.LongWordTestCase):
+
+        def setUp(self):
+            setup_both(self, TestLongWord, test_textwrap.LongWordTestCase)
+
+
+    class TestIndent(tests.TestCase, test_textwrap.IndentTestCases):
+
+        def setUp(self):
+            setup_both(self, TestIndent, test_textwrap.IndentTestCases)
+
+except ImportError:
+
+    class TestWrap(tests.TestCase):
+
+        def test_wrap(self):
+            raise TestSkipped("test.test_textwrap is not avialable.")
+
+    class TestLongWord(tests.TestCase):
+
+        def test_longword(self):
+            raise TestSkipped("test.test_textwrap is not avialable.")
+
+    class TestIndent(tests.TestCase):
+
+        def test_indent(self):
+            raise TestSkipped("test.test_textwrap is not avialable.")
 
=== added file 'bzrlib/utextwrap.py'
--- bzrlib/utextwrap.py	1970-01-01 00:00:00 +0000
+++ bzrlib/utextwrap.py	2011-05-14 15:01:51 +0000
@@ -0,0 +1,264 @@
+# Copyright (C) 2011 Canonical Ltd
+#
+# UTextWrapper._handle_long_word, UTextWrapper._wrap_chunks,
+# UTextWrapper._fix_sentence_endings, wrap and fill is copied from Python's
+# textwrap module (under PSF license) and modified for support CJK.
+# Original Copyright for these functions:
+#
+# Copyright (C) 1999-2001 Gregory P. Ward.
+# Copyright (C) 2002, 2003 Python Software Foundation.
+#
+# Written by Greg Ward <[e-mail]>
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+import sys
+import textwrap
+from unicodedata import east_asian_width as _eawidth
+
+from bzrlib import osutils
+
+__all__ = ["UTextWrapper", "fill", "wrap"]
+
+class UTextWrapper(textwrap.TextWrapper):
+    """
+    Extend TextWrapper for Unicode.
+
+    This textwrapper handles east asian double width and split word
+    even if !break_long_words when word contains double width
+    characters.
+
+    :param ambiguous_width: (keyword argument) width for character when
+                            unicodedata.east_asian_width(c) == 'A'
+                            (default: 1)
+
+    Limitations:
+    * expand_tabs doesn't fixed. It uses len() for calculating width
+      of string on left of TAB.
+    * Handles one codeunit as a single character having 1 or 2 width.
+      This is not correct when there are surrogate pairs, combined
+      characters or zero-width characters.
+    * Treats all asian character are line breakable. But it is not
+      true because line breaking is prohibited around some characters.
+      (For example, breaking before punctation mark is prohibited.)
+      See UAX # 14 "UNICODE LINE BREAKING ALGORITHM"
+    """
+
+    def __init__(self, width=None, **kwargs):
+        if width is None:
+            width = (osutils.terminal_width() or
+                        osutils.default_terminal_width) - 1
+
+        ambi_width = kwargs.pop('ambiguous_width', 1)
+        if ambi_width == 1:
+            self._east_asian_doublewidth = 'FW'
+        elif ambi_width == 2:
+            self._east_asian_doublewidth = 'FWA'
+        else:
+            raise ValueError("ambiguous_width should be 1 or 2")
+
+        # No drop_whitespace param before Python 2.6 it was always dropped
+        if sys.version_info < (2, 6):
+            self.drop_whitespace = kwargs.pop("drop_whitespace", True)
+            if not self.drop_whitespace:
+                raise ValueError("TextWrapper version must drop whitespace")
+        textwrap.TextWrapper.__init__(self, width, **kwargs)
+
+    def _unicode_char_width(self, uc):
+        """Return width of character `uc`.
+
+        :param:     uc      Single unicode character.
+        """
+        # 'A' means width of the character is not be able to determine.
+        # We assume that it's width is 2 because longer wrap may over
+        # terminal width but shorter wrap may be acceptable.
+        return (_eawidth(uc) in self._east_asian_doublewidth and 2) or 1
+
+    def _width(self, s):
+        """Returns width for s.
+
+        When s is unicode, take care of east asian width.
+        When s is bytes, treat all byte is single width character.
+        """
+        charwidth = self._unicode_char_width
+        return sum(charwidth(c) for c in s)
+
+    def _cut(self, s, width):
+        """Returns head and rest of s. (head+rest == s)
+
+        Head is large as long as _width(head) <= width.
+        """
+        w = 0
+        charwidth = self._unicode_char_width
+        for pos, c in enumerate(s):
+            w += charwidth(c)
+            if w > width:
+                return s[:pos], s[pos:]
+        return s, u''
+
+    def _fix_sentence_endings(self, chunks):
+        """_fix_sentence_endings(chunks : [string])
+
+        Correct for sentence endings buried in 'chunks'.  Eg. when the
+        original text contains "... foo.\nBar ...", munge_whitespace()
+        and split() will convert that to [..., "foo.", " ", "Bar", ...]
+        which has one too few spaces; this method simply changes the one
+        space to two.
+
+        Note: This function is copied from textwrap.TextWrap and modified
+        to use unicode always.
+        """
+        i = 0
+        L = len(chunks)-1
+        patsearch = self.sentence_end_re.search
+        while i < L:
+            if chunks[i+1] == u" " and patsearch(chunks[i]):
+                chunks[i+1] = u"  "
+                i += 2
+            else:
+                i += 1
+
+    def _handle_long_word(self, chunks, cur_line, cur_len, width):
+        # Figure out when indent is larger than the specified width, and make
+        # sure at least one character is stripped off on every pass
+        if width < 2:
+            space_left = chunks[-1] and self._width(chunks[-1][0]) or 1
+        else:
+            space_left = width - cur_len
+
+        # If we're allowed to break long words, then do so: put as much
+        # of the next chunk onto the current line as will fit.
+        if self.break_long_words:
+            head, rest = self._cut(chunks[-1], space_left)
+            cur_line.append(head)
+            if rest:
+                chunks[-1] = rest
+            else:
+                del chunks[-1]
+
+        # Otherwise, we have to preserve the long word intact.  Only add
+        # it to the current line if there's nothing already there --
+        # that minimizes how much we violate the width constraint.
+        elif not cur_line:
+            cur_line.append(chunks.pop())
+
+        # If we're not allowed to break long words, and there's already
+        # text on the current line, do nothing.  Next time through the
+        # main loop of _wrap_chunks(), we'll wind up here again, but
+        # cur_len will be zero, so the next line will be entirely
+        # devoted to the long word that we can't handle right now.
+
+    def _wrap_chunks(self, chunks):
+        lines = []
+        if self.width <= 0:
+            raise ValueError("invalid width %r (must be > 0)" % self.width)
+
+        # Arrange in reverse order so items can be efficiently popped
+        # from a stack of chucks.
+        chunks.reverse()
+
+        while chunks:
+
+            # Start the list of chunks that will make up the current line.
+            # cur_len is just the length of all the chunks in cur_line.
+            cur_line = []
+            cur_len = 0
+
+            # Figure out which static string will prefix this line.
+            if lines:
+                indent = self.subsequent_indent
+            else:
+                indent = self.initial_indent
+
+            # Maximum width for this line.
+            width = self.width - len(indent)
+
+            # First chunk on line is whitespace -- drop it, unless this
+            # is the very beginning of the text (ie. no lines started yet).
+            if self.drop_whitespace and chunks[-1].strip() == '' and lines:
+                del chunks[-1]
+
+            while chunks:
+                # Use _width instead of len for east asian width
+                l = self._width(chunks[-1])
+
+                # Can at least squeeze this chunk onto the current line.
+                if cur_len + l <= width:
+                    cur_line.append(chunks.pop())
+                    cur_len += l
+
+                # Nope, this line is full.
+                else:
+                    break
+
+            # The current line is full, and the next chunk is too big to
+            # fit on *any* line (not just this one).
+            if chunks and self._width(chunks[-1]) > width:
+                self._handle_long_word(chunks, cur_line, cur_len, width)
+
+            # If the last chunk on this line is all whitespace, drop it.
+            if self.drop_whitespace and cur_line and not cur_line[-1].strip():
+                del cur_line[-1]
+
+            # Convert current line back to a string and store it in list
+            # of all lines (return value).
+            if cur_line:
+                lines.append(indent + u''.join(cur_line))
+
+        return lines
+
+    def _split(self, text):
+        chunks = textwrap.TextWrapper._split(self, unicode(text))
+        cjk_split_chunks = []
+        for chunk in chunks:
+            prev_pos = 0
+            for pos, char in enumerate(chunk):
+                if self._unicode_char_width(char) == 2:
+                    if prev_pos < pos:
+                        cjk_split_chunks.append(chunk[prev_pos:pos])
+                    cjk_split_chunks.append(char)
+                    prev_pos = pos+1
+            if prev_pos < len(chunk):
+                cjk_split_chunks.append(chunk[prev_pos:])
+        return cjk_split_chunks
+
+    def wrap(self, text):
+        # ensure text is unicode
+        return textwrap.TextWrapper.wrap(self, unicode(text))
+
+# -- Convenience interface ---------------------------------------------
+
+def wrap(text, width=None, **kwargs):
+    """Wrap a single paragraph of text, returning a list of wrapped lines.
+
+    Reformat the single paragraph in 'text' so it fits in lines of no
+    more than 'width' columns, and return a list of wrapped lines.  By
+    default, tabs in 'text' are expanded with string.expandtabs(), and
+    all other whitespace characters (including newline) are converted to
+    space.  See TextWrapper class for available keyword args to customize
+    wrapping behaviour.
+    """
+    return UTextWrapper(width=width, **kwargs).wrap(text)
+
+def fill(text, width=None, **kwargs):
+    """Fill a single paragraph of text, returning a new string.
+
+    Reformat the single paragraph in 'text' to fit in lines of no more
+    than 'width' columns, and return a new string containing the entire
+    wrapped paragraph.  As with wrap(), tabs are expanded and other
+    whitespace characters converted to space.  See TextWrapper class for
+    available keyword args to customize wrapping behaviour.
+    """
+    return UTextWrapper(width=width, **kwargs).fill(text)
+