Eric Smith <e...@trueblade.com> added the comment:
I agree that the callback isn't needed, and it reflects the older coding style
of much of the library (such as in retrlines). Instead, I'd make this a
generator, yielding each of the dicts. (Actually in some ideal rewrite of
ftplib, the whole callback "feature" used by retrlines would go away except as
a compatibility feature, and internally everything would use generators instead
of callbacks.)
Aren't you modifying the state on the server (via "OPTS MLST"), and then if you
make a subsequent call without specifying "facts" you'll be using the value of
"facts" from the previous call to MLSD? I don't think that's desirable, but
short of calling "OPTS MLST" every time (possibly with the results of an
initial FEAT) there's no way around it.
Just today I saw a filename with "; " in the name. I think you want to use
.partition(' ') isolate the facts from the filename, and add a test with such a
filename.
I don't like the "isdigit" test. Shouldn't this decision be left to the caller?
What if a fact happens to look like an integer some of the time, but not
always? Maybe "unique" is a hex string. I don't think you'd want it converted
to an int on the occasions where it was all decimal digits, but a string
otherwise. Also look at your "modify" facts. Those are not very useful as ints.
I don't think you should invent a pseudo-fact "name", in case the standard ever
uses that. I'd prefer if you yielded tuples of (filename, fact-dict).
MLSD_DATA has newlines in it, so you get "\r\n\n" sequences in your test data.
If a fact=value string does not have an '=', you silently ignore it. I'd rather
this raise an exception and not pass silently. It's not compliant. You should
have a test for this.
It's possible for a value to have an '=' in it, so you don't want to use
.split('='), you should use .partition('='). You should add such a fact to the
test data.
Thanks for working on this. It will be a great addition to ftplib.
----------
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue11072>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
