Author r.david.murray
Recipients agillesp, r.david.murray, ryles
Date 2009-08-26.18:55:42
SpamBayes Score 1.24745e-12
Marked as misclassified No
Message-id <1251312945.41.0.52656749646.issue6759@psf.upfronthosting.co.za>
In-reply-to
Content
Thanks for working on this.

Comments on patch:

(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
possible)?
History
Date User Action Args
2009-08-26 18:55:45r.david.murraysetrecipients: + r.david.murray, ryles, agillesp
2009-08-26 18:55:45r.david.murraysetmessageid: <1251312945.41.0.52656749646.issue6759@psf.upfronthosting.co.za>
2009-08-26 18:55:43r.david.murraylinkissue6759 messages
2009-08-26 18:55:43r.david.murraycreate