Fix bug where colours were incorrectly applied

exa assumed that the COLUMNS environment variable being present always meant that the output was to a terminal, so it should use colours. But because this variable can be overridden, colours were being incorrectly set!

The ‘fix’ is to stop trying to be clever while only calculating the terminal width once, and instead just stick it in a lazy_static so it’s usable everywhere.

Cargo.lock

@@ -7,10 +7,11 @@ dependencies = [
  "getopts 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)",
  "git2 0.6.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lazy_static 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
  "locale 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "natord 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)",
- "num_cpus 1.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "num_cpus 1.6.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "number_prefix 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "scoped_threadpool 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)",
  "term_grid 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -39,7 +40,7 @@ name = "cmake"
 version = "0.1.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "gcc 0.3.50 (registry+https://github.com/rust-lang/crates.io-index)",
+ "gcc 0.3.51 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -48,7 +49,7 @@ version = "0.4.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "iso8601 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
  "locale 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "num 0.1.39 (registry+https://github.com/rust-lang/crates.io-index)",
  "pad 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -56,7 +57,7 @@ dependencies = [
 
 [[package]]
 name = "gcc"
-version = "0.3.50"
+version = "0.3.51"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -70,9 +71,9 @@ version = "0.6.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "bitflags 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
  "libgit2-sys 0.6.12 (registry+https://github.com/rust-lang/crates.io-index)",
- "url 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "url 1.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -87,7 +88,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "matches 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "unicode-bidi 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)",
- "unicode-normalization 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
+ "unicode-normalization 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -98,9 +99,14 @@ dependencies = [
  "nom 1.2.4 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "lazy_static"
+version = "0.2.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "libc"
-version = "0.2.23"
+version = "0.2.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -109,20 +115,21 @@ version = "0.6.12"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "cmake 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)",
- "gcc 0.3.50 (registry+https://github.com/rust-lang/crates.io-index)",
+ "gcc 0.3.51 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
- "libz-sys 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libz-sys 1.0.16 (registry+https://github.com/rust-lang/crates.io-index)",
  "pkg-config 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
 name = "libz-sys"
-version = "1.0.13"
+version = "1.0.16"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "gcc 0.3.50 (registry+https://github.com/rust-lang/crates.io-index)",
+ "gcc 0.3.51 (registry+https://github.com/rust-lang/crates.io-index)",
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
  "pkg-config 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
+ "vcpkg 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -130,7 +137,7 @@ name = "locale"
 version = "0.2.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -216,10 +223,10 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "num_cpus"
-version = "1.5.1"
+version = "1.6.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -238,6 +245,11 @@ dependencies = [
  "unicode-width 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "percent-encoding"
+version = "1.0.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "pkg-config"
 version = "0.3.9"
@@ -248,7 +260,7 @@ name = "rand"
 version = "0.3.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -279,7 +291,7 @@ dependencies = [
 
 [[package]]
 name = "unicode-normalization"
-version = "0.1.4"
+version = "0.1.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
@@ -289,11 +301,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "url"
-version = "1.4.1"
+version = "1.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
  "idna 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "matches 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
+ "percent-encoding 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -301,9 +314,14 @@ name = "users"
 version = "0.5.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
- "libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
+[[package]]
+name = "vcpkg"
+version = "0.2.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "zoneinfo_compiled"
 version = "0.2.1"
