Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(250)

Issue 6676090: Add code to 'fix' python encoding and it's unit test. (Closed)

Created:
9 years, 9 months ago by M-A Ruel
Modified:
9 years, 7 months ago
Reviewers:
Dirk Pranke
CC:
chromium-reviews, Nicolas Sylvain
Visibility:
Public.

Description

Add code to 'fix' python encoding and it's unit test. With this code in action, I can successfully print arabic and chinese on linux and cygwin. It fails to print chinese on my Mac but prints arabic. It prints '?' on Windows console but it *doesn't throw* which is the biggest improvement here. It was particularly a problem on windows because WindowsError's description text is in the current ANSI code page so it failed to print if the text was not pure ASCII, like error message when using Windows' French UI. R=dpranke@chromium.org BUG=none TEST=manual unit test ./tests/fix_encoding_test.py and it shouldn't throw Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=79006

Patch Set 1 #

Patch Set 2 : Fix a comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+439 lines, -14 lines) Patch
M PRESUBMIT.py View 1 chunk +1 line, -0 lines 0 comments Download
A fix_encoding.py View 1 chunk +353 lines, -0 lines 0 comments Download
M gcl.py View 2 chunks +2 lines, -0 lines 0 comments Download
M gclient.py View 2 chunks +2 lines, -0 lines 0 comments Download
M presubmit_support.py View 3 chunks +13 lines, -11 lines 0 comments Download
A tests/fix_encoding_test.py View 1 1 chunk +60 lines, -0 lines 0 comments Download
M tests/gcl_unittest.py View 1 chunk +2 lines, -1 line 0 comments Download
M tests/presubmit_unittest.py View 1 chunk +2 lines, -1 line 0 comments Download
M tests/trychange_unittest.py View 1 chunk +2 lines, -1 line 0 comments Download
M trychange.py View 2 chunks +2 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
M-A Ruel
9 years, 9 months ago (2011-03-22 17:19:16 UTC) #1
Dirk Pranke
LGTM with one possible change ... should we add this to git-cl as well? Ugh. ...
9 years, 9 months ago (2011-03-22 18:22:22 UTC) #2
M-A Ruel
On 2011/03/22 18:22:22, dpranke wrote: > LGTM with one possible change ... should we add ...
9 years, 9 months ago (2011-03-22 18:24:23 UTC) #3
Dirk Pranke
9 years, 9 months ago (2011-03-22 18:31:10 UTC) #4
Okay.

On Tue, Mar 22, 2011 at 11:24 AM,  <maruel@chromium.org> wrote:
> On 2011/03/22 18:22:22, dpranke wrote:
>>
>> LGTM with one possible change ... should we add this to git-cl as well?
>
> Can you move git-cl up a directory first? :)
>
>> Ugh. What were you doing that caused you to have to do this? Was it
>> running in
>> the French locale?
>
> Yep. But even just printing unicode in bash when the default encoding is set
> to
> 'ascii' would blow up on mac and linux so it's not only a windows-specific
> issue.
>
> M-A
>
> http://codereview.chromium.org/6676090/
>

Powered by Google App Engine
This is Rietveld 408576698