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

Proposed by methane
Status: Merged
Approved by: Martin Packman
Approved revision: no longer in the source branch.
Merged at revision: 5874
Proposed branch: lp:~songofacandy/bzr/i18n-utextwrap
Merge into: lp:bzr
Diff against target: 492 lines (+472/-0)
3 files modified
bzrlib/tests/__init__.py (+1/-0)
bzrlib/tests/test_utextwrap.py (+207/-0)
bzrlib/utextwrap.py (+264/-0)
To merge this branch: bzr merge lp:~songofacandy/bzr/i18n-utextwrap
Reviewer Review Type Date Requested Status
Martin Packman (community) Approve
Vincent Ladeuil Approve
Review via email: mp+59950@code.launchpad.net

Commit message

Add support for double width characters to textwrap.

Description of the change

I18n part 1
textwrap supports double width characters.

To post a comment you must log in.
Revision history for this message
Vincent Ladeuil (vila) wrote :

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
Revision history for this message
Alexander Belchenko (bialix) wrote :

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.

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

> 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 ?

Revision history for this message
methane (songofacandy) wrote :

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.

Revision history for this message
Alexander Belchenko (bialix) wrote :

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

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

> 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 ;)

Revision history for this message
Alexander Belchenko (bialix) wrote :

> > 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 :-)

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

> 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
Revision history for this message
methane (songofacandy) wrote :

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

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

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

review: Approve
Revision history for this message
Martin Packman (gz) wrote :
Download full text (4.7 KiB)

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
...
+ ...

Read more...

Revision history for this message
Martin Packman (gz) :
review: Needs Fixing
Revision history for this message
John A Meinel (jameinel) wrote :

-----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-----

Revision history for this message
methane (songofacandy) wrote :
Download full text (4.9 KiB)

> +# 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...

Read more...

Revision history for this message
Martin Packman (gz) wrote :

> 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.

Revision history for this message
Martin Packman (gz) wrote :

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.

Revision history for this message
methane (songofacandy) wrote :

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.

Revision history for this message
John C Barstow (jbowtie) wrote :

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.

Revision history for this message
Martin Packman (gz) wrote :

> 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.

Revision history for this message
methane (songofacandy) wrote :

> > 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.

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

@gz: Are you ok to land this now ?

Revision history for this message
Martin Packman (gz) wrote :

+ 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.

Revision history for this message
methane (songofacandy) wrote :

> + 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.

Revision history for this message
Martin Packman (gz) 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.

Revision history for this message
methane (songofacandy) wrote :

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>