@@ -319,15 +337,16 @@ dependencies = [
 "checksum byteorder 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "96c8b41881888cc08af32d47ac4edd52bc7fa27fef774be47a92443756451304"
 "checksum cmake 0.1.24 (registry+https://github.com/rust-lang/crates.io-index)" = "b8ebbb35d3dc9cd09497168f33de1acb79b265d350ab0ac34133b98f8509af1f"
 "checksum datetime 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "2d425bf1f6bbd57cf833081c1e60ac294fd74e7edd66acc91c3fca2e496bcee9"
-"checksum gcc 0.3.50 (registry+https://github.com/rust-lang/crates.io-index)" = "5f837c392f2ea61cb1576eac188653df828c861b7137d74ea4a5caa89621f9e6"
+"checksum gcc 0.3.51 (registry+https://github.com/rust-lang/crates.io-index)" = "120d07f202dcc3f72859422563522b66fe6463a4c513df062874daad05f85f0a"
 "checksum getopts 0.2.14 (registry+https://github.com/rust-lang/crates.io-index)" = "d9047cfbd08a437050b363d35ef160452c5fe8ea5187ae0a624708c91581d685"
 "checksum git2 0.6.6 (registry+https://github.com/rust-lang/crates.io-index)" = "aa01936ac96555c083c0e8553f672616274408d9d3fc5b8696603fbf63ff43ee"
 "checksum glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "8be18de09a56b60ed0edf84bc9df007e30040691af7acd1c41874faac5895bfb"
 "checksum idna 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "2233d4940b1f19f0418c158509cd7396b8d70a5db5705ce410914dc8fa603b37"
 "checksum iso8601 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "11dc464f8c6f17595d191447c9c6559298b2d023d6f846a4a23ac7ea3c46c477"
-"checksum libc 0.2.23 (registry+https://github.com/rust-lang/crates.io-index)" = "e7eb6b826bfc1fdea7935d46556250d1799b7fe2d9f7951071f4291710665e3e"
+"checksum lazy_static 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)" = "3b37545ab726dd833ec6420aaba8231c5b320814b9029ad585555d2a03e94fbf"
+"checksum libc 0.2.24 (registry+https://github.com/rust-lang/crates.io-index)" = "38f5c2b18a287cf78b4097db62e20f43cace381dc76ae5c0a3073067f78b7ddc"
 "checksum libgit2-sys 0.6.12 (registry+https://github.com/rust-lang/crates.io-index)" = "df18a822100352d9863b302faf6f8f25c0e77f0e60feb40e5dbe1238b7f13b1d"
-"checksum libz-sys 1.0.13 (registry+https://github.com/rust-lang/crates.io-index)" = "e5ee912a45d686d393d5ac87fac15ba0ba18daae14e8e7543c63ebf7fb7e970c"
+"checksum libz-sys 1.0.16 (registry+https://github.com/rust-lang/crates.io-index)" = "3fdd64ef8ee652185674455c1d450b83cbc8ad895625d543b5324d923f82e4d8"
 "checksum locale 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "5fdbe492a9c0238da900a1165c42fc5067161ce292678a6fe80921f30fe307fd"
 "checksum matches 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "100aabe6b8ff4e4a7e32c1c13523379802df0772b82466207ac25b013f193376"
 "checksum natord 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)" = "308d96db8debc727c3fd9744aac51751243420e46edf401010908da7f8d5e57c"
@@ -339,17 +358,19 @@ dependencies = [
 "checksum num-iter 0.1.33 (registry+https://github.com/rust-lang/crates.io-index)" = "f7d1891bd7b936f12349b7d1403761c8a0b85a18b148e9da4429d5d102c1a41e"
 "checksum num-rational 0.1.38 (registry+https://github.com/rust-lang/crates.io-index)" = "33c881e104a26e1accc09449374c095ff2312c8e0c27fab7bbefe16eac7c776d"
 "checksum num-traits 0.1.39 (registry+https://github.com/rust-lang/crates.io-index)" = "1708c0628602a98b52fad936cf3edb9a107af06e52e49fdf0707e884456a6af6"
-"checksum num_cpus 1.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "6e416ba127a4bb3ff398cb19546a8d0414f73352efe2857f4060d36f5fe5983a"
+"checksum num_cpus 1.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "aec53c34f2d0247c5ca5d32cca1478762f301740468ee9ee6dcb7a0dd7a0c584"
 "checksum number_prefix 0.2.7 (registry+https://github.com/rust-lang/crates.io-index)" = "59a14be9c211cb9c602bad35ac99f41e9a84b44d71b8cbd3040e3bd02a214902"
 "checksum pad 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "d1bf3336e626b898e7263790d432a711d4277e22faea20dd9f70e0cab268fa58"
+"checksum percent-encoding 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "de154f638187706bde41d9b4738748933d64e6b37bdbffc0b47a97d16a6ae356"
 "checksum pkg-config 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)" = "3a8b4c6b8165cd1a1cd4b9b120978131389f64bdaf456435caa41e630edba903"
 "checksum rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)" = "022e0636ec2519ddae48154b028864bdce4eaf7d35226ab8e65c611be97b189d"
 "checksum rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)" = "dcf128d1287d2ea9d80910b5f1120d0b8eede3fbf1abe91c40d39ea7d51e6fda"
 "checksum scoped_threadpool 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "3ef399c8893e8cb7aa9696e895427fab3a6bf265977bb96e126f24ddd2cda85a"
 "checksum term_grid 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "ccc202875496cf72a683a1ecd66f0742a830e73c202bdbd21867d73dfaac8343"
 "checksum unicode-bidi 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "a6a2c4e3710edd365cd7e78383153ed739fa31af19f9172f72d3575060f5a43a"
-"checksum unicode-normalization 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "e28fa37426fceeb5cf8f41ee273faa7c82c47dc8fba5853402841e665fcd86ff"
+"checksum unicode-normalization 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "51ccda9ef9efa3f7ef5d91e8f9b83bbe6955f9bf86aec89d5cce2c874625920f"
 "checksum unicode-width 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "bf3a113775714a22dcb774d8ea3655c53a32debae63a063acc00a91cc586245f"
-"checksum url 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)" = "3e2ba3456fbe5c0098cb877cf08b92b76c3e18e0be9e47c35b487220d377d24e"
+"checksum url 1.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "eeb819346883532a271eb626deb43c4a1bb4c4dd47c519bd78137c3e72a4fe27"
 "checksum users 0.5.2 (registry+https://github.com/rust-lang/crates.io-index)" = "a7ae8fdf783cb9652109c99886459648feb92ecc749e6b8e7930f6decba74c7c"
+"checksum vcpkg 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "9e0a7d8bed3178a8fb112199d466eeca9ed09a14ba8ad67718179b4fd5487d0b"
 "checksum zoneinfo_compiled 0.2.1 (git+https://github.com/rust-datetime/zoneinfo-compiled.git)" = "<none>"

Cargo.toml

@@ -16,6 +16,7 @@ ansi_term = "0.8.0"
 datetime = "0.4.3"
 getopts = "0.2.14"
 glob = "0.2"
+lazy_static = "0.2"
 libc = "0.2.9"
 locale = "0.2.1"
 natord = "1.0.7"

src/exa.rs

@@ -18,6 +18,10 @@ extern crate zoneinfo_compiled;
 
 #[cfg(feature="git")] extern crate git2;
 
+#[macro_use]
+extern crate lazy_static;
+
+
 use std::ffi::OsStr;
 use std::io::{stderr, Write, Result as IOResult};
 use std::path::{Component, Path};

src/options/view.rs

@@ -7,7 +7,6 @@ use output::{Grid, Details, GridDetails};
 use output::column::{Columns, TimeTypes, SizeFormat};
 use output::file_name::Classify;
 use options::{FileFilter, DirAction, Misfire};
-use term::dimensions;
 use fs::feature::xattr;
 
 
@@ -45,10 +44,6 @@ impl Mode {
     pub fn deduce(matches: &getopts::Matches, filter: FileFilter, dir_action: DirAction) -> Result<(Mode, Colours), Misfire> {
         use options::misfire::Misfire::*;
 
-        let colour_scale = || {
-            matches.opt_present("color-scale") || matches.opt_present("colour-scale")
-        };
-
         let long = || {
             if matches.opt_present("across") && !matches.opt_present("grid") {
                 Err(Useless("across", true, "long"))
@@ -57,19 +52,7 @@ impl Mode {
                 Err(Useless("oneline", true, "long"))
             }
             else {
-                let term_colours = TerminalColours::deduce(matches)?;
+                let colours = Colours::deduce(matches)?;
-                let colours = match term_colours {
-                    TerminalColours::Always    => Colours::colourful(colour_scale()),
-                    TerminalColours::Never     => Colours::plain(),
-                    TerminalColours::Automatic => {
-                        if dimensions().is_some() {
-                            Colours::colourful(colour_scale())
-                        }
-                        else {
-                            Colours::plain()
-                        }
-                    },
-                };
 
                 let details = Details {
                     columns: Some(Columns::deduce(matches)?),
@@ -105,15 +88,8 @@ impl Mode {
         };
 
         let other_options_scan = || {
-            let term_colours = TerminalColours::deduce(matches)?;
+            if let Some(width) = TerminalWidth::deduce()?.width() {
-            let term_width   = TerminalWidth::deduce()?;
+                let colours = Colours::deduce(matches)?;
-
-            if let Some(width) = term_width.width() {
-                let colours = match term_colours {
-                    TerminalColours::Always     |
-                    TerminalColours::Automatic  => Colours::colourful(colour_scale()),
-                    TerminalColours::Never      => Colours::plain(),
-                };
 
                 if matches.opt_present("oneline") {
                     if matches.opt_present("across") {
@@ -148,10 +124,7 @@ impl Mode {
                 // as the program’s stdout being connected to a file, then
                 // fallback to the lines view.
 
-                let colours = match term_colours {
+                let colours = Colours::deduce(matches)?;
-                    TerminalColours::Always    => Colours::colourful(colour_scale()),
-                    TerminalColours::Never | TerminalColours::Automatic => Colours::plain(),
-                };
 
                 if matches.opt_present("tree") {
                     let details = Details {
@@ -222,7 +195,7 @@ impl TerminalWidth {
                 Err(e)     => Err(Misfire::FailedParse(e)),
             }
         }
-        else if let Some((width, _)) = dimensions() {
+        else if let Some(width) = *TERM_WIDTH {
             Ok(TerminalWidth::Terminal(width))
         }
         else {
@@ -373,6 +346,22 @@ impl TerminalColours {
 }
 
 
+impl Colours {
+    fn deduce(matches: &getopts::Matches) -> Result<Colours, Misfire> {
+        use self::TerminalColours::*;
+
+        let tc = TerminalColours::deduce(matches)?;
+        if tc == Always || (tc == Automatic && TERM_WIDTH.is_some()) {
+            let scale = matches.opt_present("color-scale") || matches.opt_present("colour-scale");
+            Ok(Colours::colourful(scale))
+        }
+        else {
+            Ok(Colours::plain())
+        }
+    }
+}
+
+
 
 impl Classify {
     fn deduce(matches: &getopts::Matches) -> Classify {
@@ -380,3 +369,14 @@ impl Classify {
                                       else { Classify::JustFilenames }
     }
 }
+
+
+// Gets, then caches, the width of the terminal that exa is running in.
+// This gets used multiple times above, with no real guarantee of order,
+// so it’s easier to just cache it the first time it runs.
+lazy_static! {
+    static ref TERM_WIDTH: Option<usize> = {
+        use term::dimensions;
+        dimensions().map(|t| t.0)
+    };
+}

xtests/run.sh

@@ -29,8 +29,6 @@ $exa $testcases/files -lhB | diff -q - $results/files_lhb2  || exit 1
 $exa $testcases/attributes/dirs/empty-with-attribute -lh | diff -q - $results/empty  || exit 1
 
 $exa --color-scale         $testcases/files -l | diff -q - $results/files_l_scale  || exit 1
-$exa_binary                $testcases/files -l | diff -q - $results/files_l_bw     || exit 1
-$exa_binary --colour=never $testcases/files -l | diff -q - $results/files_l_bw     || exit 1
 
 
 # Grid view tests
@@ -110,6 +108,14 @@ COLUMNS=80 $exa $testcases/links    2>&1 | diff -q - $results/links        || ex
 $exa $testcases/links/* -1 | diff -q - $results/links_1_files || exit 1
 
 
+# Colours and terminals
+# Just because COLUMNS is present, doesn’t mean output is to a terminal
+COLUMNS=80 $exa_binary                    $testcases/files -l | diff -q - $results/files_l_bw  || exit 1
+COLUMNS=80 $exa_binary --colour=always    $testcases/files -l | diff -q - $results/files_l     || exit 1
+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
+
+
 # Git
 $exa $testcases/git/additions -l --git 2>&1 | diff -q - $results/git_additions  || exit 1
 $exa $testcases/git/edits     -l --git 2>&1 | diff -q - $results/git_edits      || exit 1