Finish PlainStatusBackend

Cargo.toml

@@ -43,6 +43,7 @@ vcpkg = "0.2.10"
 
 [dependencies]
 app_dirs = { version = "2", package = "app_dirs2" }
+atty = "0.2"
 byte-unit = "^4.0"
 cfg-if = "0.1"
 structopt = "0.3"

src/bin/tectonic.rs

@@ -13,10 +13,10 @@ use std::time;
 use tectonic::config::PersistentConfig;
 use tectonic::driver::{OutputFormat, PassSetting, ProcessingSessionBuilder};
 use tectonic::errors::{ErrorKind, Result};
+use tectonic::status::plain::PlainStatusBackend;
 use tectonic::status::termcolor::TermcolorStatusBackend;
 use tectonic::status::{ChatterLevel, StatusBackend};
-
-use tectonic::{errmsg, tt_error, tt_error_styled, tt_note};
+use tectonic::{errmsg, tt_error, tt_note};
 
 #[derive(Debug, StructOpt)]
 #[structopt(name = "Tectonic", about = "Process a (La)TeX document")]
@@ -37,11 +37,14 @@ struct CliOptions {
     /// How much chatter to print when running
     #[structopt(long = "chatter", short, name = "level", default_value = "default", possible_values(&["default", "minimal"]))]
     chatter_level: String,
+    /// Enable/disable colorful log output.
+    #[structopt(long = "color", name = "when", default_value = "auto", possible_values(&["always", "auto", "never"]))]
+    cli_color: String,
     /// Use only resource files cached locally
     #[structopt(short = "C", long)]
     only_cached: bool,
     /// The kind of output to generate
-    #[structopt(long, name = "format", default_value = "pdf", possible_values(&["pdf", "html", "xdv", "aux", "format"]))]
+    #[structopt(long, name = "format", default_value = "pdf", possible_values(&["pdf", "html", "xdv", "aux", "fmt"]))]
     outfmt: String,
     /// Write Makefile-format rules expressing the dependencies of this run to <dest_path>
     #[structopt(long, name = "dest_path")]
@@ -71,11 +74,7 @@ struct CliOptions {
     #[structopt(name = "outdir", short, long, parse(from_os_str))]
     outdir: Option<PathBuf>,
 }
-fn inner(
-    args: CliOptions,
-    config: PersistentConfig,
-    status: &mut TermcolorStatusBackend,
-) -> Result<()> {
+fn inner(args: CliOptions, config: PersistentConfig, status: &mut dyn StatusBackend) -> Result<()> {
     let mut sess_builder = ProcessingSessionBuilder::default();
     let format_path = args.format;
     sess_builder
@@ -187,10 +186,8 @@ fn inner(
                     "something bad happened inside {}; its output follows:\n",
                     engine
                 );
-                tt_error_styled!(status, "===============================================================================");
-                status.dump_to_stderr(&output);
-                tt_error_styled!(status, "===============================================================================");
-                tt_error_styled!(status, "");
+
+                status.dump_error_logs(&output);
             }
         }
     }
