Merge branch 'more-file-name-fields'

ogham · ogham · commit e819dd95e97e · 2017-05-07T15:31:55.000+01:00
diff --git a/src/fs/file.rs b/src/fs/file.rs
@@ -407,11 +407,24 @@ pub enum FileTarget<'dir> {
     Broken(PathBuf),
 
     /// There was an IO error when following the link. This can happen if the
-    /// file isn't a link to begin with, but also if, say, we don't have
+    /// file isn’t a link to begin with, but also if, say, we don’t have
     /// permission to follow it.
     Err(IOError),
 }
 
+impl<'dir> FileTarget<'dir> {
+
+    /// Whether this link doesn’t lead to a file, for whatever reason. This
+    /// gets used to determine how to highlight the link in grid views.
+    pub fn is_broken(&self) -> bool {
+        match self {
+            &FileTarget::Ok(_)      => false,
+            &FileTarget::Broken(_)  => true,
+            &FileTarget::Err(_)     => true,
+        }
+    }
+}
+
 
 #[cfg(test)]
 mod test {
diff --git a/src/options/view.rs b/src/options/view.rs
@@ -4,8 +4,9 @@ use getopts;
 
 use output::Colours;
 use output::{Grid, Details, GridDetails, Lines};
-use options::{FileFilter, DirAction, Misfire};
 use output::column::{Columns, TimeTypes, SizeFormat};
+use output::file_name::Classify;
+use options::{FileFilter, DirAction, Misfire};
 use term::dimensions;
 use fs::feature::xattr;
 
@@ -58,7 +59,7 @@ impl View {
                     filter: filter.clone(),
                     xattr: xattr::ENABLED && matches.opt_present("extended"),
                     colours: colours,
-                    classify: matches.opt_present("classify"),
+                    classify: Classify::deduce(matches),
                 };
 
                 Ok(details)
@@ -87,8 +88,7 @@ impl View {
         };
 
         let other_options_scan = || {
-            let classify = matches.opt_present("classify");
-
+            let classify     = Classify::deduce(matches);
             let term_colours = TerminalColours::deduce(matches)?;
             let term_width   = TerminalWidth::deduce()?;
 
@@ -366,3 +366,12 @@ impl TerminalColours {
         }
     }
 }
+
+
+
+impl Classify {
+    fn deduce(matches: &getopts::Matches) -> Classify {
+        if matches.opt_present("classify") { Classify::AddFileIndicators }
+                                      else { Classify::JustFilenames }
+    }
+}
diff --git a/src/output/details.rs b/src/output/details.rs
@@ -101,7 +101,7 @@ use output::colours::Colours;
 use output::column::{Alignment, Column, Columns, SizeFormat};
 use output::cell::{TextCell, TextCellContents, DisplayWidth};
 use output::tree::TreeTrunk;
-use output::file_name::FileName;
+use output::file_name::{FileName, LinkStyle, Classify};
 
 
 /// With the **Details** view, the output gets formatted into columns, with
@@ -142,7 +142,7 @@ pub struct Details {
     pub colours: Colours,
 
     /// Whether to show a file type indiccator.
-    pub classify: bool,
+    pub classify: Classify,
 }
 
 /// The **environment** struct contains any data that could change between
@@ -310,7 +310,7 @@ impl Details {
             let row = Row {
                 depth:    depth,
                 cells:    Some(egg.cells),
-                name:     FileName::new(&egg.file, &self.colours).paint(true, self.classify).promote(),
+                name:     FileName::new(&egg.file, LinkStyle::FullLinkPaths, self.classify, &self.colours).paint().promote(),
                 last:     index == num_eggs - 1,
             };
 
@@ -443,8 +443,8 @@ impl<'a, U: Users+Groups+'a> Table<'a, U> {
         self.rows.push(row);
     }
 
