Przeglądaj źródła

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.
Benjamin Sago 10 lat temu
rodzic
commit
4c2bf2f2e6
6 zmienionych plików z 76 dodań i 27 usunięć
  1. 3 4
      src/file.rs
  2. 54 13
      src/output/cell.rs
  3. 15 6
      src/output/details.rs
  4. 1 1
      src/output/grid.rs
  5. 2 2
      src/output/grid_details.rs
  6. 1 1
      src/output/mod.rs

+ 3 - 4
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

+ 54 - 13
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<ANSIString<'static>>;
+pub type TextCellContents = Vec<ANSIString<'static>>;
+
+
+/// 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<usize> 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
+    }
+}

+ 15 - 6
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<U> Table<U> 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<U> Table<U> 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<U> Table<U> 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<U> Table<U> 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); }

+ 1 - 1
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(),
             });
         }
 

+ 2 - 2
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);

+ 1 - 1
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;