From 42e3825dafcfc0255471c2213d3c97ddb5a21fa6 Mon Sep 17 00:00:00 2001 From: David Tolnay Date: Mon, 30 Mar 2020 13:18:41 -0700 Subject: [PATCH] Updates for review of PR 899 --- ci/script.bash | 1 + src/config.rs | 5 +++-- src/controller.rs | 41 +++++++++++++++++++++++++++-------------- src/errors.rs | 47 +++++++---------------------------------------- src/output.rs | 12 ++++++------ 5 files changed, 44 insertions(+), 62 deletions(-) diff --git a/ci/script.bash b/ci/script.bash index 0a7a3ee8..c53d62e2 100755 --- a/ci/script.bash +++ b/ci/script.bash @@ -17,3 +17,4 @@ fi cargo check --target "$TARGET" --verbose --lib --no-default-features cargo check --target "$TARGET" --verbose --lib --no-default-features --features git cargo check --target "$TARGET" --verbose --lib --no-default-features --features paging +cargo check --target "$TARGET" --verbose --lib --no-default-features --features git,paging diff --git a/src/config.rs b/src/config.rs index d06c38d6..b0544acd 100644 --- a/src/config.rs +++ b/src/config.rs @@ -5,14 +5,14 @@ pub use crate::syntax_mapping::{MappingTarget, SyntaxMapping}; pub use crate::wrap::OutputWrap; #[derive(Debug, Clone, Copy, PartialEq)] +#[cfg(feature = "paging")] pub enum PagingMode { - #[cfg(feature = "paging")] Always, - #[cfg(feature = "paging")] QuitIfOneScreen, Never, } +#[cfg(feature = "paging")] impl Default for PagingMode { fn default() -> Self { PagingMode::Never @@ -53,6 +53,7 @@ pub struct Config<'a> { pub output_wrap: OutputWrap, /// Pager or STDOUT + #[cfg(feature = "paging")] pub paging_mode: PagingMode, /// Specifies the lines that should be printed diff --git a/src/controller.rs b/src/controller.rs index 90e465bd..fd874759 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -1,8 +1,9 @@ use std::io::{self, Write}; -use std::path::Path; use crate::assets::HighlightingAssets; -use crate::config::{Config, PagingMode}; +use crate::config::Config; +#[cfg(feature = "paging")] +use crate::config::PagingMode; use crate::errors::*; use crate::inputfile::{InputFile, InputFileReader}; use crate::line_range::{LineRanges, RangeCheckResult}; @@ -24,22 +25,34 @@ impl<'b> Controller<'b> { } pub fn run_with_error_handler(&self, handle_error: impl Fn(&Error)) -> Result { - // Do not launch the pager if NONE of the input files exist - let mut paging_mode = self.config.paging_mode; - if self.config.paging_mode != PagingMode::Never { - let call_pager = self.config.files.iter().any(|file| { - if let InputFile::Ordinary(path) = file { - return Path::new(path).exists(); - } else { - return true; + let mut output_type; + + #[cfg(feature = "paging")] + { + use std::path::Path; + + // Do not launch the pager if NONE of the input files exist + let mut paging_mode = self.config.paging_mode; + if self.config.paging_mode != PagingMode::Never { + let call_pager = self.config.files.iter().any(|file| { + if let InputFile::Ordinary(path) = file { + return Path::new(path).exists(); + } else { + return true; + } + }); + if !call_pager { + paging_mode = PagingMode::Never; } - }); - if !call_pager { - paging_mode = PagingMode::Never; } + output_type = OutputType::from_mode(paging_mode, self.config.pager)?; + } + + #[cfg(not(feature = "paging"))] + { + output_type = OutputType::stdout(); } - let mut output_type = OutputType::from_mode(paging_mode, self.config.pager)?; let writer = output_type.handle()?; let mut no_errors: bool = true; diff --git a/src/errors.rs b/src/errors.rs index ff5682b9..91b890d0 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -2,20 +2,20 @@ use error_chain::error_chain; error_chain! { foreign_links { - Clap(clap::Error); - Io(std::io::Error); - SyntectError(syntect::LoadingError); - ParseIntError(std::num::ParseIntError); - GlobParsingError(globset::Error); + Clap(::clap::Error) #[cfg(feature = "application")]; + Io(::std::io::Error); + SyntectError(::syntect::LoadingError); + ParseIntError(::std::num::ParseIntError); + GlobParsingError(::globset::Error); } } pub fn default_error_handler(error: &Error) { match error { Error(ErrorKind::Io(ref io_error), _) - if io_error.kind() == std::io::ErrorKind::BrokenPipe => + if io_error.kind() == ::std::io::ErrorKind::BrokenPipe => { - std::process::exit(0); + ::std::process::exit(0); } _ => { use ansi_term::Colour::Red; @@ -23,36 +23,3 @@ pub fn default_error_handler(error: &Error) { } }; } - -// Mock out a type for clap::Error if we aren't pulling in a dependency on clap. -// -// This can be removed after migrating away from error_chain to some modern -// derive-based error library such as thiserror, in favor of: -// -// #[derive(Error)] -// pub enum Error { -// #[cfg(feature = "application")] -// Clap(clap::Error), -// ... -// } -// -#[cfg(not(feature = "application"))] -mod clap { - use std::fmt::{self, Debug, Display}; - - pub struct Error(()); - - impl Display for Error { - fn fmt(&self, _formatter: &mut fmt::Formatter) -> fmt::Result { - unreachable!() - } - } - - impl Debug for Error { - fn fmt(&self, _formatter: &mut fmt::Formatter) -> fmt::Result { - unreachable!() - } - } - - impl std::error::Error for Error {} -} diff --git a/src/output.rs b/src/output.rs index 1a3f6e88..15f29e43 100644 --- a/src/output.rs +++ b/src/output.rs @@ -2,6 +2,7 @@ use std::io::{self, Write}; #[cfg(feature = "paging")] use std::process::Child; +#[cfg(feature = "paging")] use crate::config::PagingMode; use crate::errors::*; #[cfg(feature = "paging")] @@ -15,13 +16,12 @@ pub enum OutputType { } impl OutputType { + #[cfg(feature = "paging")] pub fn from_mode(mode: PagingMode, pager: Option<&str>) -> Result { - let _ = pager; + use self::PagingMode::*; Ok(match mode { - #[cfg(feature = "paging")] - PagingMode::Always => OutputType::try_pager(false, pager)?, - #[cfg(feature = "paging")] - PagingMode::QuitIfOneScreen => OutputType::try_pager(true, pager)?, + Always => OutputType::try_pager(false, pager)?, + QuitIfOneScreen => OutputType::try_pager(true, pager)?, _ => OutputType::stdout(), }) } @@ -116,7 +116,7 @@ impl OutputType { } } - fn stdout() -> Self { + pub(crate) fn stdout() -> Self { OutputType::Stdout(io::stdout()) }