Extract trait above file name colours

This commit meddles about with both the Colours and the FileExtensions.

Even though all the renderable fields were turned into traits, the FileName struct kept on accessing fields directly on the Colours value instead of calling methods on it. It also did the usual amount of colour misappropriation (such as ‘punctuation’ instead of specifying ‘normal_arrow’)

In preparation for when custom file colours are configurable (any day now), the colourise-file-by-kind functionality (links, sockets, or directories) was separated from the colourise-file-by-name functionality (images, videos, archives). The FileStyle struct already allowed for both to be separate; it was only changed so that a type other than FileExtensions could be used instead, as long as it implements the FileColours trait. (I feel like I should re-visit the naming of all these at some point in the future)

The decision to separate the two means that FileExtensions is the one assigning the colours, rather than going through the fields on a Colours value, which have all been removed. This is why a bunch of arbitrary Styles now exist in filetype.rs.

Because the decision on which colourise-file-by-name code to use (currently just the standard extensions, or nothing if we aren’t colourising) is now determined by the Colours type (instead of being derived), it’s possible to get it wrong. And wrong it was! There was a bug where file names were colourised even though the rest of the --long output wasn’t, and this wasn’t caught by the xtests. It is now.
This commit is contained in:
Benjamin Sago 2017-08-26 20:43:47 +01:00
parent e5e23e23c7
commit bfb8a5a573
9 changed files with 127 additions and 89 deletions

View File

