[issue8603] Create a bytes version of os.environ and getenvb()
Marc-Andre Lemburg
report at bugs.python.org
Mon May 3 15:55:14 CEST 2010
Marc-Andre Lemburg <mal at egenix.com> added the comment:
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.
----------
_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue8603>
_______________________________________
More information about the Python-bugs-list
mailing list