From 4c2bf2f2e6231c14a8aa44cb49c828034c342a69 Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Thu, 17 Dec 2015 10:15:09 +0800 Subject: Encapsulate "display width" in a struct This commit introduces the `output::cell::DisplayWidth` struct, which encapsulates the Unicode *display width* of a string in a struct that makes it less easily confused with the *length* of a string. The use of this type means that it's now harder to accidentally use a string's length-in-bytes as its width. I've fixed at least one case in the code where this was being done! The only casualty is that it introduces a dependency on the output module from the file module, which will be removed next commit. --- src/file.rs | 7 +++-- src/output/cell.rs | 67 +++++++++++++++++++++++++++++++++++++--------- src/output/details.rs | 21 ++++++++++----- src/output/grid.rs | 2 +- src/output/grid_details.rs | 4 +-- src/output/mod.rs | 2 +- 6 files changed, 76 insertions(+), 27 deletions(-) (limited to 'src') diff --git a/src/file.rs b/src/file.rs index d4c9b9e..dd105b2 100644 --- a/src/file.rs +++ b/src/file.rs @@ -7,9 +7,8 @@ use std::io::Result as IOResult; use std::os::unix::fs::{MetadataExt, PermissionsExt}; use std::path::{Component, Path, PathBuf}; -use unicode_width::UnicodeWidthStr; - use dir::Dir; +use output::DisplayWidth; use self::fields as f; @@ -185,8 +184,8 @@ impl<'dir> File<'dir> { /// This is related to the number of graphemes in the string: most /// characters are 1 columns wide, but in some contexts, certain /// characters are actually 2 columns wide. - pub fn file_name_width(&self) -> usize { - UnicodeWidthStr::width(&self.name[..]) + pub fn file_name_width(&self) -> DisplayWidth { + DisplayWidth::from(&*self.name) } /// Assuming the current file is a symlink, follows the link and diff --git a/src/output/cell.rs b/src/output/cell.rs index 1d3f6e2..bb0d8ee 100644 --- a/src/output/cell.rs +++ b/src/output/cell.rs @@ -1,5 +1,7 @@ //! The `TextCell` type for the details and lines views. +use std::ops::{Deref, DerefMut}; + use ansi_term::{Style, ANSIString, ANSIStrings}; use unicode_width::UnicodeWidthStr; @@ -20,12 +22,8 @@ pub struct TextCell { /// The contents of this cell, as a vector of ANSI-styled strings. pub contents: TextCellContents, - /// The Unicode “display width” of this cell, in characters. - /// - /// As with the `File` type’s width, this is related to the number of - /// *graphemes*, rather than *characters*, in the cell: most are 1 column - /// wide, but in some contexts, certain characters are two columns wide. - pub length: usize, + /// The Unicode “display width” of this cell. + pub length: DisplayWidth, } impl TextCell { @@ -34,7 +32,7 @@ impl TextCell { /// computing the Unicode width of the text. pub fn paint(style: Style, text: String) -> Self { TextCell { - length: text.width(), + length: DisplayWidth::from(&*text), contents: vec![ style.paint(text) ], } } @@ -44,7 +42,7 @@ impl TextCell { /// `paint`, but.) pub fn paint_str(style: Style, text: &'static str) -> Self { TextCell { - length: text.len(), + length: DisplayWidth::from(text), contents: vec![ style.paint(text) ], } } @@ -57,7 +55,7 @@ impl TextCell { /// tabular data when there is *something* in each cell. pub fn blank(style: Style) -> Self { TextCell { - length: 1, + length: DisplayWidth::from(1), contents: vec![ style.paint("-") ], } } @@ -68,7 +66,7 @@ impl TextCell { pub fn add_spaces(&mut self, count: usize) { use std::iter::repeat; - self.length += count; + (*self.length) += count; let spaces: String = repeat(' ').take(count).collect(); self.contents.push(Style::default().paint(spaces)); @@ -77,12 +75,12 @@ impl TextCell { /// Adds the contents of another `ANSIString` to the end of this cell. pub fn push(&mut self, string: ANSIString<'static>, length: usize) { self.contents.push(string); - self.length += length; + (*self.length) += length; } /// Adds all the contents of another `TextCell` to the end of this cell. pub fn append(&mut self, other: TextCell) { - self.length += other.length; + (*self.length) += *other.length; self.contents.extend(other.contents); } @@ -127,4 +125,47 @@ impl TextCell { /// `TextCell` but aren’t concerned with tracking its width, because it occurs /// in the final cell of a table or grid and there’s no point padding it. This /// happens when dealing with file names. -pub type TextCellContents = Vec>; \ No newline at end of file +pub type TextCellContents = Vec>; + + +/// The Unicode “display width” of a string. +/// +/// This is related to the number of *graphemes* of a string, rather than the +/// number of *characters*, or *bytes*: although most characters are one +/// column wide, a few can be two columns wide, and this is important to note +/// when calculating widths for displaying tables in a terminal. +/// +/// This type is used to ensure that the width, rather than the length, is +/// used when constructing a `TextCell` -- it's too easy to write something +/// like `file_name.len()` and assume it will work! +/// +/// It has `From` impls that convert an input string or fixed with to values +/// of this type, and will `Deref` to the contained `usize` value. +#[derive(PartialEq, Debug, Clone, Copy, Default)] +pub struct DisplayWidth(usize); + +impl<'a> From<&'a str> for DisplayWidth { + fn from(input: &'a str) -> DisplayWidth { + DisplayWidth(UnicodeWidthStr::width(input)) + } +} + +impl From for DisplayWidth { + fn from(width: usize) -> DisplayWidth { + DisplayWidth(width) + } +} + +impl Deref for DisplayWidth { + type Target = usize; + + fn deref<'a>(&'a self) -> &'a Self::Target { + &self.0 + } +} + +impl DerefMut for DisplayWidth { + fn deref_mut<'a>(&'a mut self) -> &'a mut Self::Target { + &mut self.0 + } +} \ No newline at end of file diff --git a/src/output/details.rs b/src/output/details.rs index e9e59b2..4f224bd 100644 --- a/src/output/details.rs +++ b/src/output/details.rs @@ -125,7 +125,7 @@ use file::fields as f; use file::File; use options::{FileFilter, RecurseOptions}; use output::column::{Alignment, Column, Columns, SizeFormat}; -use output::cell::TextCell; +use output::cell::{TextCell, DisplayWidth}; use ansi_term::Style; @@ -361,7 +361,7 @@ impl Row { /// not, returns 0. fn column_width(&self, index: usize) -> usize { match self.cells { - Some(ref cells) => cells[index].length, + Some(ref cells) => *cells[index].length, None => 0, } } @@ -537,8 +537,13 @@ impl Table where U: Users { chars.push(c.attribute.paint("@")); } + // As these are all ASCII characters, we can guarantee that they’re + // all going to be one character wide, and don’t need to compute the + // cell’s display width. + let width = DisplayWidth::from(chars.len()); + TextCell { - length: chars.len(), + length: width, contents: chars, } } @@ -588,8 +593,12 @@ impl Table where U: Users { let number = if n < 10f64 { self.numeric.format_float(n, 1) } else { self.numeric.format_int(n as isize) }; + // The numbers and symbols are guaranteed to be written in ASCII, so + // we can skip the display width calculation. + let width = DisplayWidth::from(number.len() + symbol.len()); + TextCell { - length: number.len() + symbol.len(), + length: width, contents: vec![ self.colours.size.numbers.paint(number), self.colours.size.unit.paint(symbol), @@ -622,7 +631,7 @@ impl Table where U: Users { }; TextCell { - length: 2, + length: DisplayWidth::from(2), contents: vec![ git_char(git.staged), git_char(git.unstaged) @@ -679,7 +688,7 @@ impl Table where U: Users { if let Some(cells) = row.cells { for (n, (this_cell, width)) in cells.into_iter().zip(column_widths.iter()).enumerate() { - let padding = width - this_cell.length; + let padding = width - *this_cell.length; match self.columns[n].alignment() { Alignment::Left => { cell.append(this_cell); cell.add_spaces(padding); } diff --git a/src/output/grid.rs b/src/output/grid.rs index 13d2f8d..73c79e4 100644 --- a/src/output/grid.rs +++ b/src/output/grid.rs @@ -27,7 +27,7 @@ impl Grid { for file in files.iter() { grid.add(grid::Cell { contents: file_colour(&self.colours, file).paint(&*file.name).to_string(), - width: file.file_name_width(), + width: *file.file_name_width(), }); } diff --git a/src/output/grid_details.rs b/src/output/grid_details.rs index 332bfce..271a3a3 100644 --- a/src/output/grid_details.rs +++ b/src/output/grid_details.rs @@ -106,7 +106,7 @@ impl GridDetails { if row < column.len() { let cell = grid::Cell { contents: ANSIStrings(&column[row].contents).to_string(), - width: column[row].length, + width: *column[row].length, }; grid.add(cell); @@ -119,7 +119,7 @@ impl GridDetails { for cell in column.iter() { let cell = grid::Cell { contents: ANSIStrings(&cell.contents).to_string(), - width: cell.length, + width: *cell.length, }; grid.add(cell); diff --git a/src/output/mod.rs b/src/output/mod.rs index 642786e..88fcbe4 100644 --- a/src/output/mod.rs +++ b/src/output/mod.rs @@ -4,7 +4,7 @@ use colours::Colours; use file::File; use filetype::file_colour; -pub use self::cell::{TextCell, TextCellContents}; +pub use self::cell::{TextCell, TextCellContents, DisplayWidth}; pub use self::details::Details; pub use self::grid::Grid; pub use self::lines::Lines; -- cgit v1.2.3