Merge lp://qastaging/~frankban/juju-quickstart/check-locale into lp://qastaging/juju-quickstart

Proposed by Francesco Banconi
Status: Merged
Merged at revision: 67
Proposed branch: lp://qastaging/~frankban/juju-quickstart/check-locale
Merge into: lp://qastaging/juju-quickstart
Diff against target: 98 lines (+55/-0)
2 files modified
quickstart/cli/base.py (+20/-0)
quickstart/tests/cli/test_base.py (+35/-0)
To merge this branch: bzr merge lp://qastaging/~frankban/juju-quickstart/check-locale
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+217067@code.qastaging.launchpad.net

Description of the change

Check unicode support.

Ensure utf-8 is supported before starting
the Urwid interactive session.
This way we hope to avoid bad crashes
when the user has locale configuration problems.

Tests: `make check`.

QA:
If you have juju-quickstart installed, you should
be able to make it crash by overriding all the language
env vars, like the following:
`LC_ALL=C juju-quickstart -i`
You should see a UnicodeEncodeError:
'ascii' codec can't encode character u'\u2582' in position 0:
ordinal not in range(128)

With this branch, the following should instead work as usual:
`LC_ALL=C .venv/bin/python juju-quickstart -i`.

I am not sure how to make Urwid fail so that we can QA
the system exit in the code, but at least that code path
is tested.

https://codereview.appspot.com/90740043/

To post a comment you must log in.
Revision history for this message
Francesco Banconi (frankban) wrote :
Download full text (4.7 KiB)

Reviewers: mp+217067_code.launchpad.net,

Message:
Please take a look.

Description:
Check unicode support.

Ensure utf-8 is supported before starting
the Urwid interactive session.
This way we hope to avoid bad crashes
when the user has locale configuration problems.

Tests: `make check`.

QA:
If you have juju-quickstart installed, you should
be able to make it crash by overriding all the language
env vars, like the following:
`LC_ALL=C juju-quickstart -i`
You should see a UnicodeEncodeError:
'ascii' codec can't encode character u'\u2582' in position 0:
ordinal not in range(128)

With this branch, the following should instead work as usual:
`LC_ALL=C .venv/bin/python juju-quickstart -i`.

I am not sure how to make Urwid fail so that we can QA
the system exit in the code, but at least that code path
is tested.

https://code.launchpad.net/~frankban/juju-quickstart/check-locale/+merge/217067

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/90740043/

Affected files (+57, -0 lines):
   A [revision details]
   M quickstart/cli/base.py
   M quickstart/tests/cli/test_base.py

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision:
<email address hidden>

Index: quickstart/cli/base.py
=== modified file 'quickstart/cli/base.py'
--- quickstart/cli/base.py 2014-01-03 10:22:24 +0000
+++ quickstart/cli/base.py 2014-04-24 14:47:18 +0000
@@ -26,6 +26,7 @@
  from __future__ import unicode_literals

  from collections import namedtuple
+import sys

  import urwid

@@ -44,6 +45,24 @@
  )

+def _check_encoding():
+ """Set the Urwid global byte encoding to utf-8.
+
+ Exit the application if, for some reasons, the change does not have
effect.
+ """
+ urwid.set_encoding('utf-8')
+ if not urwid.supports_unicode():
+ # Note: the following message must only include ASCII characters.
+ msg = (
+ 'Error: your terminal does not seem to support UTF-8
encoding.\n'
+ 'Please check your locale settings.\n'
+ 'On Ubuntu, running the following might fix the problem:\n'
+ ' sudo locale-gen en_US.UTF-8\n'
+ ' sudo dpkg-reconfigure locales'
+ )
+ sys.exit(msg.encode('ascii'))
+
+
  class _MainLoop(urwid.MainLoop):
      """A customized Urwid loop.

@@ -109,6 +128,7 @@
            with the exit shortcut. See the quickstart.cli.views module
docstring
            for more information about this functionality.
      """
+ _check_encoding()
      # Set up the application header.
      title = urwid.Text('\npreparing...')
      header_line = urwid.Divider('\N{LOWER ONE QUARTER BLOCK}')

Index: quickstart/tests/cli/test_base.py
=== modified file 'quickstart/tests/cli/test_base.py'
--- quickstart/tests/cli/test_base.py 2014-01-03 10:22:24 +0000
+++ quickstart/tests/cli/test_base.py 2014-04-24 14:49:16 +0000
@@ -18,14 +18,49 @@

  from __future__ import unicode_literals

+from context...

Read more...

Revision history for this message
Richard Harding (rharding) wrote :
Revision history for this message
Richard Harding (rharding) wrote :
68. By Francesco Banconi

Merge trunk.

Revision history for this message
Francesco Banconi (frankban) wrote :

*** Submitted:

Check unicode support.

Ensure utf-8 is supported before starting
the Urwid interactive session.
This way we hope to avoid bad crashes
when the user has locale configuration problems.

Tests: `make check`.

QA:
If you have juju-quickstart installed, you should
be able to make it crash by overriding all the language
env vars, like the following:
`LC_ALL=C juju-quickstart -i`
You should see a UnicodeEncodeError:
'ascii' codec can't encode character u'\u2582' in position 0:
ordinal not in range(128)

With this branch, the following should instead work as usual:
`LC_ALL=C .venv/bin/python juju-quickstart -i`.

I am not sure how to make Urwid fail so that we can QA
the system exit in the code, but at least that code path
is tested.

R=rharding
CC=
https://codereview.appspot.com/90740043

https://codereview.appspot.com/90740043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches