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

Martin Packman (gz) wrote :

> It looks like you have one too few spaces there.

Worse, a tab. I've rewritten history to hide that embarrassment, shouldn't try and finish branches late.

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

Along the lines of "See `bzr help locale` for more information."?

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

Will do, I nearly did this but the spelling of ascii as 'ANSI_X3.4-1968' looked needlessly scary to me.

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

Skip change also in the new-old revisions.

Left to do is the bb.test_commit failure, and also one in bb.test_command_encoding which does something dodgy (with a comment saying as much).

« Back to merge proposal