@@ -235,8 +232,19 @@ fn main() {
     // something I'd be relatively OK with since it'd only affect the progam
     // UI, not the processing results).
 
-    let mut status =
-        TermcolorStatusBackend::new(ChatterLevel::from_str(&args.chatter_level).unwrap());
+    let chatter_level = ChatterLevel::from_str(&args.chatter_level).unwrap();
+    let use_cli_color = match &*args.cli_color {
+        "always" => true,
+        "auto" => atty::is(atty::Stream::Stdout),
+        "never" => false,
+        _ => unreachable!(),
+    };
+
+    let mut status = if use_cli_color {
+        Box::new(TermcolorStatusBackend::new(chatter_level)) as Box<dyn StatusBackend>
+    } else {
+        Box::new(PlainStatusBackend::new(chatter_level)) as Box<dyn StatusBackend>
+    };
 
     // For now ...
 
@@ -249,8 +257,8 @@ fn main() {
     // function ... all so that we can print out the word "error:" in red.
     // This code parallels various bits of the `error_chain` crate.
 
-    if let Err(ref e) = inner(args, config, &mut status) {
-        status.bare_error(e);
+    if let Err(ref e) = inner(args, config, &mut *status) {
+        tt_error!(status, ""; e);
         process::exit(1)
     }
 }

src/driver.rs

@@ -633,7 +633,7 @@ impl ProcessingSession {
     /// Assess whether we need to rerun an engine. This is the case if there
     /// was a file that the engine read and then rewrote, and the rewritten
     /// version is different than the version that it read in.
-    fn is_rerun_needed<S: StatusBackend>(&self, status: &mut S) -> Option<RerunReason> {
+    fn is_rerun_needed(&self, status: &mut dyn StatusBackend) -> Option<RerunReason> {
         // TODO: we should probably wire up diagnostics since I expect this
         // stuff could get finicky and we're going to want to be able to
         // figure out why rerun detection is breaking.
@@ -664,7 +664,7 @@ impl ProcessingSession {
     }
 
     #[allow(dead_code)]
-    fn _dump_access_info<S: StatusBackend>(&self, status: &mut S) {
+    fn _dump_access_info(&self, status: &mut dyn StatusBackend) {
         for (name, info) in &self.events.0 {
             if info.access_pattern != AccessPattern::Read {
                 let r = match info.read_digest {
@@ -697,7 +697,7 @@ impl ProcessingSession {
     /// - run BibTeX, if it seems to be required
     /// - repeat the last two steps as often as needed
     /// - write the output files to disk, including a Makefile if it was requested.
-    pub fn run<S: StatusBackend>(&mut self, status: &mut S) -> Result<()> {
+    pub fn run(&mut self, status: &mut dyn StatusBackend) -> Result<()> {
         // Do we need to generate the format file?
 
         let generate_format = if self.output_format == OutputFormat::Format {
@@ -817,10 +817,10 @@ impl ProcessingSession {
         Ok(())
     }
 
-    fn write_files<S: StatusBackend>(
+    fn write_files(
         &mut self,
         mut mf_dest_maybe: Option<&mut File>,
-        status: &mut S,
+        status: &mut dyn StatusBackend,
         only_logs: bool,
     ) -> Result<u32> {
         let root = match self.output_path {
@@ -909,11 +909,7 @@ impl ProcessingSession {
 
     /// The "default" pass really runs a bunch of sub-passes. It is a "Do What
     /// I Mean" operation.
-    fn default_pass<S: StatusBackend>(
-        &mut self,
-        bibtex_first: bool,
-        status: &mut S,
-    ) -> Result<i32> {
+    fn default_pass(&mut self, bibtex_first: bool, status: &mut dyn StatusBackend) -> Result<i32> {
         // If `bibtex_first` is true, we start by running bibtex, and run
         // proceed with the standard rerun logic. Otherwise, we run TeX,
         // auto-detect whether we need to run bibtex, possibly run it, and
@@ -1011,7 +1007,7 @@ impl ProcessingSession {
     }
 
     /// Use the TeX engine to generate a format file.
-    fn make_format_pass<S: StatusBackend>(&mut self, status: &mut S) -> Result<i32> {
+    fn make_format_pass(&mut self, status: &mut dyn StatusBackend) -> Result<i32> {
         if self.io.bundle.is_none() {
             return Err(
                 ErrorKind::Msg("cannot create formats without using a bundle".to_owned()).into(),
@@ -1089,10 +1085,10 @@ impl ProcessingSession {
     }
 
     /// Run one pass of the TeX engine.
-    fn tex_pass<S: StatusBackend>(
+    fn tex_pass(
         &mut self,
         rerun_explanation: Option<&str>,
-        status: &mut S,
+        status: &mut dyn StatusBackend,
     ) -> Result<Option<&'static str>> {
         let result = {
             let mut stack = self.io.as_stack();
@@ -1131,7 +1127,7 @@ impl ProcessingSession {
         Ok(warnings)
     }
 
-    fn bibtex_pass<S: StatusBackend>(&mut self, status: &mut S) -> Result<i32> {
+    fn bibtex_pass(&mut self, status: &mut dyn StatusBackend) -> Result<i32> {
         let result = {
             let mut stack = self.io.as_stack();
             let mut engine = BibtexEngine::new();
@@ -1167,7 +1163,7 @@ impl ProcessingSession {
         Ok(0)
     }
 
-    fn xdvipdfmx_pass<S: StatusBackend>(&mut self, status: &mut S) -> Result<i32> {
+    fn xdvipdfmx_pass(&mut self, status: &mut dyn StatusBackend) -> Result<i32> {
         {
             let mut stack = self.io.as_stack();
             let mut engine = XdvipdfmxEngine::new().with_date(self.build_date);
@@ -1185,7 +1181,7 @@ impl ProcessingSession {
         Ok(0)
     }
 
-    fn spx2html_pass<S: StatusBackend>(&mut self, status: &mut S) -> Result<i32> {
+    fn spx2html_pass(&mut self, status: &mut dyn StatusBackend) -> Result<i32> {
         {
             let mut stack = self.io.as_stack();
             let mut engine = Spx2HtmlEngine::new();

src/status/mod.rs

@@ -4,6 +4,7 @@
 
 //! A framework for showing status messages to the user.
 
+pub mod plain;
 pub mod termcolor;
 
 use std::cmp;
@@ -77,6 +78,10 @@ pub trait StatusBackend {
             None,
         )
     }
+
+    /// This is used to print TeX engine logs after it encountered errors. This prints the log,
+    /// surrounded by lines of equal signs.
+    fn dump_error_logs(&mut self, output: &[u8]);
 }
 
 /// Report a formatted informational message to the user.
@@ -133,4 +138,5 @@ impl NoopStatusBackend {
 
 impl StatusBackend for NoopStatusBackend {
     fn report(&mut self, _kind: MessageKind, _args: Arguments, _err: Option<&Error>) {}
+    fn dump_error_logs(&mut self, _output: &[u8]) {}
 }

src/status/plain.rs

@@ -0,0 +1,67 @@
+use std::fmt::Arguments;
+
+use super::{ChatterLevel, MessageKind, StatusBackend};
+use crate::errors::Error;
+use std::io::{self, Write};
+
+pub struct PlainStatusBackend {
+    chatter: ChatterLevel,
+}
+
+impl PlainStatusBackend {
+    pub fn new(chatter: ChatterLevel) -> Self {
+        PlainStatusBackend { chatter }
+    }
+}
+
+impl StatusBackend for PlainStatusBackend {
+    fn report(&mut self, kind: MessageKind, args: Arguments, err: Option<&Error>) {
+        if kind == MessageKind::Note && self.chatter <= ChatterLevel::Minimal {
+            return;
+        }
+
+        let prefix = match kind {
+            MessageKind::Note => "note:",
+            MessageKind::Warning => "warning:",
+            MessageKind::Error => "error:",
+        };
+        if kind == MessageKind::Note {
+            println!("{} {}", prefix, args);
+        } else {
+            eprintln!("{} {}", prefix, args);
+        }
+        if let Some(e) = err {
+            for item in e.iter() {
+                eprintln!("caused by: {}", item);
+            }
+            if let Some(backtrace) = e.backtrace() {
+                eprintln!("debugging: backtrace follows:");
+                eprintln!("{:?}", backtrace);
+            }
+        }
+    }
+
+    fn note_highlighted(&mut self, before: &str, highlighted: &str, after: &str) {
+        if self.chatter > ChatterLevel::Minimal {
+            self.report(
+                MessageKind::Note,
+                format_args!("{}{}{}", before, highlighted, after),
+                None,
+            );
+        }
+    }
+
+    fn dump_error_logs(&mut self, output: &[u8]) {
+        eprintln!(
+            "==============================================================================="
+        );
+
+        io::stderr()
+            .write_all(output)
+            .expect("write to stderr failed");
+
+        eprintln!(
+            "==============================================================================="
+        );
+    }
+}

src/status/termcolor.rs

@@ -137,12 +137,6 @@ impl TermcolorStatusBackend {
             });
         }
     }
-
-    pub fn dump_to_stderr(&mut self, output: &[u8]) {
-        self.stderr
-            .write_all(output)
-            .expect("write to stderr failed");
-    }
 }
 
 /// Show formatted text to the user, styled as an error message.
@@ -185,4 +179,20 @@ impl StatusBackend for TermcolorStatusBackend {
             writeln!(self.stdout, "{}", after).expect("write to stdout failed");
         }
     }
+
+    fn dump_error_logs(&mut self, output: &[u8]) {
+        tt_error_styled!(
+            self,
+            "==============================================================================="
+        );
+
+        self.stderr
+            .write_all(output)
+            .expect("write to stderr failed");
+
+        tt_error_styled!(
+            self,
+            "==============================================================================="
+        );
+    }
 }

tests/executable.rs

@@ -355,3 +355,33 @@ fn test_keep_logs_on_error() {
 
     assert!(log.contains(r"job aborted, no legal \end found"));
 }
+
+#[test]
+fn test_no_color() {
+    if env::var("RUNNING_COVERAGE").is_ok() {
+        return;
+    }
+
+    // No input files here, but output files are created.
+    let fmt_arg = get_plain_format_arg();
+
+    let tempdir = setup_and_copy_files(&[]);
+    let output_nocolor = run_tectonic_with_stdin(
+        tempdir.path(),
+        &[&fmt_arg, "-", "--color=never"],
+        "no end to this file",
+    );
+
+    // Output is not a terminal, so these two should be the same
+    let tempdir = setup_and_copy_files(&[]);
+    let output_autocolor = run_tectonic_with_stdin(
+        tempdir.path(),
+        &[&fmt_arg, "-", "--color=auto"],
+        "no end to this file",
+    );
+
+    assert_eq!(output_nocolor, output_autocolor);
+
+    error_or_panic(output_nocolor);
+    error_or_panic(output_autocolor);
+}