$ echo -ne "This is an example\r\nWhich demonstrates a problem\r\nwith
ZipFile.open(..., 'U')\r\n" > foo.txt
$ cat -v foo.txt
This is an example^M
Which demonstrates a problem^M
with ZipFile.open(..., 'U')^M
$ zip foo.zip foo.txt
adding: foo.txt (deflated 1%)
Python 2.6.2 (r262:71600, Aug 21 2009, 17:52:12)
[GCC 4.1.2 20071124 (Red Hat 4.1.2-42)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> open("foo.txt", 'U').read()
"This is an example\nWhich demonstrates a problem\nwith
>>> from zipfile import ZipFile
>>> ZipFile("foo.zip").open("foo.txt", 'U').read()
"This is an example\r\nWhich demonstrates a problem\r\nwith
Thanks for working on this. I've taken a look at the patch.
The fix to read_test looks correct. Of course, I would consider a more
descriptive variable name than 'b'.
The changes to read() are an improvement, but I think we need to be
careful when we replace "\r\n" with "\n". Basically, we've turned two
characters into one and are now potentially one character short of
'size' bytes. This doesn't match the behavior of file.read().
Another thing to work out is the lack of the 'newlines' attribute,
discussed in PEP 278.
I've noted that bz2 seems to do a pretty good job with universal newline
It's in C, however, and I don't think there's actually anything in the
library doing UL in pure Python.
(1) I think you should retain the full "ugly check" comment explaining
how the \r\n spanning a buffer is handled. I think that's a helpful
explanation for a non-obvious piece of code.
(2) Your code for setting the newlines attributes and the tests for it
look wrong to me. The attribute is supposed to list all separators
seen. I think your code is going to set it to ('\r\n', '\r') when the
separators are just '\r\n', and your test doesn't check to make sure
newlines gets set to _only_ what was seen. (Nor does it check for the
case of actual mixed line endings...a test for which would require some
additional test hackery since the existing test code didn't try to test
mixed line end files either.)
(3) Sorry for being dense, but I don't understand your code to adjust
the number of bytes in the read method or the test that tests it. I
haven't tried to play with it, but it looks odd to me that you'd be
adding '2', since I would think that the read could cover more than one
line...or am I misunderstanding how 'read(n)' works (which is entirely
* Fixed the code that populates the newlines attribute. I think I've
covered all the cases...
* Found another deviation from file object behavior in this module:
Calling read with a negative size parameter does not always return the
remainder of the file as described in
http://docs.python.org/library/stdtypes.html#file.read I went ahead and
fixed this--please let me know if I should open a separate issue and
submit a separate patch.
* Added more tests for mixed eol files, calling read with a negative
size parameter, reading a file with only crlfs
@Art: @David: A lot of changes have been made to test_zipfile since the last patch was produced. I'm unsure as to whether the patch could be applied to 2.7 as is, or it would be simpler to rework the patch for 2.7 and the py3k branches. Could one of you take a look please, thanks.
Well, you could try applying the patch and see if it applies cleanly and passes the tests. But it would be helpful to have it reworked for py3k, since we now commit first to py3k and then backport to 2.7.
Support lines in zipfile looks contradictory and buggy. This complicates the code and makes the behavior of zipfile.ZipExtFile incompatible with the interface of other file-like objects. For example, the behavior of the read, read1 and peek differ from one described in io module.
If we are working with binary data, conversion of newlines is meaningless (and how about newlines in comments?). If we are working with text, the bytes must be decoded to str. This will help io.TextIOWrapper.
I suggest two alternatives:
1. Deprecate universal newline support in zipfile. zipfile.ZipExtFile should always work with non-modified bytes, and who need the text, let wraps zipfile.ZipExtFile with io.TextIOWrapper.
2. Automatically wrap a zipfile.ZipExtFile with io.TextIOWrapper if universal newlines mode is enabled. In this case, the data type will change from bytes to str. Add modes "t" and "b" to explicitly specify the data type. Add an encoding parameter (and other parameters if needed).
I know how to remove universal newline support, I know how after this
correct these functions (with issue14371 they partially corrected), but
I don't know how to deprecate universal newline support. What should be
done? Can you initiate a discussion in Python-Ideas or Python-Dev? Now
only zipfile trying to implement this PEP (and failed), io ignores 'U'
mode, gzip checks and raises.
With regard to this issue, I note that there are tests that check that
read() returns unmodified data (UniversalNewlineTests.read_test in
Lib/test/test_zipfile.py). It looks weird, but apparently, this behavior
is expected and deliberate.