Code cleanup (commenting the why)

colours.rs

@@ -1,13 +1,23 @@
 pub enum Colour {
+    // These are the standard numeric sequences.
+    // See http://invisible-island.net/xterm/ctlseqs/ctlseqs.html
     Black = 30, Red = 31, Green = 32, Yellow = 33, Blue = 34, Purple = 35, Cyan = 36, White = 37,
 }
 
+// There are only three different styles: plain (no formatting), only
+// a foreground colour, and a catch-all for anything more complicated
+// than that. It's technically possible to write other cases such as
+// "bold foreground", but probably isn't worth writing all the code.
+
 pub enum Style {
     Plain,
     Foreground(Colour),
     Style(StyleStruct),
 }
 
+// Having a struct inside an enum is currently unfinished in Rust, but
+// should be put in there when that feature is complete.
+
 pub struct StyleStruct {
     foreground: Colour,
     background: Option<Colour>,
@@ -28,8 +38,8 @@ impl Style {
                     };
                     let bo = if bold { "1;" } else { "" };
                     let un = if underline { "4;" } else { "" };
-                    let re = format!("\x1B[{}{}{}{}m{}\x1B[0m", bo, un, bg, foreground as int, input.to_strbuf());
-                    return re.to_owned();
+                    let painted = format!("\x1B[{}{}{}{}m{}\x1B[0m", bo, un, bg, foreground as int, input.to_strbuf());
+                    return painted.to_owned();
                 }
             }
         }
