Add doc and comments

3 files changed, 55 insertions, 25 deletions

diff --git a/src/archive.rs b/src/archive.rs
index ca22d28..f76d55c 100644
--- a/src/archive.rs
+++ b/src/archive.rs
@@ -43,6 +43,11 @@ impl CompressionMethod {
         }
     }
 
+    /// Make an archive out of the given directory, and write the output to the given writer.
+    ///
+    /// Recursively includes all files and subdirectories.
+    ///
+    /// If `skip_symlinks` is `true`, symlinks fill not be followed and will just be ignored.
     pub fn create_archive<T, W>(
         self,
         dir: T,
@@ -61,6 +66,7 @@ impl CompressionMethod {
     }
 }
 
+/// Write a gzipped tarball of `dir` in `out`.
 fn tar_gz<W>(dir: &Path, skip_symlinks: bool, out: W) -> Result<(), ContextualError>
 where
     W: std::io::Write,
@@ -76,29 +82,31 @@ where
     Ok(())
 }
 
+/// Write a tarball of `dir` in `out`.
+///
+/// The target directory will be saved as a top-level directory in the archive.
+/// For example, if given `"a/b/c"`, it will be stored as just `"c"` in the archive.
 fn tar_dir<W>(dir: &Path, skip_symlinks: bool, out: W) -> Result<(), ContextualError>
 where
     W: std::io::Write,
 {
-    if let Some(inner_folder) = dir.file_name() {
-        if let Some(directory) = inner_folder.to_str() {
-            tar(dir, directory.to_string(), skip_symlinks, out).map_err(|e| {
-                ContextualError::ArchiveCreationError("tarball".to_string(), Box::new(e))
-            })
-        } else {
-            // https://doc.rust-lang.org/std/ffi/struct.OsStr.html#method.to_str
-            Err(ContextualError::InvalidPathError(
-                "Directory name contains invalid UTF-8 characters".to_string(),
-            ))
-        }
-    } else {
-        // https://doc.rust-lang.org/std/path/struct.Path.html#method.file_name
-        Err(ContextualError::InvalidPathError(
-            "Directory name terminates in \"..\"".to_string(),
-        ))
-    }
+    let inner_folder = dir.file_name().ok_or_else(|| {
+        ContextualError::InvalidPathError("Directory name terminates in \"..\"".to_string())
+    })?;
+
+    let directory = inner_folder.to_str().ok_or_else(|| {
+        ContextualError::InvalidPathError(
+            "Directory name contains invalid UTF-8 characters".to_string(),
+        )
+    })?;
+
+    tar(dir, directory.to_string(), skip_symlinks, out)
+        .map_err(|e| ContextualError::ArchiveCreationError("tarball".to_string(), Box::new(e)))
 }
 
+/// Writes a tarball of `dir` in `out`.
+///
+/// The content of `src_dir` will be saved in the archive as a folder named `inner_folder`.
 fn tar<W>(
     src_dir: &Path,
     inner_folder: String,
diff --git a/src/listing.rs b/src/listing.rs
index ba2e58e..ee9c581 100644
--- a/src/listing.rs
+++ b/src/listing.rs
@@ -1,6 +1,5 @@
 use actix_web::{fs, Body, FromRequest, HttpRequest, HttpResponse, Query, Result};
 use bytesize::ByteSize;
-use failure::Fail;
 use futures::Stream;
 use htmlescape::encode_minimal as escape_html_entity;
 use percent_encoding::{utf8_percent_encode, DEFAULT_ENCODE_SET};
@@ -248,7 +247,8 @@ pub fn directory_listing<S>(
             compression_method.extension()
         );
 
-        // Create a pipe to connect the archive creation thread and the response.
+        // We will create the archive in a separate thread, and stream the content using a pipe.
+        // The pipe is made of a futures channel, and an adapter to implement the `Write` trait.
         // Include 10 messages of buffer for erratic connection speeds.
         let (tx, rx) = futures::sync::mpsc::channel(10);
         let pipe = crate::pipe::Pipe::new(tx);
@@ -261,10 +261,19 @@ pub fn directory_listing<S>(
             }
         });
 
-        // `<rx as Stream>::Error == ()` but we want `actix_web::error::Error`
-        // It can't happen, so let's just please the type checker.
+        // `rx` is a receiver of bytes - it can act like a `Stream` of bytes, and that's exactly
+        // what actix-web wants to stream the response.
+        //
+        // But right now the error types do not match:
+        // `<rx as Stream>::Error == ()`, but we want `actix_web::error::Error`
+        //
+        // That being said, `rx` will never fail because the `Stream` implementation for `Receiver`
+        // never returns an error - it simply cannot fail.
         let rx = rx.map_err(|_| unreachable!("pipes never fail"));
 
+        // At this point, `rx` implements everything actix want for a streamed response,
+        // so we can just give a `Box::new(rx)` as streaming body.
+
         Ok(HttpResponse::Ok()
             .content_type(compression_method.content_type())
             .content_encoding(compression_method.content_encoding())
diff --git a/src/pipe.rs b/src/pipe.rs
index 710be1f..2fd9fac 100644
--- a/src/pipe.rs
+++ b/src/pipe.rs
@@ -3,12 +3,18 @@ use futures::sink::{Sink, Wait};
 use futures::sync::mpsc::Sender;
 use std::io::{Error, ErrorKind, Result, Write};
 
+/// Adapter to implement the `std::io::Write` trait on a `Sender<Bytes>` from a futures channel.
+///
+/// It uses an intermediate buffer to transfer packets.
 pub struct Pipe {
+    // Wrapping the sender in `Wait` makes it blocking, so we can implement blocking-style
+    // io::Write over the async-style Sender.
     dest: Wait<Sender<Bytes>>,
     bytes: BytesMut,
 }
 
 impl Pipe {
+    /// Wrap the given sender in a `Pipe`.
     pub fn new(destination: Sender<Bytes>) -> Self {
         Pipe {
             dest: destination.wait(),
@@ -19,17 +25,24 @@ impl Pipe {
 
 impl Drop for Pipe {
     fn drop(&mut self) {
+        // This is the correct thing to do, but is not super important since the `Sink`
+        // implementation of `Sender` just returns `Ok` without doing anything else.
         let _ = self.dest.close();
     }
 }
 
 impl Write for Pipe {
     fn write(&mut self, buf: &[u8]) -> Result<usize> {
+        // We are given a slice of bytes we do not own, so we must start by copying it.
         self.bytes.extend_from_slice(buf);
-        match self.dest.send(self.bytes.take().into()) {
-            Ok(_) => Ok(buf.len()),
-            Err(e) => Err(Error::new(ErrorKind::UnexpectedEof, e)),
-        }
+
+        // Then, take the buffer and send it in the channel.
+        self.dest
+            .send(self.bytes.take().into())
+            .map_err(|e| Error::new(ErrorKind::UnexpectedEof, e))?;
+
+        // Return how much we sent - all of it.
+        Ok(buf.len())
     }
 
     fn flush(&mut self) -> Result<()> {