mattnenterprise / rust-ftp

FTP client for Rust
Apache License 2.0
182 stars 57 forks source link

RETR and TYPE #37

Closed workanator closed 8 years ago

workanator commented 8 years ago

Hello,

the current implementation of RETR (FtpStream::get) is not correct because it does not read response lines after the data stream is read and closed and using the simple implementation (FtpStream::simple_retr) is not a good idea when files for downloading are big. In the PR I added a new function FtpStream::retr which takes an additional argument reader which is a closure which operates the data stream and inside retr all repsonses and dropping of the data stream are handled correctly.

In the PR I also added TYPE (FtpStream::transfer_type) function which is very important when the binary file should be transfered.

mattnenterprise commented 8 years ago

Looks good. Thanks for the improvement. Do you think it would be ok to just delete FtpStream::get and FtpStream::simple_retr?

workanator commented 8 years ago

Well if we simply delete get and simple_retr that may affect existing code but on the other plate the framework is in the early development state and the API could change any time. My opinion is removing get is the good idea and simple_retr may retain but it should use retr.

Also I have an idea that FtpStream should implement low-level FTP commands RETR, STOR, etc. and the framework can contain high-level FTP implementation, say FtpClient, on top of FtpStream which provides such functions as simple_retr.

mattnenterprise commented 8 years ago

I like the idea because lower level control is there if needed, but also the high level to make common operations easier. Do you have any idea what other operations would be in the FtpClient?

workanator commented 8 years ago

Something like download remote file into local file, upload local file into remote dir/file, obtain directory listing.

mattnenterprise commented 8 years ago

Ok. If you have any other ideas for this project just create an issue or pull request.