@@ -39,30 +49,33 @@ impl Style {
 impl Style {
     pub fn bold(&self) -> Style {
       match *self {
-        Plain => Style(StyleStruct { foreground: White, background: None, bold: true, underline: false }),
-        Foreground(c) => Style(StyleStruct { foreground: c, background: None, bold: true, underline: false }),
-        Style(st) => Style(StyleStruct { foreground: st.foreground, background: st.background, bold: true, underline: false }),
+        Plain => Style(StyleStruct         { foreground: White,         background: None,          bold: true, underline: false }),
+        Foreground(c) => Style(StyleStruct { foreground: c,             background: None,          bold: true, underline: false }),
+        Style(st) => Style(StyleStruct     { foreground: st.foreground, background: st.background, bold: true, underline: false }),
       }
     }
 
     pub fn underline(&self) -> Style {
       match *self {
-        Plain => Style(StyleStruct { foreground: White, background: None, bold: false, underline: true }),
-        Foreground(c) => Style(StyleStruct { foreground: c, background: None, bold: false, underline: true }),
-        Style(st) => Style(StyleStruct { foreground: st.foreground, background: st.background, bold: false, underline: true }),
+        Plain => Style(StyleStruct         { foreground: White,         background: None,          bold: false, underline: true }),
+        Foreground(c) => Style(StyleStruct { foreground: c,             background: None,          bold: false, underline: true }),
+        Style(st) => Style(StyleStruct     { foreground: st.foreground, background: st.background, bold: false, underline: true }),
       }
     }
 
     pub fn on(&self, background: Colour) -> Style {
       match *self {
-        Plain => Style(StyleStruct { foreground: White, background: Some(background), bold: false, underline: false }),
-        Foreground(c) => Style(StyleStruct { foreground: c, background: Some(background), bold: false, underline: false }),
-        Style(st) => Style(StyleStruct { foreground: st.foreground, background: Some(background), bold: false, underline: false }),
+        Plain => Style(StyleStruct         { foreground: White,         background: Some(background), bold: false, underline: false }),
+        Foreground(c) => Style(StyleStruct { foreground: c,             background: Some(background), bold: false, underline: false }),
+        Style(st) => Style(StyleStruct     { foreground: st.foreground, background: Some(background), bold: false, underline: false }),
       }
     }
 }
 
 impl Colour {
+
+    // This is a short-cut so you don't have to use Blue.normal() just
+    // to turn Blue into a Style.
     pub fn paint(&self, input: &str) -> String {
         let re = format!("\x1B[{}m{}\x1B[0m", *self as int, input);
         return re.to_owned();

exa.rs

@@ -16,15 +16,16 @@ pub mod unix;
 pub mod options;
 
 fn main() {
-    let args = os::args().iter()
-        .map(|x| x.to_strbuf())
-        .collect();
+    let args = os::args();
     
     match Options::getopts(args) {
         Err(err) => println!("Invalid options:\n{}", err.to_err_msg()),
         Ok(opts) => {
+
+            // Default to listing the current directory when a target
+            // isn't specified (mimic the behaviour of ls)
             let strs = if opts.dirs.is_empty() {
-                vec!("./".to_strbuf())
+                vec!(".".to_strbuf())
             }
             else {
                 opts.dirs.clone()
@@ -46,21 +47,33 @@ fn exa(options: &Options, path: Path) {
     let unordered_files: Vec<File> = paths.iter().map(|path| File::from_path(path)).collect();
     let files: Vec<&File> = options.transform_files(&unordered_files);
 
+    // The output gets formatted into columns, which looks nicer. To
+    // do this, we have to write the results into a table, instead of
+    // displaying each file immediately, then calculating the maximum
+    // width of each column based on the length of the results and
+    // padding the fields during output.
+
     let table: Vec<Vec<String>> = files.iter()
         .map(|f| options.columns.iter().map(|c| f.display(c)).collect())
         .collect();
 
+    // Each column needs to have its invisible colour-formatting
+    // characters stripped before it has its width calculated, or the
+    // width will be incorrect and the columns won't line up properly.
+    // This is fairly expensive to do (it uses a regex), so the
+    // results are cached.
+
     let lengths: Vec<Vec<uint>> = table.iter()
         .map(|row| row.iter().map(|col| colours::strip_formatting(col).len()).collect())
         .collect();
 
-    let maxes: Vec<uint> = range(0, options.columns.len())
+    let column_widths: Vec<uint> = range(0, options.columns.len())
         .map(|n| lengths.iter().map(|row| *row.get(n)).max().unwrap())
         .collect();
 
     for (field_lengths, row) in lengths.iter().zip(table.iter()) {
         let mut first = true;
-        for ((column_length, cell), field_length) in maxes.iter().zip(row.iter()).zip(field_lengths.iter()) {
+        for ((column_length, cell), field_length) in column_widths.iter().zip(row.iter()).zip(field_lengths.iter()) {
             if first {
                 first = false;
             } else {

file.rs

@@ -3,7 +3,7 @@ use std::io;
 
 use colours::{Plain, Style, Black, Red, Green, Yellow, Blue, Purple, Cyan};
 use column::{Column, Permissions, FileName, FileSize, User, Group};
-use format::{formatBinaryBytes, formatDecimalBytes};
+use format::{format_metric_bytes, format_IEC_bytes};
 use unix::{get_user_name, get_group_name};
 
 static MEDIA_TYPES: &'static [&'static str] = &[
@@ -15,9 +15,13 @@ static COMPRESSED_TYPES: &'static [&'static str] = &[
     "zip", "tar", "Z", "gz", "bz2", "a", "ar", "7z",
     "iso", "dmg", "tc", "rar", "par" ];
 
-// Each file is definitely going to get `stat`ted at least once, if
-// only to determine what kind of file it is, so carry the `stat`
-// result around with the file for safe keeping.
+// Instead of working with Rust's Paths, we have our own File object
+// that holds the Path and various cached information. Each file is
+// definitely going to have its filename used at least once, its stat
+// information queried at least once, and its file extension extracted
+// at least once, so we may as well carry around that information with
+// the actual path.
+
 pub struct File<'a> {
     pub name: &'a str,
     pub ext:  Option<&'a str>,
@@ -27,12 +31,13 @@ pub struct File<'a> {
 
 impl<'a> File<'a> {
     pub fn from_path(path: &'a Path) -> File<'a> {
+        // Getting the string from a filename fails whenever it's not
+        // UTF-8 representable - just assume it is for now.
         let filename: &str = path.filename_str().unwrap();
 
-        // We have to use lstat here instad of file.stat(), as it
-        // doesn't follow symbolic links. Otherwise, the stat() call
-        // will fail if it encounters a link that's target is
-        // non-existent.
+        // Use lstat here instead of file.stat(), as it doesn't follow
+        // symbolic links. Otherwise, the stat() call will fail if it
+        // encounters a link that's target is non-existent.
         let stat: io::FileStat = match fs::lstat(path) {
             Ok(stat) => stat,
             Err(e) => fail!("Couldn't stat {}: {}", filename, e),
@@ -47,7 +52,10 @@ impl<'a> File<'a> {
     }
 
     fn ext(name: &'a str) -> Option<&'a str> {
-        let re = regex!(r"\.(.+)$");
+        // The extension is the series of characters after a dot at
+        // the end of a filename. This deliberately also counts
+        // dotfiles - the ".git" folder has the extension "git".
+        let re = regex!(r"\.([^.]+)$");
         re.captures(name).map(|caps| caps.at(1))
     }
 
@@ -57,38 +65,41 @@ impl<'a> File<'a> {
 
     pub fn display(&self, column: &Column) -> String {
         match *column {
-            Permissions => self.permissions(),
+            Permissions => self.permissions_string(),
             FileName => self.file_colour().paint(self.name.as_slice()),
-            FileSize(si) => self.file_size(si),
+            FileSize(use_iec) => self.file_size(use_iec),
+
+            // Display the ID if the user/group doesn't exist, which
+            // usually means it was deleted but its files weren't.
             User => get_user_name(self.stat.unstable.uid as i32).unwrap_or(self.stat.unstable.uid.to_str()),
             Group => get_group_name(self.stat.unstable.gid as u32).unwrap_or(self.stat.unstable.gid.to_str()),
         }
     }
 
-    fn file_size(&self, si: bool) -> String {
+    fn file_size(&self, use_iec_prefixes: bool) -> String {
         // Don't report file sizes for directories. I've never looked
         // at one of those numbers and gained any information from it.
         if self.stat.kind == io::TypeDirectory {
             Black.bold().paint("---")
         } else {
-            let sizeStr = if si {
-                formatBinaryBytes(self.stat.size)
+            let size_str = if use_iec_prefixes {
+                format_IEC_bytes(self.stat.size)
             } else {
-                formatDecimalBytes(self.stat.size)
+                format_metric_bytes(self.stat.size)
             };
 
-            return Green.bold().paint(sizeStr.as_slice());
+            return Green.bold().paint(size_str.as_slice());
         }
     }
 
     fn type_char(&self) -> String {
         return match self.stat.kind {
-            io::TypeFile => ".".to_strbuf(),
-            io::TypeDirectory => Blue.paint("d"),
-            io::TypeNamedPipe => Yellow.paint("|"),
+            io::TypeFile         => ".".to_strbuf(),
+            io::TypeDirectory    => Blue.paint("d"),
+            io::TypeNamedPipe    => Yellow.paint("|"),
             io::TypeBlockSpecial => Purple.paint("s"),
-            io::TypeSymlink => Cyan.paint("l"),
-            _ => "?".to_owned(),
+            io::TypeSymlink      => Cyan.paint("l"),
+            _                    => "?".to_owned(),
         }
     }
 
@@ -116,38 +127,30 @@ impl<'a> File<'a> {
         }
     }
 
-    fn permissions(&self) -> String {
+    fn permissions_string(&self) -> String {
         let bits = self.stat.perm;
         return format!("{}{}{}{}{}{}{}{}{}{}",
             self.type_char(),
-            File::bit(bits, io::UserRead, "r", Yellow.bold()),
-            File::bit(bits, io::UserWrite, "w", Red.bold()),
-            File::bit(bits, io::UserExecute, "x", Green.bold().underline()),
-            File::bit(bits, io::GroupRead, "r", Yellow.normal()),
-            File::bit(bits, io::GroupWrite, "w", Red.normal()),
-            File::bit(bits, io::GroupExecute, "x", Green.normal()),
-            File::bit(bits, io::OtherRead, "r", Yellow.normal()),
-            File::bit(bits, io::OtherWrite, "w", Red.normal()),
-            File::bit(bits, io::OtherExecute, "x", Green.normal()),
+
+            // The first three are bold because they're the ones used
+            // most often.
+            File::permission_bit(bits, io::UserRead,     "r", Yellow.bold()),
+            File::permission_bit(bits, io::UserWrite,    "w", Red.bold()),
+            File::permission_bit(bits, io::UserExecute,  "x", Green.bold().underline()),
+            File::permission_bit(bits, io::GroupRead,    "r", Yellow.normal()),
+            File::permission_bit(bits, io::GroupWrite,   "w", Red.normal()),
+            File::permission_bit(bits, io::GroupExecute, "x", Green.normal()),
+            File::permission_bit(bits, io::OtherRead,    "r", Yellow.normal()),
+            File::permission_bit(bits, io::OtherWrite,   "w", Red.normal()),
+            File::permission_bit(bits, io::OtherExecute, "x", Green.normal()),
        );
     }
 
-    fn bit(bits: io::FilePermission, bit: io::FilePermission, other: &'static str, style: Style) -> String {
+    fn permission_bit(bits: io::FilePermission, bit: io::FilePermission, character: &'static str, style: Style) -> String {
         if bits.contains(bit) {
-            style.paint(other.as_slice())
+            style.paint(character.as_slice())
         } else {
             Black.bold().paint("-".as_slice())
         }
     }
 }
-
-impl<'a> Clone for File<'a> {
-    fn clone(&self) -> File<'a> {
-        return File {
-            path: self.path,
-            stat: self.stat,
-            name: self.name.clone(),
-            ext:  self.ext.clone(),
-        };
-    }
-}

format.rs

@@ -15,10 +15,10 @@ fn formatBytes(mut amount: u64, kilo: u64, prefixes: &[&str]) -> String {
     format!("{}{}", amount, prefixes[prefix])
 }
 
-pub fn formatBinaryBytes(amount: u64) -> String {
+pub fn format_IEC_bytes(amount: u64) -> String {
     formatBytes(amount, 1024, IEC_PREFIXES)
 }
 
-pub fn formatDecimalBytes(amount: u64) -> String {
+pub fn format_metric_bytes(amount: u64) -> String {
     formatBytes(amount, 1000, METRIC_PREFIXES)
 }

options.rs

@@ -21,8 +21,8 @@ impl SortField {
         match word.as_slice() {
             "name" => Name,
             "size" => Size,
-            "ext" => Extension,
-            _ => fail!("Invalid sorting order"),
+            "ext"  => Extension,
+            _      => fail!("Invalid sorting order"),
         }
     }
 }
@@ -65,7 +65,7 @@ impl Options {
         return columns;
     }
 
-    fn show(&self, f: &File) -> bool {
+    fn should_display(&self, f: &File) -> bool {
         if self.showInvisibles {
             true
         } else {
@@ -75,7 +75,7 @@ impl Options {
 
     pub fn transform_files<'a>(&self, unordered_files: &'a Vec<File<'a>>) -> Vec<&'a File<'a>> {
         let mut files: Vec<&'a File<'a>> = unordered_files.iter()
-            .filter(|&f| self.show(f))
+            .filter(|&f| self.should_display(f))
             .collect();
 
         match self.sortField {