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

Martin Pool (mbp) wrote :

- argv = _specified_or_unicode_argv(argv)
+ if argv is not None:
+ argv = argv[1:]

It looks like you have one too few spaces there.

The whole topic of locales is probably a bit too complex to explain in an error message. We could mention a URL about it.

+++ bzrlib/osutils.py 2011-04-15 00:49:24 +0000
@@ -96,8 +96,8 @@
         user_encoding = get_user_encoding()
         return [a.decode(user_encoding) for a in sys.argv[1:]]
     except UnicodeDecodeError:
- raise errors.BzrError(("Parameter '%r' is unsupported by the current "
- "encoding." % a))
+ raise errors.BzrError("Parameter %r encoding is unsupported by the "
+ "application locale." % (a,))

One thing I would do here is make the error message include the detected encoding to give people an idea where to start. (It may already be in the crash report but perhaps that's not so visible.)

Aside from that and the point about skip it looks good. Thanks for tackling it.

review: Needs Fixing

« Back to merge proposal