From 449592c1502b5804ae2a32247e23dde31b95daaa Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Sat, 24 Jan 2015 17:26:26 +0000 Subject: [PATCH] Do column width calculations ourselves (speedup) Instead of stripping the ANSI formatting characters from our strings, work out the length without them and use that. This is per-column, but most of them are simple (just the same number of characters in the non-coloured string). Sometimes, this is really simple: for example, trwxrwxrwx permissions strings are always going to be ten characters long, and the strings that get returned are chock full of ANSI escape codes. This should have a small benefit on performance. --- src/column.rs | 16 +++++++ src/file.rs | 123 +++++++++++++++++++++++++++++++------------------- src/output.rs | 21 +++------ 3 files changed, 99 insertions(+), 61 deletions(-) diff --git a/src/column.rs b/src/column.rs index 013b07f..d842a86 100644 --- a/src/column.rs +++ b/src/column.rs @@ -1,5 +1,7 @@ use std::iter::repeat; +use ansi_term::Style; + #[derive(PartialEq, Show)] pub enum Column { Permissions, @@ -79,3 +81,17 @@ impl Alignment { } } } + +pub struct Cell { + pub length: usize, + pub text: String, +} + +impl Cell { + pub fn paint(style: Style, string: &str) -> Cell { + Cell { + text: style.paint(string).to_string(), + length: string.len(), + } + } +} diff --git a/src/file.rs b/src/file.rs index f682b74..8a6d060 100644 --- a/src/file.rs +++ b/src/file.rs @@ -9,7 +9,7 @@ use users::Users; use number_prefix::{binary_prefix, decimal_prefix, Prefixed, Standalone, PrefixNames}; -use column::{Column, SizeFormat}; +use column::{Column, SizeFormat, Cell}; use column::Column::*; use dir::Dir; use filetype::HasType; @@ -68,7 +68,7 @@ impl<'a> File<'a> { } /// Get the data for a column, formatted as a coloured string. - pub fn display(&self, column: &Column, users_cache: &mut U) -> String { + pub fn display(&self, column: &Column, users_cache: &mut U) -> Cell { match *column { Permissions => self.permissions_string(), FileName => self.file_name_view(), @@ -86,11 +86,16 @@ impl<'a> File<'a> { /// /// It consists of the file name coloured in the appropriate style, and, /// if it's a symlink, an arrow pointing to the file it links to, also - /// coloured in the appropriate style. Files that don't exist will be - /// coloured red. - pub fn file_name_view(&self) -> String { - let name = self.name.as_slice(); - let displayed_name = self.file_colour().paint(name); + /// coloured in the appropriate style. + /// + /// If the symlink target doesn't exist, then instead of displaying + /// an error, highlight the target and arrow in red. The error would + /// be shown out of context, and it's almost always because the + /// target doesn't exist. + pub fn file_name_view(&self) -> Cell { + let name = &*self.name; + let style = self.file_colour(); + if self.stat.kind == io::FileType::Symlink { match fs::readlink(&self.path) { Ok(path) => { @@ -98,13 +103,31 @@ impl<'a> File<'a> { Some(dir) => dir.path.join(path), None => path, }; - format!("{} {}", displayed_name, self.target_file_name_and_arrow(&target_path)) + + match self.target_file(&target_path) { + Ok(file) => Cell { + length: 0, // These lengths are never actually used... + text: format!("{} {} {}{}{}", + style.paint(name), + GREY.paint("=>"), + Cyan.paint(target_path.dirname_str().unwrap()), + Cyan.paint("/"), + file.file_colour().paint(file.name.as_slice())), + }, + Err(filename) => Cell { + length: 0, // ...because the rightmost column lengths are ignored! + text: format!("{} {} {}", + style.paint(name), + Red.paint("=>"), + Red.underline().paint(filename.as_slice())), + }, + } } - Err(_) => displayed_name.to_string(), + Err(_) => Cell::paint(style, name), } } else { - displayed_name.to_string() + Cell::paint(style, name) } } @@ -122,36 +145,35 @@ impl<'a> File<'a> { self.name.as_slice().width(false) } - fn target_file_name_and_arrow(&self, target_path: &Path) -> String { + /// Assuming the current file is a symlink, follows the link and + /// returns a File object from the path the link points to. + /// + /// If statting the file fails (usually because the file on the + /// 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); // Use stat instead of lstat - we *want* to follow links. - let link_target = fs::stat(target_path).map(|stat| File { - path: target_path.clone(), - dir: self.dir, - stat: stat, - name: filename.to_string(), - ext: ext(filename.as_slice()), - }); - - // Statting a path usually fails because the file at the - // other end doesn't exist. Show this by highlighting the - // target file in red instead of displaying an error, because - // the error would be shown out of context (before the - // results, not right by the file) and it's almost always for - // that reason anyway. - - match link_target { - Ok(file) => format!("{} {}{}{}", GREY.paint("=>"), Cyan.paint(target_path.dirname_str().unwrap()), Cyan.paint("/"), file.file_colour().paint(filename.as_slice())), - Err(_) => format!("{} {}", Red.paint("=>"), Red.underline().paint(filename.as_slice())), + if let Ok(stat) = fs::stat(target_path) { + Ok(File { + path: target_path.clone(), + dir: self.dir, + stat: stat, + name: filename.to_string(), + ext: ext(filename.as_slice()), + }) + } + else { + Err(filename.to_string()) } } /// This file's number of hard links as a coloured string. - fn hard_links(&self) -> String { + fn hard_links(&self) -> Cell { let style = if self.has_multiple_links() { Red.on(Yellow) } else { Red.normal() }; - style.paint(self.stat.unstable.nlink.to_string().as_slice()).to_string() + Cell::paint(style, &*self.stat.unstable.nlink.to_string()) } /// Whether this is a regular file with more than one link. @@ -164,17 +186,17 @@ impl<'a> File<'a> { } /// This file's inode as a coloured string. - fn inode(&self) -> String { - Purple.paint(self.stat.unstable.inode.to_string().as_slice()).to_string() + fn inode(&self) -> Cell { + Cell::paint(Purple.normal(), &*self.stat.unstable.inode.to_string()) } /// This file's number of filesystem blocks (if available) as a coloured string. - fn blocks(&self) -> String { + fn blocks(&self) -> Cell { if self.stat.kind == io::FileType::RegularFile || self.stat.kind == io::FileType::Symlink { - Cyan.paint(self.stat.unstable.blocks.to_string().as_slice()).to_string() + Cell::paint(Cyan.normal(), &*self.stat.unstable.blocks.to_string()) } else { - GREY.paint("-").to_string() + Cell { text: GREY.paint("-").to_string(), length: 1 } } } @@ -183,7 +205,7 @@ impl<'a> File<'a> { /// If the user is not present, then it formats the uid as a number /// instead. This usually happens when a user is deleted, but still owns /// files. - fn user(&self, users_cache: &mut U) -> String { + fn user(&self, users_cache: &mut U) -> Cell { let uid = self.stat.unstable.uid as i32; let user_name = match users_cache.get_user_by_uid(uid) { @@ -192,13 +214,13 @@ impl<'a> File<'a> { }; let style = if users_cache.get_current_uid() == uid { Yellow.bold() } else { Plain }; - style.paint(user_name.as_slice()).to_string() + Cell::paint(style, &*user_name) } /// This file's group name as a coloured string. /// /// As above, if not present, it formats the gid as a number instead. - fn group(&self, users_cache: &mut U) -> String { + fn group(&self, users_cache: &mut U) -> Cell { let gid = self.stat.unstable.gid as u32; let mut style = Plain; @@ -215,7 +237,7 @@ impl<'a> File<'a> { None => gid.to_string(), }; - style.paint(group_name.as_slice()).to_string() + Cell::paint(style, &*group_name) } /// This file's size, formatted using the given way, as a coloured string. @@ -224,22 +246,27 @@ impl<'a> File<'a> { /// some filesystems, I've never looked at one of those numbers and gained /// any information from it, so by emitting "-" instead, the table is less /// cluttered with numbers. - fn file_size(&self, size_format: SizeFormat) -> String { + fn file_size(&self, size_format: SizeFormat) -> Cell { if self.stat.kind == io::FileType::Directory { - GREY.paint("-").to_string() + Cell { text: GREY.paint("-").to_string(), length: 1 } } else { let result = match size_format { SizeFormat::DecimalBytes => decimal_prefix(self.stat.size as f64), SizeFormat::BinaryBytes => binary_prefix(self.stat.size as f64), - SizeFormat::JustBytes => return Green.bold().paint(self.stat.size.to_string().as_slice()).to_string(), + SizeFormat::JustBytes => return Cell::paint(Green.bold(), &*self.stat.size.to_string()) }; match result { - Standalone(bytes) => Green.bold().paint(bytes.to_string().as_slice()).to_string(), + Standalone(bytes) => Cell::paint(Green.bold(), &*bytes.to_string()), Prefixed(prefix, n) => { let number = if n < 10f64 { format!("{:.1}", n) } else { format!("{:.0}", n) }; - format!("{}{}", Green.bold().paint(number.as_slice()), Green.paint(prefix.symbol())) + let symbol = prefix.symbol(); + + Cell { + text: format!("{}{}", Green.bold().paint(&*number), Green.paint(symbol)), + length: number.len() + symbol.len(), + } } } } @@ -265,14 +292,14 @@ impl<'a> File<'a> { /// Each character is given its own colour. The first three permission /// bits are bold because they're the ones used most often, and executable /// files are underlined to make them stand out more. - fn permissions_string(&self) -> String { + fn permissions_string(&self) -> Cell { let bits = self.stat.perm; let executable_colour = match self.stat.kind { io::FileType::RegularFile => Green.bold().underline(), _ => Green.bold(), }; - return format!("{}{}{}{}{}{}{}{}{}{}", + let string = format!("{}{}{}{}{}{}{}{}{}{}", self.type_char(), File::permission_bit(&bits, io::USER_READ, "r", Yellow.bold()), File::permission_bit(&bits, io::USER_WRITE, "w", Red.bold()), @@ -284,6 +311,8 @@ impl<'a> File<'a> { File::permission_bit(&bits, io::OTHER_WRITE, "w", Red.normal()), File::permission_bit(&bits, io::OTHER_EXECUTE, "x", Green.normal()), ); + + Cell { text: string, length: 10 } } /// Helper method for the permissions string. diff --git a/src/output.rs b/src/output.rs index 1f2fc68..2a0993b 100644 --- a/src/output.rs +++ b/src/output.rs @@ -1,13 +1,12 @@ use std::cmp::max; use std::iter::{AdditiveIterator, repeat}; -use column::Column; +use column::{Column, Cell}; use column::Alignment::Left; use file::File; use users::OSUsers; use ansi_term::Style::Plain; -use ansi_term::strip_formatting; #[derive(PartialEq, Show)] pub enum View { @@ -29,7 +28,7 @@ impl View { /// The lines view literally just displays each file, line-by-line. fn lines_view(files: Vec) { for file in files.iter() { - println!("{}", file.file_name_view()); + println!("{}", file.file_name_view().text); } } @@ -130,22 +129,16 @@ fn details_view(columns: &Vec, files: Vec, header: bool) { let mut cache = OSUsers::empty_cache(); - let mut table: Vec> = files.iter() + let mut table: Vec> = files.iter() .map(|f| columns.iter().map(|c| f.display(c, &mut cache)).collect()) .collect(); if header { - table.insert(0, columns.iter().map(|c| Plain.underline().paint(c.header()).to_string()).collect()); + table.insert(0, columns.iter().map(|c| Cell::paint(Plain.underline(), c.header())).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> = table.iter() - .map(|row| row.iter().map(|col| strip_formatting(col.as_slice()).len()).collect()) + .map(|row| row.iter().map(|c| c.length).collect()) .collect(); let column_widths: Vec = range(0, columns.len()) @@ -160,11 +153,11 @@ fn details_view(columns: &Vec, files: Vec, header: bool) { if num == columns.len() - 1 { // The final column doesn't need to have trailing spaces - print!("{}", row[num]); + print!("{}", row[num].text); } else { let padding = column_widths[num] - field_widths[num]; - print!("{}", column.alignment().pad_string(&row[num], padding)); + print!("{}", column.alignment().pad_string(&row[num].text, padding)); } } print!("\n"); -- GitLab