From 19a08bee8a99e8c725b9781568013a9f686605a3 Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Wed, 20 Sep 2023 14:42:03 -0400 Subject: [PATCH] cli: clean-up crate This does a variety of polishing. 1. Deprecate the tty methods in favor of std's IsTerminal trait. 2. Trim down un-needed dependencies. 3. Use bstr to implement escaping. 4. Various aesthetic polishing. I'm doing this as prep work before adding more to this crate. And as part of a general effort toward reducing ripgrep's dependencies. --- Cargo.lock | 7 +- crates/cli/Cargo.toml | 13 ++-- crates/cli/src/decompress.rs | 54 ++++++------- crates/cli/src/escape.rs | 121 +---------------------------- crates/cli/src/human.rs | 62 ++++++--------- crates/cli/src/lib.rs | 116 ++++++++++++++++----------- crates/cli/src/pattern.rs | 25 ++---- crates/cli/src/process.rs | 33 ++++---- crates/cli/src/wtr.rs | 9 +-- crates/core/args.rs | 8 +- crates/grep/examples/simplegrep.rs | 23 +++--- 11 files changed, 165 insertions(+), 306 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0527cc8c..2f30fbfd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -187,10 +187,7 @@ version = "0.1.9" dependencies = [ "bstr", "globset", - "lazy_static", "log", - "regex", - "same-file", "termcolor", "winapi-util", ] @@ -612,9 +609,9 @@ checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" [[package]] name = "winapi-util" -version = "0.1.5" +version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "70ec6ce85bb158151cae5e5c87f95a8e97d2c0c4b001223f33a334e3ce5de178" +checksum = "f29e6f9198ba0d26b4c9f07dbe6f9ed633e1f3d5b8b414090084349e46a52596" dependencies = [ "winapi", ] diff --git a/crates/cli/Cargo.toml b/crates/cli/Cargo.toml index 5226b762..0ce69873 100644 --- a/crates/cli/Cargo.toml +++ b/crates/cli/Cargo.toml @@ -11,16 +11,13 @@ repository = "https://github.com/BurntSushi/ripgrep/tree/master/crates/cli" readme = "README.md" keywords = ["regex", "grep", "cli", "utility", "util"] license = "Unlicense OR MIT" -edition = "2018" +edition = "2021" [dependencies] -bstr = "1.6.0" +bstr = { version = "1.6.2", features = ["std"] } globset = { version = "0.4.10", path = "../globset" } -lazy_static = "1.1.0" -log = "0.4.5" -regex = "1.1" -same-file = "1.0.4" -termcolor = "1.0.4" +log = "0.4.20" +termcolor = "1.3.0" [target.'cfg(windows)'.dependencies.winapi-util] -version = "0.1.1" +version = "0.1.6" diff --git a/crates/cli/src/decompress.rs b/crates/cli/src/decompress.rs index 72eefdda..9e93c982 100644 --- a/crates/cli/src/decompress.rs +++ b/crates/cli/src/decompress.rs @@ -1,8 +1,10 @@ -use std::ffi::{OsStr, OsString}; -use std::fs::File; -use std::io; -use std::path::{Path, PathBuf}; -use std::process::Command; +use std::{ + ffi::{OsStr, OsString}, + fs::File, + io, + path::{Path, PathBuf}, + process::Command, +}; use globset::{Glob, GlobSet, GlobSetBuilder}; @@ -161,7 +163,7 @@ impl DecompressionMatcher { /// Create a new matcher with default rules. /// /// To add more matching rules, build a matcher with - /// [`DecompressionMatcherBuilder`](struct.DecompressionMatcherBuilder.html). + /// [`DecompressionMatcherBuilder`]. pub fn new() -> DecompressionMatcher { DecompressionMatcherBuilder::new() .build() @@ -221,9 +223,8 @@ impl DecompressionReaderBuilder { path: P, ) -> Result { let path = path.as_ref(); - let mut cmd = match self.matcher.command(path) { - None => return DecompressionReader::new_passthru(path), - Some(cmd) => cmd, + let Some(mut cmd) = self.matcher.command(path) else { + return DecompressionReader::new_passthru(path); }; cmd.arg(path); @@ -302,9 +303,7 @@ impl DecompressionReaderBuilder { /// The default matching rules are probably good enough for most cases, and if /// they require revision, pull requests are welcome. In cases where they must /// be changed or extended, they can be customized through the use of -/// [`DecompressionMatcherBuilder`](struct.DecompressionMatcherBuilder.html) -/// and -/// [`DecompressionReaderBuilder`](struct.DecompressionReaderBuilder.html). +/// [`DecompressionMatcherBuilder`] and [`DecompressionReaderBuilder`]. /// /// By default, this reader will asynchronously read the processes' stderr. /// This prevents subtle deadlocking bugs for noisy processes that write a lot @@ -320,15 +319,14 @@ impl DecompressionReaderBuilder { /// matcher. /// /// ```no_run -/// use std::io::Read; -/// use std::process::Command; +/// use std::{io::Read, process::Command}; +/// /// use grep_cli::DecompressionReader; /// -/// # fn example() -> Result<(), Box<::std::error::Error>> { /// let mut rdr = DecompressionReader::new("/usr/share/man/man1/ls.1.gz")?; /// let mut contents = vec![]; /// rdr.read_to_end(&mut contents)?; -/// # Ok(()) } +/// # Ok::<(), Box>(()) /// ``` #[derive(Debug)] pub struct DecompressionReader { @@ -347,9 +345,7 @@ impl DecompressionReader { /// /// This uses the default matching rules for determining how to decompress /// the given file. To change those matching rules, use - /// [`DecompressionReaderBuilder`](struct.DecompressionReaderBuilder.html) - /// and - /// [`DecompressionMatcherBuilder`](struct.DecompressionMatcherBuilder.html). + /// [`DecompressionReaderBuilder`] and [`DecompressionMatcherBuilder`]. /// /// When creating readers for many paths. it is better to use the builder /// since it will amortize the cost of constructing the matcher. @@ -453,10 +449,7 @@ fn try_resolve_binary>( use std::env; fn is_exe(path: &Path) -> bool { - let md = match path.metadata() { - Err(_) => return false, - Ok(md) => md, - }; + let Ok(md) = path.metadata() else { return false }; !md.is_dir() } @@ -464,15 +457,12 @@ fn try_resolve_binary>( if prog.is_absolute() { return Ok(prog.to_path_buf()); } - let syspaths = match env::var_os("PATH") { - Some(syspaths) => syspaths, - None => { - let msg = "system PATH environment variable not found"; - return Err(CommandError::io(io::Error::new( - io::ErrorKind::Other, - msg, - ))); - } + let Some(syspaths) = env::var_os("PATH") else { + let msg = "system PATH environment variable not found"; + return Err(CommandError::io(io::Error::new( + io::ErrorKind::Other, + msg, + ))); }; for syspath in env::split_paths(&syspaths) { if syspath.as_os_str().is_empty() { diff --git a/crates/cli/src/escape.rs b/crates/cli/src/escape.rs index 6d06abb5..9b442343 100644 --- a/crates/cli/src/escape.rs +++ b/crates/cli/src/escape.rs @@ -1,21 +1,7 @@ use std::ffi::OsStr; -use std::str; use bstr::{ByteSlice, ByteVec}; -/// A single state in the state machine used by `unescape`. -#[derive(Clone, Copy, Eq, PartialEq)] -enum State { - /// The state after seeing a `\`. - Escape, - /// The state after seeing a `\x`. - HexFirst, - /// The state after seeing a `\x[0-9A-Fa-f]`. - HexSecond(char), - /// Default state. - Literal, -} - /// Escapes arbitrary bytes into a human readable string. /// /// This converts `\t`, `\r` and `\n` into their escaped forms. It also @@ -38,17 +24,7 @@ enum State { /// assert_eq!(r"foo\nbar\xFFbaz", escape(b"foo\nbar\xFFbaz")); /// ``` pub fn escape(bytes: &[u8]) -> String { - let mut escaped = String::new(); - for (s, e, ch) in bytes.char_indices() { - if ch == '\u{FFFD}' { - for b in bytes[s..e].bytes() { - escape_byte(b, &mut escaped); - } - } else { - escape_char(ch, &mut escaped); - } - } - escaped + bytes.escape_bytes().to_string() } /// Escapes an OS string into a human readable string. @@ -89,76 +65,7 @@ pub fn escape_os(string: &OsStr) -> String { /// assert_eq!(&b"foo\nbar\xFFbaz"[..], &*unescape(r"foo\nbar\xFFbaz")); /// ``` pub fn unescape(s: &str) -> Vec { - use self::State::*; - - let mut bytes = vec![]; - let mut state = Literal; - for c in s.chars() { - match state { - Escape => match c { - '\\' => { - bytes.push(b'\\'); - state = Literal; - } - 'n' => { - bytes.push(b'\n'); - state = Literal; - } - 'r' => { - bytes.push(b'\r'); - state = Literal; - } - 't' => { - bytes.push(b'\t'); - state = Literal; - } - 'x' => { - state = HexFirst; - } - c => { - bytes.extend(format!(r"\{}", c).into_bytes()); - state = Literal; - } - }, - HexFirst => match c { - '0'..='9' | 'A'..='F' | 'a'..='f' => { - state = HexSecond(c); - } - c => { - bytes.extend(format!(r"\x{}", c).into_bytes()); - state = Literal; - } - }, - HexSecond(first) => match c { - '0'..='9' | 'A'..='F' | 'a'..='f' => { - let ordinal = format!("{}{}", first, c); - let byte = u8::from_str_radix(&ordinal, 16).unwrap(); - bytes.push(byte); - state = Literal; - } - c => { - let original = format!(r"\x{}{}", first, c); - bytes.extend(original.into_bytes()); - state = Literal; - } - }, - Literal => match c { - '\\' => { - state = Escape; - } - c => { - bytes.extend(c.to_string().as_bytes()); - } - }, - } - } - match state { - Escape => bytes.push(b'\\'), - HexFirst => bytes.extend(b"\\x"), - HexSecond(c) => bytes.extend(format!("\\x{}", c).into_bytes()), - Literal => {} - } - bytes + Vec::unescape_bytes(s) } /// Unescapes an OS string. @@ -171,27 +78,6 @@ pub fn unescape_os(string: &OsStr) -> Vec { unescape(&string.to_string_lossy()) } -/// Adds the given codepoint to the given string, escaping it if necessary. -fn escape_char(cp: char, into: &mut String) { - if cp.is_ascii() { - escape_byte(cp as u8, into); - } else { - into.push(cp); - } -} - -/// Adds the given byte to the given string, escaping it if necessary. -fn escape_byte(byte: u8, into: &mut String) { - match byte { - 0x21..=0x5B | 0x5D..=0x7D => into.push(byte as char), - b'\n' => into.push_str(r"\n"), - b'\r' => into.push_str(r"\r"), - b'\t' => into.push_str(r"\t"), - b'\\' => into.push_str(r"\\"), - _ => into.push_str(&format!(r"\x{:02X}", byte)), - } -} - #[cfg(test)] mod tests { use super::{escape, unescape}; @@ -215,7 +101,8 @@ mod tests { #[test] fn nul() { assert_eq!(b(b"\x00"), unescape(r"\x00")); - assert_eq!(r"\x00", escape(b"\x00")); + assert_eq!(b(b"\x00"), unescape(r"\0")); + assert_eq!(r"\0", escape(b"\x00")); } #[test] diff --git a/crates/cli/src/human.rs b/crates/cli/src/human.rs index ba8bf08a..61b430de 100644 --- a/crates/cli/src/human.rs +++ b/crates/cli/src/human.rs @@ -1,10 +1,3 @@ -use std::error; -use std::fmt; -use std::io; -use std::num::ParseIntError; - -use regex::Regex; - /// An error that occurs when parsing a human readable size description. /// /// This error provides an end user friendly message describing why the @@ -18,7 +11,7 @@ pub struct ParseSizeError { #[derive(Clone, Debug, Eq, PartialEq)] enum ParseSizeErrorKind { InvalidFormat, - InvalidInt(ParseIntError), + InvalidInt(std::num::ParseIntError), Overflow, } @@ -30,7 +23,7 @@ impl ParseSizeError { } } - fn int(original: &str, err: ParseIntError) -> ParseSizeError { + fn int(original: &str, err: std::num::ParseIntError) -> ParseSizeError { ParseSizeError { original: original.to_string(), kind: ParseSizeErrorKind::InvalidInt(err), @@ -45,22 +38,18 @@ impl ParseSizeError { } } -impl error::Error for ParseSizeError { - fn description(&self) -> &str { - "invalid size" - } -} +impl std::error::Error for ParseSizeError {} -impl fmt::Display for ParseSizeError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl std::fmt::Display for ParseSizeError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { use self::ParseSizeErrorKind::*; match self.kind { InvalidFormat => write!( f, - "invalid format for size '{}', which should be a sequence \ - of digits followed by an optional 'K', 'M' or 'G' \ - suffix", + "invalid format for size '{}', which should be a non-empty \ + sequence of digits followed by an optional 'K', 'M' or 'G' \ + suffix", self.original ), InvalidInt(ref err) => write!( @@ -73,9 +62,9 @@ impl fmt::Display for ParseSizeError { } } -impl From for io::Error { - fn from(size_err: ParseSizeError) -> io::Error { - io::Error::new(io::ErrorKind::Other, size_err) +impl From for std::io::Error { + fn from(size_err: ParseSizeError) -> std::io::Error { + std::io::Error::new(std::io::ErrorKind::Other, size_err) } } @@ -88,29 +77,24 @@ impl From for io::Error { /// /// Additional suffixes may be added over time. pub fn parse_human_readable_size(size: &str) -> Result { - lazy_static::lazy_static! { - // Normally I'd just parse something this simple by hand to avoid the - // regex dep, but we bring regex in any way for glob matching, so might - // as well use it. - static ref RE: Regex = Regex::new(r"^([0-9]+)([KMG])?$").unwrap(); + let digits_end = + size.as_bytes().iter().take_while(|&b| b.is_ascii_digit()).count(); + let digits = &size[..digits_end]; + if digits.is_empty() { + return Err(ParseSizeError::format(size)); } + let value = + digits.parse::().map_err(|e| ParseSizeError::int(size, e))?; - let caps = match RE.captures(size) { - Some(caps) => caps, - None => return Err(ParseSizeError::format(size)), - }; - let value: u64 = - caps[1].parse().map_err(|err| ParseSizeError::int(size, err))?; - let suffix = match caps.get(2) { - None => return Ok(value), - Some(cap) => cap.as_str(), - }; + let suffix = &size[digits_end..]; + if suffix.is_empty() { + return Ok(value); + } let bytes = match suffix { "K" => value.checked_mul(1 << 10), "M" => value.checked_mul(1 << 20), "G" => value.checked_mul(1 << 30), - // Because if the regex matches this group, it must be [KMG]. - _ => unreachable!(), + _ => return Err(ParseSizeError::format(size)), }; bytes.ok_or_else(|| ParseSizeError::overflow(size)) } diff --git a/crates/cli/src/lib.rs b/crates/cli/src/lib.rs index 53b4d2c3..a16d4c7d 100644 --- a/crates/cli/src/lib.rs +++ b/crates/cli/src/lib.rs @@ -11,27 +11,11 @@ and Linux. # Standard I/O -The -[`is_readable_stdin`](fn.is_readable_stdin.html), -[`is_tty_stderr`](fn.is_tty_stderr.html), -[`is_tty_stdin`](fn.is_tty_stdin.html) -and -[`is_tty_stdout`](fn.is_tty_stdout.html) -routines query aspects of standard I/O. `is_readable_stdin` determines whether -stdin can be usefully read from, while the `tty` methods determine whether a -tty is attached to stdin/stdout/stderr. - -`is_readable_stdin` is useful when writing an application that changes behavior -based on whether the application was invoked with data on stdin. For example, -`rg foo` might recursively search the current working directory for -occurrences of `foo`, but `rg foo < file` might only search the contents of -`file`. - -The `tty` methods are useful for similar reasons. Namely, commands like `ls` -will change their output depending on whether they are printing to a terminal -or not. For example, `ls` shows a file on each line when stdout is redirected -to a file or a pipe, but condenses the output to show possibly many files on -each line when stdout is connected to a tty. +[`is_readable_stdin`] determines whether stdin can be usefully read from. It +is useful when writing an application that changes behavior based on whether +the application was invoked with data on stdin. For example, `rg foo` might +recursively search the current working directory for occurrences of `foo`, but +`rg foo < file` might only search the contents of `file`. # Coloring and buffering @@ -165,21 +149,21 @@ mod pattern; mod process; mod wtr; -use std::io::IsTerminal; - -pub use crate::decompress::{ - resolve_binary, DecompressionMatcher, DecompressionMatcherBuilder, - DecompressionReader, DecompressionReaderBuilder, -}; -pub use crate::escape::{escape, escape_os, unescape, unescape_os}; -pub use crate::human::{parse_human_readable_size, ParseSizeError}; -pub use crate::pattern::{ - pattern_from_bytes, pattern_from_os, patterns_from_path, - patterns_from_reader, patterns_from_stdin, InvalidPatternError, -}; -pub use crate::process::{CommandError, CommandReader, CommandReaderBuilder}; -pub use crate::wtr::{ - stdout, stdout_buffered_block, stdout_buffered_line, StandardStream, +pub use crate::{ + decompress::{ + resolve_binary, DecompressionMatcher, DecompressionMatcherBuilder, + DecompressionReader, DecompressionReaderBuilder, + }, + escape::{escape, escape_os, unescape, unescape_os}, + human::{parse_human_readable_size, ParseSizeError}, + pattern::{ + pattern_from_bytes, pattern_from_os, patterns_from_path, + patterns_from_reader, patterns_from_stdin, InvalidPatternError, + }, + process::{CommandError, CommandReader, CommandReaderBuilder}, + wtr::{ + stdout, stdout_buffered_block, stdout_buffered_line, StandardStream, + }, }; /// Returns true if and only if stdin is believed to be readable. @@ -189,34 +173,60 @@ pub use crate::wtr::{ /// might search the current directory for occurrences of `foo` where as /// `command foo < some-file` or `cat some-file | command foo` might instead /// only search stdin for occurrences of `foo`. +/// +/// Note that this isn't perfect and essentially corresponds to a heuristic. +/// When things are unclear (such as if an error occurs during introspection to +/// determine whether stdin is readable), this prefers to return `false`. That +/// means it's possible for an end user to pipe something into your program and +/// have this return `false` and thus potentially lead to ignoring the user's +/// stdin data. While not ideal, this is perhaps better than falsely assuming +/// stdin is readable, which would result in blocking forever on reading stdin. +/// Regardless, commands should always provide explicit fallbacks to override +/// behavior. For example, `rg foo -` will explicitly search stdin and `rg foo +/// ./` will explicitly search the current working directory. pub fn is_readable_stdin() -> bool { + use std::io::IsTerminal; + #[cfg(unix)] fn imp() -> bool { - use same_file::Handle; - use std::os::unix::fs::FileTypeExt; - - let ft = match Handle::stdin().and_then(|h| h.as_file().metadata()) { - Err(_) => return false, - Ok(md) => md.file_type(), + use std::{ + fs::File, + os::{fd::AsFd, unix::fs::FileTypeExt}, }; + + let stdin = std::io::stdin(); + let Ok(fd) = stdin.as_fd().try_clone_to_owned() else { return false }; + let file = File::from(fd); + let Ok(md) = file.metadata() else { return false }; + let ft = md.file_type(); ft.is_file() || ft.is_fifo() || ft.is_socket() } #[cfg(windows)] fn imp() -> bool { - use winapi_util as winutil; - - winutil::file::typ(winutil::HandleRef::stdin()) + winapi_util::file::typ(winapi_util::HandleRef::stdin()) .map(|t| t.is_disk() || t.is_pipe()) .unwrap_or(false) } - !is_tty_stdin() && imp() + #[cfg(not(any(unix, windows)))] + fn imp() -> bool { + false + } + + !std::io::stdin().is_terminal() && imp() } /// Returns true if and only if stdin is believed to be connected to a tty /// or a console. +/// +/// Note that this is now just a wrapper around +/// [`std::io::IsTerminal`](https://doc.rust-lang.org/std/io/trait.IsTerminal.html). +/// Callers should prefer using the `IsTerminal` trait directly. This routine +/// is deprecated and will be removed in the next semver incompatible release. +#[deprecated(since = "0.1.10", note = "use std::io::IsTerminal instead")] pub fn is_tty_stdin() -> bool { + use std::io::IsTerminal; std::io::stdin().is_terminal() } @@ -228,12 +238,26 @@ pub fn is_tty_stdin() -> bool { /// terminal or whether it's being redirected somewhere else. For example, /// implementations of `ls` will often show one item per line when stdout is /// redirected, but will condensed output when printing to a tty. +/// +/// Note that this is now just a wrapper around +/// [`std::io::IsTerminal`](https://doc.rust-lang.org/std/io/trait.IsTerminal.html). +/// Callers should prefer using the `IsTerminal` trait directly. This routine +/// is deprecated and will be removed in the next semver incompatible release. +#[deprecated(since = "0.1.10", note = "use std::io::IsTerminal instead")] pub fn is_tty_stdout() -> bool { + use std::io::IsTerminal; std::io::stdout().is_terminal() } /// Returns true if and only if stderr is believed to be connected to a tty /// or a console. +/// +/// Note that this is now just a wrapper around +/// [`std::io::IsTerminal`](https://doc.rust-lang.org/std/io/trait.IsTerminal.html). +/// Callers should prefer using the `IsTerminal` trait directly. This routine +/// is deprecated and will be removed in the next semver incompatible release. +#[deprecated(since = "0.1.10", note = "use std::io::IsTerminal instead")] pub fn is_tty_stderr() -> bool { + use std::io::IsTerminal; std::io::stderr().is_terminal() } diff --git a/crates/cli/src/pattern.rs b/crates/cli/src/pattern.rs index 9662d526..f2466882 100644 --- a/crates/cli/src/pattern.rs +++ b/crates/cli/src/pattern.rs @@ -1,10 +1,4 @@ -use std::error; -use std::ffi::OsStr; -use std::fmt; -use std::fs::File; -use std::io; -use std::path::Path; -use std::str; +use std::{ffi::OsStr, io, path::Path}; use bstr::io::BufReadExt; @@ -28,14 +22,10 @@ impl InvalidPatternError { } } -impl error::Error for InvalidPatternError { - fn description(&self) -> &str { - "invalid pattern" - } -} +impl std::error::Error for InvalidPatternError {} -impl fmt::Display for InvalidPatternError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl std::fmt::Display for InvalidPatternError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, "found invalid UTF-8 in pattern at byte offset {}: {} \ @@ -77,7 +67,7 @@ pub fn pattern_from_os(pattern: &OsStr) -> Result<&str, InvalidPatternError> { pub fn pattern_from_bytes( pattern: &[u8], ) -> Result<&str, InvalidPatternError> { - str::from_utf8(pattern).map_err(|err| InvalidPatternError { + std::str::from_utf8(pattern).map_err(|err| InvalidPatternError { original: escape(pattern), valid_up_to: err.valid_up_to(), }) @@ -91,7 +81,7 @@ pub fn pattern_from_bytes( /// path. pub fn patterns_from_path>(path: P) -> io::Result> { let path = path.as_ref(); - let file = File::open(path).map_err(|err| { + let file = std::fs::File::open(path).map_err(|err| { io::Error::new( io::ErrorKind::Other, format!("{}: {}", path.display(), err), @@ -135,7 +125,6 @@ pub fn patterns_from_stdin() -> io::Result> { /// ``` /// use grep_cli::patterns_from_reader; /// -/// # fn example() -> Result<(), Box<::std::error::Error>> { /// let patterns = "\ /// foo /// bar\\s+foo @@ -147,7 +136,7 @@ pub fn patterns_from_stdin() -> io::Result> { /// r"bar\s+foo", /// r"[a-z]{3}", /// ]); -/// # Ok(()) } +/// # Ok::<(), Box>(()) /// ``` pub fn patterns_from_reader(rdr: R) -> io::Result> { let mut patterns = vec![]; diff --git a/crates/cli/src/process.rs b/crates/cli/src/process.rs index 4280b07a..11e02566 100644 --- a/crates/cli/src/process.rs +++ b/crates/cli/src/process.rs @@ -1,9 +1,7 @@ -use std::error; -use std::fmt; -use std::io::{self, Read}; -use std::iter; -use std::process; -use std::thread::{self, JoinHandle}; +use std::{ + io::{self, Read}, + process, +}; /// An error that can occur while running a command and reading its output. /// @@ -40,14 +38,10 @@ impl CommandError { } } -impl error::Error for CommandError { - fn description(&self) -> &str { - "command error" - } -} +impl std::error::Error for CommandError {} -impl fmt::Display for CommandError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { +impl std::fmt::Display for CommandError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self.kind { CommandErrorKind::Io(ref e) => e.fmt(f), CommandErrorKind::Stderr(ref bytes) => { @@ -55,7 +49,7 @@ impl fmt::Display for CommandError { if msg.trim().is_empty() { write!(f, "") } else { - let div = iter::repeat('-').take(79).collect::(); + let div = "-".repeat(79); write!( f, "\n{div}\n{msg}\n{div}", @@ -161,18 +155,17 @@ impl CommandReaderBuilder { /// is returned as an error. /// /// ```no_run -/// use std::io::Read; -/// use std::process::Command; +/// use std::{io::Read, process::Command}; +/// /// use grep_cli::CommandReader; /// -/// # fn example() -> Result<(), Box<::std::error::Error>> { /// let mut cmd = Command::new("gzip"); /// cmd.arg("-d").arg("-c").arg("/usr/share/man/man1/ls.1.gz"); /// /// let mut rdr = CommandReader::new(&mut cmd)?; /// let mut contents = vec![]; /// rdr.read_to_end(&mut contents)?; -/// # Ok(()) } +/// # Ok::<(), Box>(()) /// ``` #[derive(Debug)] pub struct CommandReader { @@ -279,7 +272,7 @@ impl io::Read for CommandReader { /// stderr. #[derive(Debug)] enum StderrReader { - Async(Option>), + Async(Option>), Sync(process::ChildStderr), } @@ -287,7 +280,7 @@ impl StderrReader { /// Create a reader for stderr that reads contents asynchronously. fn r#async(mut stderr: process::ChildStderr) -> StderrReader { let handle = - thread::spawn(move || stderr_to_command_error(&mut stderr)); + std::thread::spawn(move || stderr_to_command_error(&mut stderr)); StderrReader::Async(Some(handle)) } diff --git a/crates/cli/src/wtr.rs b/crates/cli/src/wtr.rs index b6755d1d..18c1175a 100644 --- a/crates/cli/src/wtr.rs +++ b/crates/cli/src/wtr.rs @@ -1,9 +1,6 @@ -use std::io; +use std::io::{self, IsTerminal}; -use termcolor; -use termcolor::HyperlinkSpec; - -use crate::is_tty_stdout; +use termcolor::{self, HyperlinkSpec}; /// A writer that supports coloring with either line or block buffering. pub struct StandardStream(StandardStreamKind); @@ -23,7 +20,7 @@ pub struct StandardStream(StandardStreamKind); /// The color choice given is passed along to the underlying writer. To /// completely disable colors in all cases, use `ColorChoice::Never`. pub fn stdout(color_choice: termcolor::ColorChoice) -> StandardStream { - if is_tty_stdout() { + if std::io::stdout().is_terminal() { stdout_buffered_line(color_choice) } else { stdout_buffered_block(color_choice) diff --git a/crates/core/args.rs b/crates/core/args.rs index f3af1dab..9984a592 100644 --- a/crates/core/args.rs +++ b/crates/core/args.rs @@ -2,7 +2,7 @@ use std::cmp; use std::env; use std::ffi::{OsStr, OsString}; use std::fs; -use std::io::{self, Write}; +use std::io::{self, IsTerminal, Write}; use std::path::{Path, PathBuf}; use std::process; use std::str::FromStr; @@ -976,7 +976,7 @@ impl ArgMatches { } else if preference == "ansi" { ColorChoice::AlwaysAnsi } else if preference == "auto" { - if cli::is_tty_stdout() || self.is_present("pretty") { + if std::io::stdout().is_terminal() || self.is_present("pretty") { ColorChoice::Auto } else { ColorChoice::Never @@ -1110,7 +1110,7 @@ impl ArgMatches { if self.is_present("no-heading") || self.is_present("vimgrep") { false } else { - cli::is_tty_stdout() + std::io::stdout().is_terminal() || self.is_present("heading") || self.is_present("pretty") } @@ -1178,7 +1178,7 @@ impl ArgMatches { // generally want to show line numbers by default when printing to a // tty for human consumption, except for one interesting case: when // we're only searching stdin. This makes pipelines work as expected. - (cli::is_tty_stdout() && !self.is_only_stdin(paths)) + (std::io::stdout().is_terminal() && !self.is_only_stdin(paths)) || self.is_present("line-number") || self.is_present("column") || self.is_present("pretty") diff --git a/crates/grep/examples/simplegrep.rs b/crates/grep/examples/simplegrep.rs index 218b6935..fc73ea96 100644 --- a/crates/grep/examples/simplegrep.rs +++ b/crates/grep/examples/simplegrep.rs @@ -1,14 +1,15 @@ -use std::env; -use std::error::Error; -use std::ffi::OsString; -use std::process; +use std::{env, error::Error, ffi::OsString, io::IsTerminal, process}; -use grep::cli; -use grep::printer::{ColorSpecs, StandardBuilder}; -use grep::regex::RegexMatcher; -use grep::searcher::{BinaryDetection, SearcherBuilder}; -use termcolor::ColorChoice; -use walkdir::WalkDir; +use { + grep::{ + cli, + printer::{ColorSpecs, StandardBuilder}, + regex::RegexMatcher, + searcher::{BinaryDetection, SearcherBuilder}, + }, + termcolor::ColorChoice, + walkdir::WalkDir, +}; fn main() { if let Err(err) = try_main() { @@ -36,7 +37,7 @@ fn search(pattern: &str, paths: &[OsString]) -> Result<(), Box> { .build(); let mut printer = StandardBuilder::new() .color_specs(ColorSpecs::default_with_color()) - .build(cli::stdout(if cli::is_tty_stdout() { + .build(cli::stdout(if std::io::stdout().is_terminal() { ColorChoice::Auto } else { ColorChoice::Never