Code review comment for lp:~gz/bzr/conflicts_non_ascii_ui_686161

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

On 5/20/2011 3:26 PM, Martin [gz] wrote:
> Martin [gz] has proposed merging lp:~gz/bzr/conflicts_non_ascii_ui_686161 into lp:bzr with lp:~gz/bzr/trivial_refactor_conflict_stanza_tests as a prerequisite.
>
> Requested reviews:
> bzr-core (bzr-core)
> Related bugs:
> Bug #686161 in Bazaar: "conflicts involving unicode paths can't be displayed"
> https://bugs.launchpad.net/bzr/+bug/686161
>
> For more details, see:
> https://code.launchpad.net/~gz/bzr/conflicts_non_ascii_ui_686161/+merge/61763
>
> Conflict objects contain paths, which are unicode, but the conflicts ui calls str() on them which only works for ascii paths.
>
> This branch adds tests that cover the Conflict stringifying code, and changes str() calls. Note, despite changing the __str__ method to __unicode__ this won't protect other callers from doing the wrong thing. Anyone currently using str() will continue to work in the ascii case and continue to fail in the non-ascii case due to the joys of Python string logic.
>
> A particularly fun gotcha was the per-conflict tests actually passed regardless, because of this:
>
>>>> class U(object):
> ... def __str__(self):
> ... return u"\xa7"
> ...
>>>> unicode(U())
> u'\xa7'
>>>> str(U())
> Traceback (most recent call last):
> File "<stdin>", line 1, in <module>
> UnicodeEncodeError: 'ascii' codec can't encode character u'\xa7' in position 0: ordinal not in range(128)
>

Looks fine to me.

 merge: approve

John
=:->

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

iEYEARECAAYFAk3WcnsACgkQJdeBCYSNAAOJ0wCguczgoaf5SdOLfFbLRNFVucBU
RR8AnRQy0plKnZBN0vnex9aune5M+HWH
=BhG6
-----END PGP SIGNATURE-----