@ -4,10 +4,13 @@
//! those are the only metadata that we have access to without reading the
//! files contents.
use ansi_term::Style;
use fs::File;
use output::file_name::FileColours;
#[derive(Debug)]
#[derive(Debug, Default, PartialEq)]
pub struct FileExtensions;
impl FileExtensions {
@ -15,7 +18,7 @@ impl FileExtensions {
/// An “immediate” file is something that can be run or activated somehow
/// in order to kick off the build of a project. Its usually only present
/// in directories full of source code.
pub fn is_immediate(&self, file: &File) -> bool {
fn is_immediate(&self, file: &File) -> bool {
file.name.starts_with("README") || file.name_is_one_of( &[
"Makefile", "Cargo.toml", "SConstruct", "CMakeLists.txt",
"build.gradle", "Rakefile", "Gruntfile.js",
@ -23,7 +26,7 @@ impl FileExtensions {
])
}
pub fn is_image(&self, file: &File) -> bool {
fn is_image(&self, file: &File) -> bool {
file.extension_is_one_of( &[
"png", "jpeg", "jpg", "gif", "bmp", "tiff", "tif",
"ppm", "pgm", "pbm", "pnm", "webp", "raw", "arw",
@ -32,7 +35,7 @@ impl FileExtensions {
])
}
pub fn is_video(&self, file: &File) -> bool {
fn is_video(&self, file: &File) -> bool {
file.extension_is_one_of( &[
"avi", "flv", "m2v", "mkv", "mov", "mp4", "mpeg",
"mpg", "ogm", "ogv", "vob", "wmv", "webm", "m2ts",
@ -40,26 +43,26 @@ impl FileExtensions {
])
}
pub fn is_music(&self, file: &File) -> bool {
fn is_music(&self, file: &File) -> bool {
file.extension_is_one_of( &[
"aac", "m4a", "mp3", "ogg", "wma", "mka", "opus",
])
}
// Lossless music, rather than any other kind of data...
pub fn is_lossless(&self, file: &File) -> bool {
fn is_lossless(&self, file: &File) -> bool {
file.extension_is_one_of( &[
"alac", "ape", "flac", "wav",
])
}
pub fn is_crypto(&self, file: &File) -> bool {
fn is_crypto(&self, file: &File) -> bool {
file.extension_is_one_of( &[
"asc", "enc", "gpg", "pgp", "sig", "signature", "pfx", "p12",
])
}
pub fn is_document(&self, file: &File) -> bool {
fn is_document(&self, file: &File) -> bool {
file.extension_is_one_of( &[
"djvu", "doc", "docx", "dvi", "eml", "eps", "fotd",
"odp", "odt", "pdf", "ppt", "pptx", "rtf",
@ -67,7 +70,7 @@ impl FileExtensions {
])
}
pub fn is_compressed(&self, file: &File) -> bool {
fn is_compressed(&self, file: &File) -> bool {
file.extension_is_one_of( &[
"zip", "tar", "Z", "z", "gz", "bz2", "a", "ar", "7z",
"iso", "dmg", "tc", "rar", "par", "tgz", "xz", "txz",
@ -75,13 +78,13 @@ impl FileExtensions {
])
}
pub fn is_temp(&self, file: &File) -> bool {
fn is_temp(&self, file: &File) -> bool {
file.name.ends_with('~')
|| (file.name.starts_with('#') && file.name.ends_with('#'))
|| file.extension_is_one_of( &[ "tmp", "swp", "swo", "swn", "bak" ])
}
pub fn is_compiled(&self, file: &File) -> bool {
fn is_compiled(&self, file: &File) -> bool {
if file.extension_is_one_of( &[ "class", "elc", "hi", "o", "pyc" ]) {
true
}
@ -93,3 +96,23 @@ impl FileExtensions {
}
}
}
impl FileColours for FileExtensions {
fn colour_file(&self, file: &File) -> Option<Style> {
use ansi_term::Colour::*;
Some(match file {
f if self.is_immediate(f) => Yellow.bold().underline(),
f if self.is_image(f) => Fixed(133).normal(),
f if self.is_video(f) => Fixed(135).normal(),
f if self.is_music(f) => Fixed(92).normal(),
f if self.is_lossless(f) => Fixed(93).normal(),
f if self.is_crypto(f) => Fixed(109).normal(),
f if self.is_document(f) => Fixed(105).normal(),
f if self.is_compressed(f) => Red.normal(),
f if self.is_temp(f) => Fixed(244).normal(),
f if self.is_compiled(f) => Fixed(137).normal(),
_ => return None,
})
}
}

View File

@ -251,7 +251,7 @@ mod customs_test {
let vars = MockVars { ls: $ls, exa: $exa };
for result in parse_for_test(&[], &[], Both, |mf| Colours::deduce(mf, &vars, || Some(80))) {
assert_eq!(result, Ok(c));
assert_eq!(result.as_ref(), Ok(&c));
}
}
};

View File

@ -2,14 +2,13 @@ use output::Colours;
use output::{View, Mode, grid, details};
use output::grid_details::{self, RowThreshold};
use output::table::{TimeTypes, Environment, SizeFormat, Columns, Options as TableOptions};
use output::file_name::{Classify, FileStyle};
use output::file_name::{Classify, FileStyle, NoFileColours};
use output::time::TimeFormat;
use options::{flags, Misfire, Vars};
use options::parser::MatchedFlags;
use fs::feature::xattr;
use info::filetype::FileExtensions;
impl View {
@ -18,7 +17,7 @@ impl View {
pub fn deduce<V: Vars>(matches: &MatchedFlags, vars: &V) -> Result<View, Misfire> {
let mode = Mode::deduce(matches, vars)?;
let colours = Colours::deduce(matches, vars, || *TERM_WIDTH)?;
let style = FileStyle::deduce(matches)?;
let style = FileStyle::deduce(matches, &colours)?;
Ok(View { mode, colours, style })
}
}
@ -332,9 +331,15 @@ impl TimeTypes {
impl FileStyle {
fn deduce(matches: &MatchedFlags) -> Result<FileStyle, Misfire> {
#[allow(trivial_casts)]
fn deduce(matches: &MatchedFlags, colours: &Colours) -> Result<FileStyle, Misfire> {
use info::filetype::FileExtensions;
let classify = Classify::deduce(matches)?;
let exts = FileExtensions;
let exts = if colours.colourful { Box::new(FileExtensions) as Box<_> }
else { Box::new(NoFileColours) as Box<_> };
Ok(FileStyle { classify, exts })
}
}

View File

@ -2,14 +2,15 @@ use ansi_term::Style;
use ansi_term::Colour::{Red, Green, Yellow, Blue, Cyan, Purple, Fixed};
use output::render;
use output::file_name::Colours as FileNameColours;
#[derive(Clone, Copy, Debug, Default, PartialEq)]
#[derive(Debug, Default, PartialEq)]
pub struct Colours {
pub colourful: bool,
pub scale: bool,
pub filekinds: FileKinds,
pub filetypes: FileTypes,
pub perms: Permissions,
pub size: Size,
pub users: Users,
@ -28,7 +29,6 @@ pub struct Colours {
pub control_char: Style,
}
// Colours for files depending on their filesystem type.
#[derive(Clone, Copy, Debug, Default, PartialEq)]
pub struct FileKinds {
pub normal: Style,
@ -42,21 +42,6 @@ pub struct FileKinds {
pub executable: Style,
}
// Colours for files depending on their name or extension.
#[derive(Clone, Copy, Debug, Default, PartialEq)]
pub struct FileTypes {
pub image: Style,
pub video: Style,
pub music: Style,
pub lossless: Style,
pub crypto: Style,
pub document: Style,
pub compressed: Style,
pub temp: Style,
pub immediate: Style,
pub compiled: Style,
}
#[derive(Clone, Copy, Debug, Default, PartialEq)]
pub struct Permissions {
pub user_read: Style,
@ -123,6 +108,7 @@ impl Colours {
pub fn colourful(scale: bool) -> Colours {
Colours {
colourful: true,
scale: scale,
filekinds: FileKinds {
@ -137,19 +123,6 @@ impl Colours {
executable: Green.bold(),
},
filetypes: FileTypes {
image: Fixed(133).normal(),
video: Fixed(135).normal(),
music: Fixed(92).normal(),
lossless: Fixed(93).normal(),
crypto: Fixed(109).normal(),
document: Fixed(105).normal(),
compressed: Red.normal(),
temp: Fixed(244).normal(),
immediate: Yellow.bold().underline(),
compiled: Fixed(137).normal(),
},
perms: Permissions {
user_read: Yellow.bold(),
user_write: Red.bold(),
@ -307,3 +280,12 @@ impl render::UserColours for Colours {
fn someone_else(&self) -> Style { self.users.user_someone_else }
}
impl FileNameColours for Colours {
fn broken_arrow(&self) -> Style { self.broken_arrow }
fn broken_filename(&self) -> Style { self.broken_filename }
fn normal_arrow(&self) -> Style { self.punctuation }
fn control_char(&self) -> Style { self.control_char }
fn symlink_path(&self) -> Style { self.symlink_path }
fn executable_file(&self) -> Style { self.filekinds.executable }
}

View File

@ -3,10 +3,9 @@ use std::path::Path;
use ansi_term::{ANSIString, Style};
use fs::{File, FileTarget};
use info::filetype::FileExtensions;
use output::Colours;
use output::escape;
use output::cell::TextCellContents;
use output::render::FiletypeColours;
/// Basically a file name factory.
@ -17,19 +16,19 @@ pub struct FileStyle {
pub classify: Classify,
/// Mapping of file extensions to colours, to highlight regular files.
pub exts: FileExtensions,
pub exts: Box<FileColours>,
}
impl FileStyle {
/// Create a new `FileName` that prints the given files name, painting it
/// with the remaining arguments.
pub fn for_file<'a, 'dir>(&'a self, file: &'a File<'dir>, colours: &'a Colours) -> FileName<'a, 'dir> {
pub fn for_file<'a, 'dir, C: Colours>(&'a self, file: &'a File<'dir>, colours: &'a C) -> FileName<'a, 'dir, C> {
FileName {
file, colours,
link_style: LinkStyle::JustFilenames,
exts: &self.exts,
classify: self.classify,
exts: &*self.exts,
target: if file.is_link() { Some(file.link_target()) }
else { None }
}
@ -75,15 +74,15 @@ impl Default for Classify {
/// 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> {
pub struct FileName<'a, 'dir: 'a, C: Colours+'a> {
/// A reference to the file that we're getting the name of.
/// A reference to the file that were getting the name of.
file: &'a File<'dir>,
/// The colours used to paint the file name and its surrounding text.
colours: &'a Colours,
colours: &'a C,
/// The file that this file points to if it's a link.
/// The file that this file points to if its a link.
target: Option<FileTarget<'dir>>,
/// How to handle displaying links.
@ -93,11 +92,11 @@ pub struct FileName<'a, 'dir: 'a> {
classify: Classify,
/// Mapping of file extensions to colours, to highlight regular files.
exts: &'a FileExtensions,
exts: &'a FileColours,
}
impl<'a, 'dir> FileName<'a, 'dir> {
impl<'a, 'dir, C: Colours> FileName<'a, 'dir, C> {
/// Sets the flag on this file name to display link targets with an
/// arrow followed by their path.
@ -131,7 +130,7 @@ impl<'a, 'dir> FileName<'a, 'dir> {
match *target {
FileTarget::Ok(ref target) => {
bits.push(Style::default().paint(" "));
bits.push(self.colours.punctuation.paint("->"));
bits.push(self.colours.normal_arrow().paint("->"));
bits.push(Style::default().paint(" "));
if let Some(parent) = target.path.parent() {
@ -156,9 +155,9 @@ impl<'a, 'dir> FileName<'a, 'dir> {
FileTarget::Broken(ref broken_path) => {
bits.push(Style::default().paint(" "));
bits.push(self.colours.broken_arrow.paint("->"));
bits.push(self.colours.broken_arrow().paint("->"));
bits.push(Style::default().paint(" "));
escape(broken_path.display().to_string(), &mut bits, self.colours.broken_filename, self.colours.control_char.underline());
escape(broken_path.display().to_string(), &mut bits, self.colours.broken_filename(), self.colours.control_char().underline());
},
FileTarget::Err(_) => {
@ -182,11 +181,11 @@ impl<'a, 'dir> FileName<'a, 'dir> {
let coconut = parent.components().count();
if coconut == 1 && parent.has_root() {
bits.push(self.colours.symlink_path.paint("/"));
bits.push(self.colours.symlink_path().paint("/"));
}
else if coconut >= 1 {
escape(parent.to_string_lossy().to_string(), bits, self.colours.symlink_path, self.colours.control_char);
bits.push(self.colours.symlink_path.paint("/"));
escape(parent.to_string_lossy().to_string(), bits, self.colours.symlink_path(), self.colours.control_char());
bits.push(self.colours.symlink_path().paint("/"));
}
}
@ -223,7 +222,7 @@ impl<'a, 'dir> FileName<'a, 'dir> {
fn coloured_file_name<'unused>(&self) -> Vec<ANSIString<'unused>> {
let file_style = self.style();
let mut bits = Vec::new();
escape(self.file.name.clone(), &mut bits, file_style, self.colours.control_char);
escape(self.file.name.clone(), &mut bits, file_style, self.colours.control_char());
bits
}
@ -232,43 +231,57 @@ impl<'a, 'dir> FileName<'a, 'dir> {
/// 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 {
use output::render::FiletypeColours;
// Override the style with the “broken link” style when this file is
// a link that we cant follow for whatever reason. This is used when
// theres no other place to show that the link doesnt work.
if let LinkStyle::JustFilenames = self.link_style {
if let Some(ref target) = self.target {
if target.is_broken() {
return self.colours.broken_arrow;
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 {
self.kind_style()
.or_else(|| self.exts.colour_file(self.file))
.unwrap_or_else(|| self.colours.normal())
}
fn kind_style(&self) -> Option<Style> {
Some(match self.file {
f if f.is_directory() => self.colours.directory(),
f if f.is_executable_file() => self.colours.filekinds.executable,
f if f.is_executable_file() => self.colours.executable_file(),
f if f.is_link() => self.colours.symlink(),
f if f.is_pipe() => self.colours.pipe(),
f if f.is_block_device() => self.colours.block_device(),
f if f.is_char_device() => self.colours.char_device(),
f if f.is_socket() => self.colours.socket(),
f if !f.is_file() => self.colours.special(),
f if self.exts.is_immediate(f) => self.colours.filetypes.immediate,
f if self.exts.is_image(f) => self.colours.filetypes.image,
f if self.exts.is_video(f) => self.colours.filetypes.video,
f if self.exts.is_music(f) => self.colours.filetypes.music,
f if self.exts.is_lossless(f) => self.colours.filetypes.lossless,
f if self.exts.is_crypto(f) => self.colours.filetypes.crypto,
f if self.exts.is_document(f) => self.colours.filetypes.document,
f if self.exts.is_compressed(f) => self.colours.filetypes.compressed,
f if self.exts.is_temp(f) => self.colours.filetypes.temp,
f if self.exts.is_compiled(f) => self.colours.filetypes.compiled,
_ => self.colours.normal(),
}
_ => return None,
})
}
}
pub trait Colours: FiletypeColours {
fn broken_arrow(&self) -> Style;
fn broken_filename(&self) -> Style;
fn normal_arrow(&self) -> Style;
fn control_char(&self) -> Style;
fn symlink_path(&self) -> Style;
fn executable_file(&self) -> Style;
}
// needs Debug because FileStyle derives it
use std::fmt::Debug;
use std::marker::Sync;
pub trait FileColours: Debug+Sync {
fn colour_file(&self, file: &File) -> Option<Style>;
}
#[derive(PartialEq, Debug)]
pub struct NoFileColours;
impl FileColours for NoFileColours {
fn colour_file(&self, _file: &File) -> Option<Style> { None }
}

View File

@ -1,7 +1,6 @@
use std::io::{Write, Result as IOResult};
use ansi_term::ANSIStrings;
use fs::File;
use output::file_name::{FileName, FileStyle};
@ -25,7 +24,7 @@ impl<'a> Render<'a> {
Ok(())
}
fn render_file<'f>(&self, file: &'f File<'a>) -> FileName<'f, 'a> {
fn render_file<'f>(&self, file: &'f File<'a>) -> FileName<'f, 'a, Colours> {
self.style.for_file(file, self.colours).with_link_paths()
}
}

View File

@ -0,0 +1,6 @@
#SAVEFILE# compressed.deb crypto.asc image.svg VIDEO.AVI
backup~ compressed.tar.gz crypto.signature lossless.flac video.wmv
compiled.class compressed.tar.xz document.pdf lossless.wav
compiled.coffee compressed.tgz DOCUMENT.XLSX Makefile
compiled.js compressed.txz file.tmp music.mp3
compiled.o COMPRESSED.ZIP IMAGE.PNG MUSIC.OGG

6
xtests/file_names_bw Normal file
View File

@ -0,0 +1,6 @@
ansi: [\u{1b}[34mblue\u{1b}[0m] form-feed: [\u{c}] new-line-dir: [\n]
ascii: hello invalid-utf8-1: [<5B>] new-line: [\n]
backspace: [\u{8}] invalid-utf8-2: [<5B>(] return: [\r]
bell: [\u{7}] invalid-utf8-3: [<5B>(] tab: [\t]
emoji: [🆒] invalid-utf8-4: [<5B>(<28>(] utf-8: pâté
escape: [\u{1b}] links vertical-tab: [\u{b}]

View File

@ -178,6 +178,10 @@ COLUMNS=80 $exa_binary --colour=always $testcases/files -l | diff -q - $resul
COLUMNS=80 $exa_binary --colour=never $testcases/files -l | diff -q - $results/files_l_bw || exit 1
COLUMNS=80 $exa_binary --colour=automatic $testcases/files -l | diff -q - $results/files_l_bw || exit 1
# Switching colour off
COLUMNS=80 $exa_binary --colour=never $testcases/file-names | diff -q - $results/file_names_bw || exit 1
COLUMNS=80 $exa_binary --colour=never $testcases/file-names-exts | diff -q - $results/file-names-exts-bw || exit 1
# Git
$exa $testcases/git/additions -l --git 2>&1 | diff -q - $results/git_additions || exit 1