[MERGE] setlocale

[Martin Pool]

On Tue, Jun 17, 2008 at 5:39 AM, Martin von Gagern
<Martin.vGagern at gmx.net> wrote:
> Hi!
>
> This is a patch for bzr to call setlocale in order to set the current
> program locale according to the environment. This is required by some third
> party libraries in order to correctly determine things like the file system
> character encoding. It has the side effect of using locale specific names
> for days of the week in some cases.

That sounds reasonable; I guess i'd like to hear from Jelmer whether
he thinks this will work well with bzr-svn as that's where it
originally came up.

> === modified file 'bzr'
> --- bzr 2008-05-10 11:33:32 +0000
> +++ bzr 2008-06-08 14:24:56 +0000
> @@ -59,6 +59,26 @@
>     profile_imports.install()
>     profiling = True
>
> +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.

> +try:
> +    locale.setlocale(locale.LC_ALL, '')
> +except locale.Error, e:
> +    sys.stderr.write('bzr: warning: %s\n'
> +                     '  bzr could not set the application locale.\n'
> +                     '  Although this should be no problem for bzr
> itself,\n'
> +                     '  it might cause problems with some plugins.\n'
> +                     '  To investigate the issue, look at the output\n'
> +                     '  of the locale(1p) tool available on POSIX
> systems.\n'
> +                     % e)
>
>  try:
>     import bzrlib

This seems reasonable.

>
> === modified file 'bzrlib/log.py'
> --- bzrlib/log.py       2008-05-30 00:22:33 +0000
> +++ bzrlib/log.py       2008-06-08 18:10:37 +0000
> @@ -698,9 +698,6 @@
>
>     def log_revision(self, revision):
>         to_file = self.to_file
> -        date_str = format_date(revision.rev.timestamp,
> -                               revision.rev.timezone or 0,
> -                               self.show_timezone)
>         is_merge = ''
>         if len(revision.rev.parent_ids) > 1:
>             is_merge = ' [merge]'
>

OK, this was never used.

> === modified file 'bzrlib/osutils.py'
> --- bzrlib/osutils.py   2008-06-11 03:56:46 +0000
> +++ bzrlib/osutils.py   2008-06-16 19:13:43 +0000
> @@ -672,7 +672,13 @@
>         offset_str = ' %+03d%02d' % (offset / 3600, (offset / 60) % 60)
>     else:
>         offset_str = ''
> -    return (time.strftime(date_fmt, tt) +  offset_str)
> +    date_str = time.strftime(date_fmt, tt)
> +    if not isinstance(date_str, unicode):
> +        try:
> +            date_str = date_str.decode('utf8')
> +        except UnicodeDecodeError:
> +            date_str = date_str.decode(bzrlib.user_encoding, 'replace')
> +    return date_str + offset_str
>
>
>  def compact_date(when):
>
> === modified file 'bzrlib/tests/__init__.py'
> --- bzrlib/tests/__init__.py    2008-06-06 13:56:24 +0000
> +++ bzrlib/tests/__init__.py    2008-06-16 18:19:47 +0000
> @@ -106,6 +106,26 @@
>
>  default_transport = LocalURLServer
>
> +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.

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

(I haven't read the whole thing yet but hope that helps.)

-- 
Martin <http://launchpad.net/~mbp/>