This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

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 <>
In-reply-to <>
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
Date User Action Args
2010-05-03 13:55:15lemburgsetrecipients: + lemburg, loewis, gregory.p.smith, pitrou, vstinner, ezio.melotti, Arfrever
2010-05-03 13:55:12lemburglinkissue8603 messages
2010-05-03 13:55:11lemburgcreate