Implement Windows FTP directory listing parser.

Implement Windows FTP directory listing parser.

TEST=Covered by net_unittests.
BUG=25520

[Paweł Hajdan Jr., 2009-11-06 17:43:15]

These parsers are so easy to write. :)

[eroman, 2009-11-06 19:35:11]

LGTM!

http://codereview.chromium.org/374010/diff/1/2
File net/data/ftp/dir-listing-windows-1 (right):

http://codereview.chromium.org/374010/diff/1/2#newcode1
Line 1: 11-02-09  05:32PM       <DIR>          NT
It doesn't look like the new file sets svn:eol-style property.
I reckon these should be CRLF

http://codereview.chromium.org/374010/diff/1/8
File net/ftp/ftp_directory_listing_parsers.cc (right):

http://codereview.chromium.org/374010/diff/1/8#newcode97
Line 97: // Date should be in format MM-DD-YY[YY].
I don't see any tests for the 4 digit year case.

http://codereview.chromium.org/374010/diff/1/8#newcode131
Line 131: else if (am_or_pm != ASCIIToUTF16("AM"))
I have noticed the parser code uses ASCIIToUTF16 in a lot of places to do string
comparison. I would recommend adding some sort of:

 EqualsASCII(const String16& a, const char* b)

to string_util.h, since we already have that sort of function for lowercase
comparisons)

http://codereview.chromium.org/374010/diff/1/10
File net/ftp/ftp_directory_listing_parsers_unittest.cc (right):

http://codereview.chromium.org/374010/diff/1/10#newcode108
Line 108: { "01-06-01  02:42AM                  458 Readme.txt",
nit: I recommend varying the "458" and "Readme.txt" throughout these.

[Paweł Hajdan Jr., 2009-11-06 21:03:48]

On 2009/11/06 19:35:11, eroman wrote:
> LGTM!
> 
> http://codereview.chromium.org/374010/diff/1/2
> File net/data/ftp/dir-listing-windows-1 (right):
> 
> http://codereview.chromium.org/374010/diff/1/2#newcode1
> Line 1: 11-02-09  05:32PM       <DIR>          NT
> It doesn't look like the new file sets svn:eol-style property.
> I reckon these should be CRLF

I don't really know what to do with it (I'm using git). What do you recommend?

> http://codereview.chromium.org/374010/diff/1/8
> File net/ftp/ftp_directory_listing_parsers.cc (right):
> 
> http://codereview.chromium.org/374010/diff/1/8#newcode97
> Line 97: // Date should be in format MM-DD-YY[YY].
> I don't see any tests for the 4 digit year case.

Added.

> http://codereview.chromium.org/374010/diff/1/8#newcode131
> Line 131: else if (am_or_pm != ASCIIToUTF16("AM"))
> I have noticed the parser code uses ASCIIToUTF16 in a lot of places to do
string
> comparison. I would recommend adding some sort of:
> 
>  EqualsASCII(const String16& a, const char* b)

Oh, it was there. I used it where applicable.

> http://codereview.chromium.org/374010/diff/1/10
> File net/ftp/ftp_directory_listing_parsers_unittest.cc (right):
> 
> http://codereview.chromium.org/374010/diff/1/10#newcode108
> Line 108: { "01-06-01  02:42AM                  458 Readme.txt",
> nit: I recommend varying the "458" and "Readme.txt" throughout these.

I added some small variations.

[eroman, 2009-11-06 21:25:11]

LGTM

> I don't really know what to do with it (I'm using git).
> What do you recommend?

You can just commit as is then, and use svn later to update the properties.