From e5e23e23c7602992d72786d0642e2c5e9717a34a Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Sat, 26 Aug 2017 15:29:39 +0100 Subject: [PATCH 1/8] Use the file type colour trait methods --- src/output/file_name.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/output/file_name.rs b/src/output/file_name.rs index fbc0623..112f12e 100644 --- a/src/output/file_name.rs +++ b/src/output/file_name.rs @@ -232,6 +232,7 @@ 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 can’t follow for whatever reason. This is used when @@ -247,14 +248,14 @@ impl<'a, 'dir> FileName<'a, 'dir> { // Otherwise, just apply a bunch of rules in order. For example, // executable image files should be executable rather than images. match self.file { - f if f.is_directory() => self.colours.filekinds.directory, + f if f.is_directory() => self.colours.directory(), f if f.is_executable_file() => self.colours.filekinds.executable, - f if f.is_link() => self.colours.filekinds.symlink, - f if f.is_pipe() => self.colours.filekinds.pipe, - f if f.is_block_device() => self.colours.filekinds.block_device, - f if f.is_char_device() => self.colours.filekinds.char_device, - f if f.is_socket() => self.colours.filekinds.socket, - f if !f.is_file() => self.colours.filekinds.special, + 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, @@ -267,7 +268,7 @@ impl<'a, 'dir> FileName<'a, 'dir> { f if self.exts.is_temp(f) => self.colours.filetypes.temp, f if self.exts.is_compiled(f) => self.colours.filetypes.compiled, - _ => self.colours.filekinds.normal, + _ => self.colours.normal(), } } } From bfb8a5a5733be72a3ad1925f3947da8a778bbc3c Mon Sep 17 00:00:00 2001 From: Benjamin Sago Date: Sat, 26 Aug 2017 20:43:47 +0100 Subject: [PATCH 2/8] Extract trait above file name colours MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/info/filetype.rs | 45 ++++++++++++++----- src/options/colours.rs | 2 +- src/options/view.rs | 15 ++++--- src/output/colours.rs | 44 ++++++------------- src/output/file_name.rs | 91 ++++++++++++++++++++++----------------- src/output/lines.rs | 3 +- xtests/file-names-exts-bw | 6 +++ xtests/file_names_bw | 6 +++ xtests/run.sh | 4 ++ 9 files changed, 127 insertions(+), 89 deletions(-) create mode 100644 xtests/file-names-exts-bw create mode 100644 xtests/file_names_bw diff --git a/src/info/filetype.rs b/src/info/filetype.rs index e608603..a35aa62 100644 --- a/src/info/filetype.rs +++ b/src/info/filetype.rs @@ -4,10 +4,13 @@ //! those are the only metadata that we have access to without reading the //! file’s 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. It’s 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