-    pub fn filename(&self, file: File, links: bool) -> TextCellContents {
-        FileName::new(&file, &self.opts.colours).paint(links, self.opts.classify)
+    pub fn filename(&self, file: File, links: LinkStyle) -> TextCellContents {
+        FileName::new(&file, links, self.opts.classify, &self.opts.colours).paint()
     }
 
     pub fn add_file_with_cells(&mut self, cells: Vec<TextCell>, name_cell: TextCell, depth: usize, last: bool) {
diff --git a/src/output/file_name.rs b/src/output/file_name.rs
@@ -8,20 +8,51 @@ use output::escape;
 use output::cell::TextCellContents;
 
 
+/// A **file name** holds all the information necessary to display the name
+/// of the given file. This is used in all of the views.
 pub struct FileName<'a, 'dir: 'a> {
-    file:    &'a File<'dir>,
+
+    /// A reference to the file that we're getting the name of.
+    file: &'a File<'dir>,
+
+    /// The colours used to paint the file name and its surrounding text.
     colours: &'a Colours,
+
+    /// The file that this file points to if it's a link.
+    target: Option<FileTarget<'dir>>,
+
+    /// How to handle displaying links.
+    link_style: LinkStyle,
+
+    /// Whether to append file class characters to file names.
+    classify: Classify,
 }
 
+
 impl<'a, 'dir> FileName<'a, 'dir> {
-    pub fn new(file: &'a File<'dir>, colours: &'a Colours) -> FileName<'a, 'dir> {
+
+    /// Create a new `FileName` that prints the given file’s name, painting it
+    /// with the remaining arguments.
+    pub fn new(file: &'a File<'dir>, link_style: LinkStyle, classify: Classify, colours: &'a Colours) -> FileName<'a, 'dir> {
+        let target =  if file.is_link() { Some(file.link_target()) }
+                                                       else { None };
         FileName {
             file: file,
             colours: colours,
+            target: target,
+            link_style: link_style,
+            classify: classify,
         }
     }
 
-    pub fn paint(&self, links: bool, classify: bool) -> TextCellContents {
+
+    /// Paints the name of the file using the colours, resulting in a vector
+    /// of coloured cells that can be printed to the terminal.
+    ///
+    /// This method returns some `TextCellContents`, rather than a `TextCell`,
+    /// because for the last cell in a table, it doesn’t need to have its
+    /// width calculated.
+    pub fn paint(&self) -> TextCellContents {
         let mut bits = Vec::new();
 
         if self.file.dir.is_none() {
@@ -36,9 +67,9 @@ impl<'a, 'dir> FileName<'a, 'dir> {
             }
         }
 
-        if links && self.file.is_link() {
-            match self.file.link_target() {
-                FileTarget::Ok(target) => {
+        if let (LinkStyle::FullLinkPaths, Some(ref target)) = (self.link_style, self.target.as_ref()) {
+            match **target {
+                FileTarget::Ok(ref target) => {
                     bits.push(Style::default().paint(" "));
                     bits.push(self.colours.punctuation.paint("->"));
                     bits.push(Style::default().paint(" "));
@@ -48,14 +79,14 @@ impl<'a, 'dir> FileName<'a, 'dir> {
                     }
 
                     if !target.name.is_empty() {
-                        let target = FileName::new(&target, self.colours);
+                        let target = FileName::new(&target, LinkStyle::FullLinkPaths, Classify::JustFilenames, self.colours);
                         for bit in target.coloured_file_name() {
                             bits.push(bit);
                         }
                     }
                 },
 
-                FileTarget::Broken(broken_path) => {
+                FileTarget::Broken(ref broken_path) => {
                     bits.push(Style::default().paint(" "));
                     bits.push(self.colours.broken_arrow.paint("->"));
                     bits.push(Style::default().paint(" "));
@@ -64,10 +95,10 @@ impl<'a, 'dir> FileName<'a, 'dir> {
 
                 FileTarget::Err(_) => {
                     // Do nothing -- the error gets displayed on the next line
-                }
+                },
             }
         }
-        else if classify {
+        else if let Classify::AddFileIndicators = self.classify {
             if let Some(class) = self.classify_char() {
                 bits.push(Style::default().paint(class));
             }
@@ -76,6 +107,7 @@ impl<'a, 'dir> FileName<'a, 'dir> {
         bits.into()
     }
 
+
     /// Adds the bits of the parent path to the given bits vector.
     /// The path gets its characters escaped based on the colours.
     fn add_parent_bits(&self, bits: &mut Vec<ANSIString>, parent: &Path) {
@@ -90,6 +122,7 @@ impl<'a, 'dir> FileName<'a, 'dir> {
         }
     }
 
+
     /// The character to be displayed after a file when classifying is on, if
     /// the file’s type has one associated with it.
     fn classify_char(&self) -> Option<&'static str> {
@@ -108,6 +141,7 @@ impl<'a, 'dir> FileName<'a, 'dir> {
         }
     }
 
+
     /// Returns at least one ANSI-highlighted string representing this file’s
     /// name using the given set of colours.
     ///
@@ -125,7 +159,25 @@ impl<'a, 'dir> FileName<'a, 'dir> {
         bits
     }
 
+
+    /// Figures out which colour to paint the filename part of the output,
+    /// depending on which “type” of file it appears to be -- either from the
+    /// class on the filesystem or from its name.
     pub fn style(&self) -> Style {
+
+        // Override the style with the “broken link” style when this file is
+        // a link that we can’t follow for whatever reason. This is used when
+        // there’s no other place to show that the link doesn’t work.
+        if let LinkStyle::JustFilenames = self.link_style {
+            if let Some(ref target) = self.target {
+                if target.is_broken() {
+                    return self.colours.broken_arrow;
+                }
+            }
+        }
+
+        // Otherwise, just apply a bunch of rules in order. For example,
+        // executable image files should be executable rather than images.
         match self.file {
             f if f.is_directory()        => self.colours.filetypes.directory,
             f if f.is_executable_file()  => self.colours.filetypes.executable,
@@ -149,3 +201,38 @@ impl<'a, 'dir> FileName<'a, 'dir> {
         }
     }
 }
+
+
+/// When displaying a file name, there needs to be some way to handle broken
+/// links, depending on how long the resulting Cell can be.
+#[derive(PartialEq, Debug, Copy, Clone)]
+pub enum LinkStyle {
+
+    /// Just display the file names, but colour them differently if they’re
+    /// a broken link or can’t be followed.
+    JustFilenames,
+
+    /// Display all files in their usual style, but follow each link with an
+    /// arrow pointing to their path, colouring the path differently if it’s
+    /// a broken link, and doing nothing if it can’t be followed.
+    FullLinkPaths,
+}
+
+
+/// Whether to append file class characters to the file names.
+#[derive(PartialEq, Debug, Copy, Clone)]
+pub enum Classify {
+
+    /// Just display the file names, without any characters.
+    JustFilenames,
+
+    /// Add a character after the file name depending on what class of file
+    /// it is.
+    AddFileIndicators,
+}
+
+impl Default for Classify {
+    fn default() -> Classify {
+        Classify::JustFilenames
+    }
+}
diff --git a/src/output/grid.rs b/src/output/grid.rs
@@ -4,15 +4,15 @@ use term_grid as grid;
 
 use fs::File;
 use output::colours::Colours;
-use output::file_name::FileName;
+use output::file_name::{FileName, LinkStyle, Classify};
 
 
 #[derive(PartialEq, Debug, Copy, Clone)]
 pub struct Grid {
     pub across: bool,
     pub console_width: usize,
     pub colours: Colours,
-    pub classify: bool,
+    pub classify: Classify,
 }
 
 impl Grid {
@@ -28,7 +28,7 @@ impl Grid {
         grid.reserve(files.len());
 
         for file in files.iter() {
-            let filename = FileName::new(file, &self.colours).paint(false, self.classify);
+            let filename = FileName::new(file, LinkStyle::JustFilenames, self.classify, &self.colours).paint();
             let width = filename.width();
 
             grid.add(grid::Cell {
@@ -43,7 +43,7 @@ impl Grid {
         else {
             // File names too long for a grid - drop down to just listing them!
             for file in files.iter() {
-                let name_cell = FileName::new(file, &self.colours).paint(false, self.classify);
+                let name_cell = FileName::new(file, LinkStyle::JustFilenames, self.classify, &self.colours).paint();
                 writeln!(w, "{}", name_cell.strings())?;
             }
             Ok(())
diff --git a/src/output/grid_details.rs b/src/output/grid_details.rs
@@ -12,6 +12,8 @@ use output::cell::TextCell;
 use output::column::Column;
 use output::details::{Details, Table, Environment};
 use output::grid::Grid;
+use output::file_name::LinkStyle;
+
 
 #[derive(PartialEq, Debug, Clone)]
 pub struct GridDetails {
@@ -45,7 +47,7 @@ impl GridDetails {
                               .collect::<Vec<_>>();
 
             let file_names = files.into_iter()
-                                  .map(|file| first_table.filename(file, false).promote())
+                                  .map(|file| first_table.filename(file, LinkStyle::JustFilenames).promote())
                                   .collect::<Vec<_>>();
 
             (cells, file_names)
diff --git a/src/output/lines.rs b/src/output/lines.rs
@@ -4,21 +4,21 @@ use ansi_term::ANSIStrings;
 
 use fs::File;
 
-use output::file_name::FileName;
+use output::file_name::{FileName, LinkStyle, Classify};
 use super::colours::Colours;
 
 
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct Lines {
     pub colours: Colours,
-    pub classify: bool,
+    pub classify: Classify,
 }
 
 /// The lines view literally just displays each file, line-by-line.
 impl Lines {
     pub fn view<W: Write>(&self, files: Vec<File>, w: &mut W) -> IOResult<()> {
         for file in files {
-            let name_cell = FileName::new(&file, &self.colours).paint(true, self.classify);
+            let name_cell = FileName::new(&file, LinkStyle::FullLinkPaths, self.classify, &self.colours).paint();
             writeln!(w, "{}", ANSIStrings(&name_cell))?;
         }
         Ok(())
diff --git a/xtests/file_names_R b/xtests/file_names_R
@@ -6,7 +6,7 @@ emoji: [🆒]                       invalid-utf8-4: [�(�(]  utf-8: pâté
 escape: [[31m\u{1b}[0m]                 [1;34mlinks[0m                   vertical-tab: [[31m\u{b}[0m]
 
 /testcases/file-names/links:
-[36manother: [[31m\n[36m][0m  [36mbroken[0m  [36msubfile[0m
+[36manother: [[31m\n[36m][0m  [31mbroken[0m  [36msubfile[0m
 
 /testcases/file-names/new-line-dir: [\n]:
 another: [[31m\n[0m]  subfile
diff --git a/xtests/links b/xtests/links
@@ -1,2 +1,2 @@
-[36mbroken[0m       [36mforbidden[0m  [36mparent_dir[0m  some_file           [36msome_file_relative[0m
-[36mcurrent_dir[0m  [36mitself[0m     [36mroot[0m        [36msome_file_absolute[0m  [36musr[0m
+[31mbroken[0m       [31mforbidden[0m  [36mparent_dir[0m  some_file           [36msome_file_relative[0m
+[36mcurrent_dir[0m  [31mitself[0m     [36mroot[0m        [36msome_file_absolute[0m  [36musr[0m

-Original file line number
+Diff line change
 escape: [[31m\u{1b}[0m]                 [1;34mlinks[0m                   vertical-tab: [[31m\u{b}[0m]
 /testcases/file-names/links:
 -[36manother: [[31m\n[36m][0m  [36mbroken[0m  [36msubfile[0m
 +[36manother: [[31m\n[36m][0m  [31mbroken[0m  [36msubfile[0m
 /testcases/file-names/new-line-dir: [\n]:
 another: [[31m\n[0m]  subfile
-Original file line number
+Diff line change
@@ @@ -1,2 +1,2 @@ @@
 -[36mbroken[0m       [36mforbidden[0m  [36mparent_dir[0m  some_file           [36msome_file_relative[0m
 -[36mcurrent_dir[0m  [36mitself[0m     [36mroot[0m        [36msome_file_absolute[0m  [36musr[0m
 +[31mbroken[0m       [31mforbidden[0m  [36mparent_dir[0m  some_file           [36msome_file_relative[0m
 +[36mcurrent_dir[0m  [31mitself[0m     [36mroot[0m        [36msome_file_absolute[0m  [36musr[0m