From 1da1142a7ef049017a35b2af76ece3ce9a43fc36 Mon Sep 17 00:00:00 2001 From: Ben S Date: Mon, 23 Feb 2015 11:32:35 +0000 Subject: Fix panic when previewing symlink to ., .., or / The old implementation blindly assumed that a symlink target would have a directory compoment, which the current directory, parent directory, and root directory technically don't have. Fixes #20. --- src/file.rs | 95 ++++++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 72 insertions(+), 23 deletions(-) (limited to 'src/file.rs') diff --git a/src/file.rs b/src/file.rs index d1cd7b8..6d89fef 100644 --- a/src/file.rs +++ b/src/file.rs @@ -54,18 +54,7 @@ impl<'a> File<'a> { /// Create a new File object from the given Stat result, and other data. pub fn with_stat(stat: io::FileStat, path: &Path, parent: Option<&'a Dir>, recurse: bool) -> File<'a> { - - // The filename to display is the last component of the path. However, - // the path has no components for `.`, `..`, and `/`, so in these - // cases, the entire path is used. - let bytes = match path.components().last() { - Some(b) => b, - None => path.as_vec(), - }; - - // Convert the string to UTF-8, replacing any invalid characters with - // replacement characters. - let filename = String::from_utf8_lossy(bytes); + let filename = path_filename(path); // If we are recursing, then the `this` field contains a Dir object // that represents the current File as a directory, if it is a @@ -81,8 +70,8 @@ impl<'a> File<'a> { path: path.clone(), dir: parent, stat: stat, - name: filename.to_string(), ext: ext(&filename), + name: filename, this: this, } } @@ -146,12 +135,44 @@ impl<'a> File<'a> { }; match self.target_file(&target_path) { - Ok(file) => format!("{} {} {}{}{}", - style.paint(name), - GREY.paint("=>"), - Cyan.paint(target_path.dirname_str().unwrap()), - Cyan.paint("/"), - file.file_colour().paint(&file.name)), + Ok(file) => { + + // Generate a preview for the path this symlink links to. + // The preview should consist of the directory of the file + // (if present) in cyan, an extra slash if necessary, then + // the target file, colourised in the appropriate style. + let mut path_prefix = String::new(); + + // The root directory has the name "/", which has to be + // catered for separately, otherwise there'll be two + // slashes in the resulting output. + if file.path.is_absolute() && file.name != "/" { + path_prefix.push_str("/"); + } + + let path_bytes: Vec<&[u8]> = file.path.components().collect(); + if !path_bytes.is_empty() { + // Use init() to add all but the last component of the + // path to the prefix. init() panics when given an + // empty list, hence the check. + for component in path_bytes.init().iter() { + let string = String::from_utf8_lossy(component).to_string(); + path_prefix.push_str(&string); + } + } + + // Only add a slash when there's something in the path + // prefix so far. + if path_bytes.len() > 1 { + path_prefix.push_str("/"); + } + + format!("{} {} {}", + style.paint(name), + GREY.paint("=>"), + ANSIStrings(&[ Cyan.paint(&path_prefix), + file.file_colour().paint(&file.name) ])) + }, Err(filename) => format!("{} {} {}", style.paint(name), Red.paint("=>"), @@ -184,8 +205,7 @@ impl<'a> File<'a> { /// other end doesn't exist), returns the *filename* of the file /// that should be there. fn target_file(&self, target_path: &Path) -> Result { - let v = target_path.filename().unwrap(); - let filename = String::from_utf8_lossy(v); + let filename = path_filename(target_path); // Use stat instead of lstat - we *want* to follow links. if let Ok(stat) = fs::stat(target_path) { @@ -193,8 +213,8 @@ impl<'a> File<'a> { path: target_path.clone(), dir: self.dir, stat: stat, - name: filename.to_string(), ext: ext(&filename), + name: filename, this: None, }) } @@ -425,6 +445,21 @@ impl<'a> File<'a> { } } +/// Extract the filename to display from a path, converting it from UTF-8 +/// lossily, into a String. +/// +/// The filename to display is the last component of the path. However, +/// the path has no components for `.`, `..`, and `/`, so in these +/// cases, the entire path is used. +fn path_filename(path: &Path) -> String { + let bytes = match path.components().last() { + Some(b) => b, + None => path.as_vec(), + }; + + String::from_utf8_lossy(bytes).to_string() +} + /// Extract an extension from a string, if one is present, in lowercase. /// /// The extension is the series of characters after the last dot. This @@ -440,15 +475,29 @@ fn ext<'a>(name: &'a str) -> Option { #[cfg(test)] pub mod test { pub use super::*; + pub use super::path_filename; + pub use column::{Cell, Column}; pub use std::old_io as io; + pub use output::details::UserLocale; + pub use users::{User, Group}; pub use users::mock::MockUsers; pub use ansi_term::Style::Plain; pub use ansi_term::Colour::Yellow; - pub use output::details::UserLocale; + #[test] + fn current_filename() { + let filename = path_filename(&Path::new(".")); + assert_eq!(&filename[..], ".") + } + + #[test] + fn parent_filename() { + let filename = path_filename(&Path::new("..")); + assert_eq!(&filename[..], "..") + } #[test] fn extension() { -- cgit v1.2.3