Author lemburg
Recipients Arfrever, ezio.melotti, gregory.p.smith, lemburg, loewis, pitrou, vstinner
Date 2010-05-03.13:55:11
SpamBayes Score 0.000517981
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.
History
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