[MERGE] setlocale (#128496)

Martin von Gagern Martin.vGagern at gmx.net
Tue Jun 17 09:38:26 BST 2008


Martin Pool wrote:
>> +if sys.platform == 'darwin':
>> +    # work around egregious python 2.4 bug
>> +    sys.platform = 'posix'
>> +    try:
>> +        import locale
>> +    finally:
>> +        sys.platform = 'darwin'
>> +else:
>> +    import locale
>> +
> 
> This is duplicated (rather than moved) from osutils, which seems
> undesirable.  I believe this hack just needs to be done when the
> locale module is loaded and so doing it only from bzr would be more
> appropriate than inside osutils.

Hmmm... I hope using this hack at first occurrence is enough, as I now 
notice that I'm importing the locale in other places as well. Removed 
the duplicate code from osutils, but someone on darwin with python 2.4 
should probably test this.

>> +def _safe_string(string_or_unicode):
>> +    try:
>> +        if isinstance(string_or_unicode, str):
>> +            return string_or_unicode.encode('string_escape').replace(r'\n',
>> '\n')
>> +        if isinstance(string_or_unicode, unicode):
>> +            return
>> string_or_unicode.encode('unicode_escape').replace(r'\n', '\n')
>> +        if string_or_unicode is None:
>> +            return '<None>'
>> +        return str(string_or_unicode).encode('string_escape')
>> +    except Exception:
>> +        pass
>> +    try:
>> +        return ('%r' % string_or_unicode).encode('string_escape')
>> +    except Exception:
>> +        pass
>> +    try:
>> +        return "unconvertable %s" % str(string_or_unicode.__class__)
>> +    except Exception:
>> +        return "unconvertable thingy"
>> +
> 
> Discarding all exceptions like this is not ok; it might be something
> like KeyboardInterrupt that should stop the whole process.  I'm not
> totally clear what this is for.

I had to make the testing framework more robust here, as I repeatedly 
encountered errors while reporting tests, which would crash the whole 
selftest. First I only had the contents of the first try block, but when 
that failed as well with some TypeError or ValueError or some such, I 
got desperate and wanted any kind of output at all instead of the test 
suite breaking.

Notice that a KeyboardInterrupt is a BaseException but not an instance 
of Exception, so there wouldn't be any problem in my code for this. On 
the other hand, I might as well use StandardError, as that should handle 
all the cases I encountered but not too much more.

> Also, rather than changing so many callers, perhaps you need an
> equivalent to assertEqualDiff that knows what encodings are expected?

In a perfect world, I would assume the output to assertEqualDiff to 
always be 7-bit ASCII, so that terminal settings won't interfere with 
the viewability of the test result. Also assertEqualDiff has problems 
highlighting diffs for multi-byte characters. But as we don't live in a 
perfect world, and as I don't intend to write a testsuite for the 
complete testsuite, breaking each test to see whether its output 
displays correctly, I instead opted for this approach, to try displaying 
the results as done so far, but if that fails, revert to something that 
should make sense in most cases, should be displayable anywhere, and 
doesn't break.

Resubmission attached.

Greetings,
  Martin

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: setlocale-3494.patch
Url: https://lists.ubuntu.com/archives/bazaar/attachments/20080617/10be793d/attachment-0001.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 260 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080617/10be793d/attachment-0001.pgp 


More information about the bazaar mailing list