Message104838
| Author |
lemburg |
| Recipients |
Arfrever, ezio.melotti, gregory.p.smith, lemburg, loewis, pitrou, vstinner |
| Date |
2010年05月03日.13:55:11 |
| SpamBayes Score |
0.00051798136 |
| Marked as misclassified |
No |
| Message-id |
<4BDED5BD.50807@egenix.com> |
| In-reply-to |
<1272877821.14.0.518627744268.issue8603@psf.upfronthosting.co.za> |
| Content |
A view comments on the patch:
+ def __init__(self, data, encodekey, decodekey, encodevalue, decodevalue, putenv, unsetenv):
As general guideline: When adding new parameter, please add them to the
end of the parameter list and preferably with a default argument in order
to not break the API.
Doesn't matter much in this case, since _Environ is only used internally,
but it's still good practice.
--
+data = {}
+for key, value in environ.items():
+ data[_keymap(key)] = fsencode(value)
Please put such init code into a function or make sure that the global
module space is not polluted with temporary variables such as data,
key, value.
--
+ # bytes environ
+ environb = _Environ(data, _keymap, fsencode, fsencode, fsencode, _putenv, _unsetenv)
This looks wrong even though it will work, but that's only a
side-effect of how fsencode is coded and that's not how the
stdlib should be coded (see below).
--
+ def fsencode(value):
+ """
+ unicode to file system
+ """
+ if isinstance(value, bytes):
+ return value
+ else:
+ return value.encode(sys.getfilesystemencoding(), 'surrogateescape')
The function should not accept bytes as input or at least
document this pass-through behavior, leaving the user to decide
whether that's wanted or not.
--
The patch is also missing code which keeps the two dictionaries in
sync. If os.environ changes, os.environb would have to change as
well. |
|