Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(74)

Issue 1874048: Rewrite Python import machinery to use unicode

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 6 months ago by haypo
Modified:
9 years, 6 months ago
Visibility:
Public.

Description

Related Python issue: http://bugs.python.org/issue9425 Read also http://mail.python.org/pipermail/python-dev/2010-July/101619.html

Patch Set 1 #

Total comments: 80
Unified diffs Side-by-side diffs Delta from patch set Stats (+1404 lines, -748 lines) Patch
M Doc/library/sys.rst View 1 chunk +3 lines, -3 lines 0 comments Download
M Include/Python.h View 1 chunk +3 lines, -1 line 2 comments Download
M Include/fileobject.h View 1 chunk +18 lines, -2 lines 0 comments Download
M Include/import.h View 2 chunks +15 lines, -6 lines 2 comments Download
M Include/moduleobject.h View 1 chunk +1 line, -0 lines 0 comments Download
M Include/sysmodule.h View 1 chunk +3 lines, -2 lines 0 comments Download
M Include/warnings.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Lib/distutils/file_util.py View 1 chunk +1 line, -1 line 5 comments Download
M Lib/platform.py View 5 chunks +26 lines, -24 lines 2 comments Download
M Lib/test/test_import.py View 1 chunk +7 lines, -0 lines 1 comment Download
M Lib/test/test_sax.py View 1 chunk +5 lines, -0 lines 0 comments Download
M Lib/test/test_subprocess.py View 3 chunks +7 lines, -7 lines 5 comments Download
M Lib/test/test_sys.py View 1 chunk +3 lines, -2 lines 3 comments Download
M Lib/test/test_urllib.py View 1 chunk +6 lines, -2 lines 0 comments Download
M Lib/test/test_urllib2.py View 2 chunks +5 lines, -0 lines 0 comments Download
M Lib/test/test_xml_etree.py View 2 chunks +6 lines, -0 lines 2 comments Download
M Modules/getpath.c View 7 chunks +109 lines, -100 lines 3 comments Download
M Modules/main.c View 6 chunks +76 lines, -23 lines 8 comments Download
M Modules/zipimport.c View 23 chunks +122 lines, -80 lines 3 comments Download
M Objects/codeobject.c View 3 chunks +17 lines, -0 lines 4 comments Download
M Objects/fileobject.c View 2 chunks +22 lines, -10 lines 1 comment Download
M Objects/moduleobject.c View 5 chunks +11 lines, -14 lines 3 comments Download
M Objects/object.c View 2 chunks +6 lines, -0 lines 2 comments Download
M Objects/typeobject.c View 1 chunk +5 lines, -7 lines 0 comments Download
M Objects/unicodeobject.c View 1 chunk +6 lines, -5 lines 0 comments Download
M PC/import_nt.c View 3 chunks +15 lines, -3 lines 4 comments Download
M Parser/tokenizer.c View 1 chunk +8 lines, -4 lines 3 comments Download
M Python/_warnings.c View 3 chunks +50 lines, -19 lines 0 comments Download
M Python/ast.c View 2 chunks +14 lines, -2 lines 2 comments Download
M Python/bltinmodule.c View 5 chunks +19 lines, -5 lines 0 comments Download
M Python/ceval.c View 2 chunks +0 lines, -7 lines 2 comments Download
M Python/compile.c View 2 chunks +12 lines, -2 lines 0 comments Download
M Python/errors.c View 1 chunk +1 line, -1 line 2 comments Download
M Python/import.c View 81 chunks +571 lines, -387 lines 16 comments Download
M Python/importdl.h View 1 chunk +1 line, -1 line 0 comments Download
M Python/importdl.c View 3 chunks +17 lines, -10 lines 0 comments Download
M Python/pythonrun.c View 10 chunks +161 lines, -8 lines 2 comments Download
M Python/sysmodule.c View 4 chunks +50 lines, -10 lines 3 comments Download

Messages

Total messages: 9
Ezio Melotti
http://codereview.appspot.com/1874048/diff/1/11 File Lib/test/test_import.py (right): http://codereview.appspot.com/1874048/diff/1/11#newcode298 Lib/test/test_import.py:298: pass This "else" seems unnecessary. http://codereview.appspot.com/1874048/diff/1/13 File Lib/test/test_subprocess.py (right): ...
9 years, 6 months ago (2010-07-30 00:38:52 UTC) #1
haypo
http://codereview.appspot.com/1874048/diff/1/20 File Modules/zipimport.c (right): http://codereview.appspot.com/1874048/diff/1/20#newcode82 Modules/zipimport.c:82: path = PyBytes_AsString(pathbytes); On 2010/07/30 00:38:52, Ezio Melotti wrote: ...
9 years, 6 months ago (2010-07-30 03:45:40 UTC) #2
Antoine Pitrou
There seem to be a lot of FIXMEs waiting for resolution ;) http://codereview.appspot.com/1874048/diff/1/3 File Include/Python.h ...
9 years, 6 months ago (2010-07-30 13:35:47 UTC) #3
haypo
Answer to Antoine's comments http://codereview.appspot.com/1874048/diff/1/3 File Include/Python.h (right): http://codereview.appspot.com/1874048/diff/1/3#newcode131 Include/Python.h:131: /* _Py_char2wchar and _Py_wchar2char lives ...
9 years, 6 months ago (2010-07-30 20:47:04 UTC) #4
haypo
http://codereview.appspot.com/1874048/diff/1/35 File Python/import.c (right): http://codereview.appspot.com/1874048/diff/1/35#newcode1454 Python/import.c:1454: && file[len-3] == '.' > `file[-4:-1] != ".py"` > ...
9 years, 6 months ago (2010-07-30 21:06:33 UTC) #5
amaury
The remaining XXX or FIXME should be addressed. And new public functions should be documented. ...
9 years, 6 months ago (2010-07-31 11:33:01 UTC) #6
haypo
Answer to Amaury's comments. http://codereview.appspot.com/1874048/diff/1/17 File Lib/test/test_xml_etree.py (right): http://codereview.appspot.com/1874048/diff/1/17#newcode27 Lib/test/test_xml_etree.py:27: # ignore all tests if ...
9 years, 6 months ago (2010-07-31 21:40:52 UTC) #7
merwok
http://codereview.appspot.com/1874048/diff/1/9 File Lib/distutils/file_util.py (right): http://codereview.appspot.com/1874048/diff/1/9#newcode228 Lib/distutils/file_util.py:228: f = open(filename, "w", encoding="utf-8", errors="surrogateescape") distutils is feature-frozen, ...
9 years, 6 months ago (2010-07-31 22:15:40 UTC) #8
merwok
9 years, 6 months ago (2010-08-10 21:40:30 UTC) #9
http://codereview.appspot.com/1874048/diff/1/9
File Lib/distutils/file_util.py (right):

http://codereview.appspot.com/1874048/diff/1/9#newcode228
Lib/distutils/file_util.py:228: f = open(filename, "w", encoding="utf-8",
errors="surrogateescape")
On 2010/07/30 13:35:47, Antoine Pitrou wrote:
> Why the hardcoding to utf-8?

Distutils writes files in UTF-8; it’s undocumented I think. See #9561 opened by
Haypo today.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b