Revision history for this message
Martin Packman (gz) wrote :

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

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

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bzrlib/tests/__init__.py'
2--- bzrlib/tests/__init__.py 2011-05-13 12:51:05 +0000
3+++ bzrlib/tests/__init__.py 2011-05-14 15:01:51 +0000
4@@ -3899,6 +3899,7 @@
5 'bzrlib.tests.test_upgrade',
6 'bzrlib.tests.test_upgrade_stacked',
7 'bzrlib.tests.test_urlutils',
8+ 'bzrlib.tests.test_utextwrap',
9 'bzrlib.tests.test_version',
10 'bzrlib.tests.test_version_info',
11 'bzrlib.tests.test_versionedfile',
12
13=== added file 'bzrlib/tests/test_utextwrap.py'
14--- bzrlib/tests/test_utextwrap.py 1970-01-01 00:00:00 +0000
15+++ bzrlib/tests/test_utextwrap.py 2011-05-14 15:01:51 +0000
16@@ -0,0 +1,207 @@
17+# Copyright (C) 2011 Canonical Ltd
18+#
19+# This program is free software; you can redistribute it and/or modify
20+# it under the terms of the GNU General Public License as published by
21+# the Free Software Foundation; either version 2 of the License, or
22+# (at your option) any later version.
23+#
24+# This program is distributed in the hope that it will be useful,
25+# but WITHOUT ANY WARRANTY; without even the implied warranty of
26+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
27+# GNU General Public License for more details.
28+#
29+# You should have received a copy of the GNU General Public License
30+# along with this program; if not, write to the Free Software
31+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
32+#
33+
34+"""Tests of the bzrlib.utextwrap."""
35+
36+from bzrlib import tests, utextwrap
37+from bzrlib.tests import TestSkipped
38+
39+# Japanese "Good morning".
40+# Each character have double width. So total 8 width on console.
41+_str_D = u'\u304a\u306f\u3088\u3046'
42+
43+_str_S = u"hello"
44+
45+# Combine single width characters and double width characters.
46+_str_SD = _str_S + _str_D
47+_str_DS = _str_D + _str_S
48+
49+class TestUTextWrap(tests.TestCase):
50+
51+ def check_width(self, text, expected_width):
52+ w = utextwrap.UTextWrapper()
53+ self.assertEqual(
54+ w._width(text),
55+ expected_width,
56+ "Width of %r should be %d" % (text, expected_width))
57+
58+ def test_width(self):
59+ self.check_width(_str_D, 8)
60+ self.check_width(_str_SD, 13)
61+
62+ def check_cut(self, text, width, pos):
63+ w = utextwrap.UTextWrapper()
64+ self.assertEqual((text[:pos], text[pos:]), w._cut(text, width))
65+
66+ def test_cut(self):
67+ s = _str_SD
68+ self.check_cut(s, 0, 0)
69+ self.check_cut(s, 1, 1)
70+ self.check_cut(s, 5, 5)
71+ self.check_cut(s, 6, 5)
72+ self.check_cut(s, 7, 6)
73+ self.check_cut(s, 12, 8)
74+ self.check_cut(s, 13, 9)
75+ self.check_cut(s, 14, 9)
76+ self.check_cut(u'A'*5, 3, 3)
77+
78+ def test_split(self):
79+ w = utextwrap.UTextWrapper()
80+ self.assertEqual(list(_str_D), w._split(_str_D))
81+ self.assertEqual([_str_S]+list(_str_D), w._split(_str_SD))
82+ self.assertEqual(list(_str_D)+[_str_S], w._split(_str_DS))
83+
84+ def test_wrap(self):
85+ self.assertEqual(list(_str_D), utextwrap.wrap(_str_D, 1))
86+ self.assertEqual(list(_str_D), utextwrap.wrap(_str_D, 2))
87+ self.assertEqual(list(_str_D), utextwrap.wrap(_str_D, 3))
88+ self.assertEqual(list(_str_D),
89+ utextwrap.wrap(_str_D, 3, break_long_words=False))
90+
91+class TestUTextFill(tests.TestCase):
92+
93+ def test_fill_simple(self):
94+ # Test only can call fill() because it's just '\n'.join(wrap(text)).
95+ self.assertEqual("%s\n%s" % (_str_D[:2], _str_D[2:]),
96+ utextwrap.fill(_str_D, 4))
97+
98+ def test_fill_with_breaks(self):
99+ # Demonstrate complicated case.
100+ text = u"spam ham egg spamhamegg" + _str_D + u" spam" + _str_D*2
101+ self.assertEqual(u'\n'.join(["spam ham",
102+ "egg spam",
103+ "hamegg" + _str_D[0],
104+ _str_D[1:],
105+ "spam" + _str_D[:2],
106+ _str_D[2:]+_str_D[:2],
107+ _str_D[2:]]),
108+ utextwrap.fill(text, 8))
109+
110+ def test_fill_without_breaks(self):
111+ text = u"spam ham egg spamhamegg" + _str_D + u" spam" + _str_D*2
112+ self.assertEqual(u'\n'.join(["spam ham",
113+ "egg",
114+ "spamhamegg",
115+ # border between single width and double
116+ # width.
117+ _str_D,
118+ "spam" + _str_D[:2],
119+ _str_D[2:]+_str_D[:2],
120+ _str_D[2:]]),
121+ utextwrap.fill(text, 8, break_long_words=False))
122+
123+ def test_fill_indent_with_breaks(self):
124+ w = utextwrap.UTextWrapper(8, initial_indent=' '*4,
125+ subsequent_indent=' '*4)
126+ self.assertEqual(u'\n'.join([" hell",
127+ " o" + _str_D[0],
128+ " " + _str_D[1:3],
129+ " " + _str_D[3]
130+ ]),
131+ w.fill(_str_SD))
132+
133+ def test_fill_indent_without_breaks(self):
134+ w = utextwrap.UTextWrapper(8, initial_indent=' '*4,
135+ subsequent_indent=' '*4)
136+ w.break_long_words = False
137+ self.assertEqual(u'\n'.join([" hello",
138+ " " + _str_D[:2],
139+ " " + _str_D[2:],
140+ ]),
141+ w.fill(_str_SD))
142+
143+ def test_fill_indent_without_breaks_with_fixed_width(self):
144+ w = utextwrap.UTextWrapper(8, initial_indent=' '*4,
145+ subsequent_indent=' '*4)
146+ w.break_long_words = False
147+ w.width = 3
148+ self.assertEqual(u'\n'.join([" hello",
149+ " " + _str_D[0],
150+ " " + _str_D[1],
151+ " " + _str_D[2],
152+ " " + _str_D[3],
153+ ]),
154+ w.fill(_str_SD))
155+
156+class TestUTextWrapAmbiWidth(tests.TestCase):
157+ _cyrill_char = u"\u0410" # east_asian_width() == 'A'
158+
159+ def test_ambiwidth1(self):
160+ w = utextwrap.UTextWrapper(4, ambiguous_width=1)
161+ s = self._cyrill_char*8
162+ self.assertEqual([self._cyrill_char*4]*2, w.wrap(s))
163+
164+ def test_ambiwidth2(self):
165+ w = utextwrap.UTextWrapper(4, ambiguous_width=2)
166+ s = self._cyrill_char*8
167+ self.assertEqual([self._cyrill_char*2]*4, w.wrap(s))
168+
169+
170+# Regression test with Python's test_textwrap
171+# Note that some distribution including Ubuntu doesn't install
172+# Python's test suite.
173+try:
174+ from test import test_textwrap
175+
176+ def override_textwrap_symbols(testcase):
177+ # Override the symbols imported by test_textwrap so it uses our own
178+ # replacements.
179+ testcase.overrideAttr(test_textwrap, 'TextWrapper',
180+ utextwrap.UTextWrapper)
181+ testcase.overrideAttr(test_textwrap, 'wrap', utextwrap.wrap)
182+ testcase.overrideAttr(test_textwrap, 'fill', utextwrap.fill)
183+
184+
185+ def setup_both(testcase, base_class, reused_class):
186+ super(base_class, testcase).setUp()
187+ override_textwrap_symbols(testcase)
188+ reused_class.setUp(testcase)
189+
190+
191+ class TestWrap(tests.TestCase, test_textwrap.WrapTestCase):
192+
193+ def setUp(self):
194+ setup_both(self, TestWrap, test_textwrap.WrapTestCase)
195+
196+
197+ class TestLongWord(tests.TestCase, test_textwrap.LongWordTestCase):
198+
199+ def setUp(self):
200+ setup_both(self, TestLongWord, test_textwrap.LongWordTestCase)
201+
202+
203+ class TestIndent(tests.TestCase, test_textwrap.IndentTestCases):
204+
205+ def setUp(self):
206+ setup_both(self, TestIndent, test_textwrap.IndentTestCases)
207+
208+except ImportError:
209+
210+ class TestWrap(tests.TestCase):
211+
212+ def test_wrap(self):
213+ raise TestSkipped("test.test_textwrap is not avialable.")
214+
215+ class TestLongWord(tests.TestCase):
216+
217+ def test_longword(self):
218+ raise TestSkipped("test.test_textwrap is not avialable.")
219+
220+ class TestIndent(tests.TestCase):
221+
222+ def test_indent(self):
223+ raise TestSkipped("test.test_textwrap is not avialable.")
224
225=== added file 'bzrlib/utextwrap.py'
226--- bzrlib/utextwrap.py 1970-01-01 00:00:00 +0000
227+++ bzrlib/utextwrap.py 2011-05-14 15:01:51 +0000
228@@ -0,0 +1,264 @@
229+# Copyright (C) 2011 Canonical Ltd
230+#
231+# UTextWrapper._handle_long_word, UTextWrapper._wrap_chunks,
232+# UTextWrapper._fix_sentence_endings, wrap and fill is copied from Python's
233+# textwrap module (under PSF license) and modified for support CJK.
234+# Original Copyright for these functions:
235+#
236+# Copyright (C) 1999-2001 Gregory P. Ward.
237+# Copyright (C) 2002, 2003 Python Software Foundation.
238+#
239+# Written by Greg Ward <gward@python.net>
240+# This program is free software; you can redistribute it and/or modify
241+# it under the terms of the GNU General Public License as published by
242+# the Free Software Foundation; either version 2 of the License, or
243+# (at your option) any later version.
244+#
245+# This program is distributed in the hope that it will be useful,
246+# but WITHOUT ANY WARRANTY; without even the implied warranty of
247+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
248+# GNU General Public License for more details.
249+#
250+# You should have received a copy of the GNU General Public License
251+# along with this program; if not, write to the Free Software
252+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
253+
254+import sys
255+import textwrap
256+from unicodedata import east_asian_width as _eawidth
257+
258+from bzrlib import osutils
259+
260+__all__ = ["UTextWrapper", "fill", "wrap"]
261+
262+class UTextWrapper(textwrap.TextWrapper):
263+ """
264+ Extend TextWrapper for Unicode.
265+
266+ This textwrapper handles east asian double width and split word
267+ even if !break_long_words when word contains double width
268+ characters.
269+
270+ :param ambiguous_width: (keyword argument) width for character when
271+ unicodedata.east_asian_width(c) == 'A'
272+ (default: 1)
273+
274+ Limitations:
275+ * expand_tabs doesn't fixed. It uses len() for calculating width
276+ of string on left of TAB.
277+ * Handles one codeunit as a single character having 1 or 2 width.
278+ This is not correct when there are surrogate pairs, combined
279+ characters or zero-width characters.
280+ * Treats all asian character are line breakable. But it is not
281+ true because line breaking is prohibited around some characters.
282+ (For example, breaking before punctation mark is prohibited.)
283+ See UAX # 14 "UNICODE LINE BREAKING ALGORITHM"
284+ """
285+
286+ def __init__(self, width=None, **kwargs):
287+ if width is None:
288+ width = (osutils.terminal_width() or
289+ osutils.default_terminal_width) - 1
290+
291+ ambi_width = kwargs.pop('ambiguous_width', 1)
292+ if ambi_width == 1:
293+ self._east_asian_doublewidth = 'FW'
294+ elif ambi_width == 2:
295+ self._east_asian_doublewidth = 'FWA'
296+ else:
297+ raise ValueError("ambiguous_width should be 1 or 2")
298+
299+ # No drop_whitespace param before Python 2.6 it was always dropped
300+ if sys.version_info < (2, 6):
301+ self.drop_whitespace = kwargs.pop("drop_whitespace", True)
302+ if not self.drop_whitespace:
303+ raise ValueError("TextWrapper version must drop whitespace")
304+ textwrap.TextWrapper.__init__(self, width, **kwargs)
305+
306+ def _unicode_char_width(self, uc):
307+ """Return width of character `uc`.
308+
309+ :param: uc Single unicode character.
310+ """
311+ # 'A' means width of the character is not be able to determine.
312+ # We assume that it's width is 2 because longer wrap may over
313+ # terminal width but shorter wrap may be acceptable.
314+ return (_eawidth(uc) in self._east_asian_doublewidth and 2) or 1
315+
316+ def _width(self, s):
317+ """Returns width for s.
318+
319+ When s is unicode, take care of east asian width.
320+ When s is bytes, treat all byte is single width character.
321+ """
322+ charwidth = self._unicode_char_width
323+ return sum(charwidth(c) for c in s)
324+
325+ def _cut(self, s, width):
326+ """Returns head and rest of s. (head+rest == s)
327+
328+ Head is large as long as _width(head) <= width.
329+ """
330+ w = 0
331+ charwidth = self._unicode_char_width
332+ for pos, c in enumerate(s):
333+ w += charwidth(c)
334+ if w > width:
335+ return s[:pos], s[pos:]
336+ return s, u''
337+
338+ def _fix_sentence_endings(self, chunks):
339+ """_fix_sentence_endings(chunks : [string])
340+
341+ Correct for sentence endings buried in 'chunks'. Eg. when the
342+ original text contains "... foo.\nBar ...", munge_whitespace()
343+ and split() will convert that to [..., "foo.", " ", "Bar", ...]
344+ which has one too few spaces; this method simply changes the one
345+ space to two.
346+
347+ Note: This function is copied from textwrap.TextWrap and modified
348+ to use unicode always.
349+ """
350+ i = 0
351+ L = len(chunks)-1
352+ patsearch = self.sentence_end_re.search
353+ while i < L:
354+ if chunks[i+1] == u" " and patsearch(chunks[i]):
355+ chunks[i+1] = u" "
356+ i += 2
357+ else:
358+ i += 1
359+
360+ def _handle_long_word(self, chunks, cur_line, cur_len, width):
361+ # Figure out when indent is larger than the specified width, and make
362+ # sure at least one character is stripped off on every pass
363+ if width < 2:
364+ space_left = chunks[-1] and self._width(chunks[-1][0]) or 1
365+ else:
366+ space_left = width - cur_len
367+
368+ # If we're allowed to break long words, then do so: put as much
369+ # of the next chunk onto the current line as will fit.
370+ if self.break_long_words:
371+ head, rest = self._cut(chunks[-1], space_left)
372+ cur_line.append(head)
373+ if rest:
374+ chunks[-1] = rest
375+ else:
376+ del chunks[-1]
377+
378+ # Otherwise, we have to preserve the long word intact. Only add
379+ # it to the current line if there's nothing already there --
380+ # that minimizes how much we violate the width constraint.
381+ elif not cur_line:
382+ cur_line.append(chunks.pop())
383+
384+ # If we're not allowed to break long words, and there's already
385+ # text on the current line, do nothing. Next time through the
386+ # main loop of _wrap_chunks(), we'll wind up here again, but
387+ # cur_len will be zero, so the next line will be entirely
388+ # devoted to the long word that we can't handle right now.
389+
390+ def _wrap_chunks(self, chunks):
391+ lines = []
392+ if self.width <= 0:
393+ raise ValueError("invalid width %r (must be > 0)" % self.width)
394+
395+ # Arrange in reverse order so items can be efficiently popped
396+ # from a stack of chucks.
397+ chunks.reverse()
398+
399+ while chunks:
400+
401+ # Start the list of chunks that will make up the current line.
402+ # cur_len is just the length of all the chunks in cur_line.
403+ cur_line = []
404+ cur_len = 0
405+
406+ # Figure out which static string will prefix this line.
407+ if lines:
408+ indent = self.subsequent_indent
409+ else:
410+ indent = self.initial_indent
411+
412+ # Maximum width for this line.
413+ width = self.width - len(indent)
414+
415+ # First chunk on line is whitespace -- drop it, unless this
416+ # is the very beginning of the text (ie. no lines started yet).
417+ if self.drop_whitespace and chunks[-1].strip() == '' and lines:
418+ del chunks[-1]
419+
420+ while chunks:
421+ # Use _width instead of len for east asian width
422+ l = self._width(chunks[-1])
423+
424+ # Can at least squeeze this chunk onto the current line.
425+ if cur_len + l <= width:
426+ cur_line.append(chunks.pop())
427+ cur_len += l
428+
429+ # Nope, this line is full.
430+ else:
431+ break
432+
433+ # The current line is full, and the next chunk is too big to
434+ # fit on *any* line (not just this one).
435+ if chunks and self._width(chunks[-1]) > width:
436+ self._handle_long_word(chunks, cur_line, cur_len, width)
437+
438+ # If the last chunk on this line is all whitespace, drop it.
439+ if self.drop_whitespace and cur_line and not cur_line[-1].strip():
440+ del cur_line[-1]
441+
442+ # Convert current line back to a string and store it in list
443+ # of all lines (return value).
444+ if cur_line:
445+ lines.append(indent + u''.join(cur_line))
446+
447+ return lines
448+
449+ def _split(self, text):
450+ chunks = textwrap.TextWrapper._split(self, unicode(text))
451+ cjk_split_chunks = []
452+ for chunk in chunks:
453+ prev_pos = 0
454+ for pos, char in enumerate(chunk):
455+ if self._unicode_char_width(char) == 2:
456+ if prev_pos < pos:
457+ cjk_split_chunks.append(chunk[prev_pos:pos])
458+ cjk_split_chunks.append(char)
459+ prev_pos = pos+1
460+ if prev_pos < len(chunk):
461+ cjk_split_chunks.append(chunk[prev_pos:])
462+ return cjk_split_chunks
463+
464+ def wrap(self, text):
465+ # ensure text is unicode
466+ return textwrap.TextWrapper.wrap(self, unicode(text))
467+
468+# -- Convenience interface ---------------------------------------------
469+
470+def wrap(text, width=None, **kwargs):
471+ """Wrap a single paragraph of text, returning a list of wrapped lines.
472+
473+ Reformat the single paragraph in 'text' so it fits in lines of no
474+ more than 'width' columns, and return a list of wrapped lines. By
475+ default, tabs in 'text' are expanded with string.expandtabs(), and
476+ all other whitespace characters (including newline) are converted to
477+ space. See TextWrapper class for available keyword args to customize
478+ wrapping behaviour.
479+ """
480+ return UTextWrapper(width=width, **kwargs).wrap(text)
481+
482+def fill(text, width=None, **kwargs):
483+ """Fill a single paragraph of text, returning a new string.
484+
485+ Reformat the single paragraph in 'text' to fit in lines of no more
486+ than 'width' columns, and return a new string containing the entire
487+ wrapped paragraph. As with wrap(), tabs are expanded and other
488+ whitespace characters converted to space. See TextWrapper class for
489+ available keyword args to customize wrapping behaviour.
490+ """
491+ return UTextWrapper(width=width, **kwargs).fill(text)
492+