From 2c9ee8d0ec0a871c05353c70cac5802d3b38ec98 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Fri, 3 Nov 2023 02:27:11 +0800 Subject: [PATCH 01/22] refactor!: overall reorganization --- Cargo.lock | 18 ------ Cargo.toml | 1 - src/error.rs | 25 +------- src/input.rs | 138 ++++++++++++++++++++++---------------------- src/replacer/mod.rs | 75 +----------------------- 5 files changed, 73 insertions(+), 184 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f67cce..b15c7f2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -324,12 +324,6 @@ version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" -[[package]] -name = "hermit-abi" -version = "0.3.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d77f7ec81a6d05a3abb01ab6eb7590f6083d08449fe5a1c8b1e620283546ccb7" - [[package]] name = "insta" version = "1.34.0" @@ -343,17 +337,6 @@ dependencies = [ "yaml-rust", ] -[[package]] -name = "is-terminal" -version = "0.4.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cb0889898416213fab133e1d33a0e5858a48177452750691bde3666d0fdbaf8b" -dependencies = [ - "hermit-abi", - "rustix", - "windows-sys 0.48.0", -] - [[package]] name = "itertools" version = "0.11.0" @@ -657,7 +640,6 @@ dependencies = [ "clap_mangen", "console", "insta", - "is-terminal", "memmap2", "proptest", "rayon", diff --git a/Cargo.toml b/Cargo.toml index 97ffc3b..c9eef4d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,6 @@ memmap2 = "0.9.0" tempfile = "3.8.0" thiserror = "1.0.50" ansi_term = "0.12.1" -is-terminal = "0.4.9" clap.workspace = true [dev-dependencies] diff --git a/src/error.rs b/src/error.rs index 517defd..ad5fe31 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,7 +1,4 @@ -use std::{ - fmt::{self, Write}, - path::PathBuf, -}; +use std::path::PathBuf; use crate::replacer::InvalidReplaceCapture; @@ -15,30 +12,10 @@ pub enum Error { TempfilePersist(#[from] tempfile::PersistError), #[error("file doesn't have parent path: {0}")] InvalidPath(PathBuf), - #[error("failed processing files:\n{0}")] - FailedProcessing(FailedJobs), #[error("{0}")] InvalidReplaceCapture(#[from] InvalidReplaceCapture), } -pub struct FailedJobs(Vec<(PathBuf, Error)>); - -impl From> for FailedJobs { - fn from(vec: Vec<(PathBuf, Error)>) -> Self { - Self(vec) - } -} - -impl fmt::Display for FailedJobs { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.write_str("\tFailedJobs(\n")?; - for (path, err) in &self.0 { - f.write_str(&format!("\t{:?}: {}\n", path, err))?; - } - f.write_char(')') - } -} - // pretty-print the error impl std::fmt::Debug for Error { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { diff --git a/src/input.rs b/src/input.rs index 79a174d..b099bcc 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,10 +1,15 @@ -use std::{fs::File, io::prelude::*, path::PathBuf}; +use std::{ + fs::{File, self}, + io::{Write, stdin, stdout, Read}, + path::PathBuf, + ops::DerefMut, +}; use crate::{Error, Replacer, Result}; -use is_terminal::IsTerminal; +use memmap2::{Mmap, MmapMut, MmapOptions}; -#[derive(Debug)] +#[derive(Debug, PartialEq)] pub(crate) enum Source { Stdin, Files(Vec), @@ -16,83 +21,80 @@ pub(crate) struct App { } impl App { - fn stdin_replace(&self, is_tty: bool) -> Result<()> { - let mut buffer = Vec::with_capacity(256); - let stdin = std::io::stdin(); - let mut handle = stdin.lock(); - handle.read_to_end(&mut buffer)?; - - let stdout = std::io::stdout(); - let mut handle = stdout.lock(); - - handle.write_all(&if is_tty { - self.replacer.replace_preview(&buffer) - } else { - self.replacer.replace(&buffer) - })?; - - Ok(()) - } - pub(crate) fn new(source: Source, replacer: Replacer) -> Self { Self { source, replacer } } + pub(crate) fn run(&self, preview: bool) -> Result<()> { - let is_tty = std::io::stdout().is_terminal(); + let sources: Vec<(PathBuf, Mmap)> = match &self.source { + Source::Stdin => { + let mut handle = stdin().lock(); + let mut buf = Vec::new(); + handle.read_to_end(&mut buf)?; + let mut mmap = MmapOptions::new() + .len(buf.len()) + .map_anon()?; + mmap.copy_from_slice(&buf); + let mmap = mmap.make_read_only()?; + vec![(PathBuf::from("STDIN"), mmap)] + }, + Source::Files(paths) => { + let mut refs = Vec::new(); + for path in paths { + if !path.exists() { + return Err(Error::InvalidPath(path.clone())); + } + let mmap = unsafe { Mmap::map(&File::open(path)?)? }; + refs.push((path.clone(), mmap)); + } + refs + }, + }; + let needs_separator = sources.len() > 1; - match (&self.source, preview) { - (Source::Stdin, true) => self.stdin_replace(is_tty), - (Source::Stdin, false) => self.stdin_replace(is_tty), - (Source::Files(paths), false) => { - use rayon::prelude::*; + let replaced: Vec<_> = { + use rayon::prelude::*; + sources.par_iter() + .map(|(path, mmap)| { + let replaced = self.replacer.replace(mmap); + (path, mmap, replaced) + }) + .collect() + }; - let failed_jobs: Vec<_> = paths - .par_iter() - .filter_map(|p| { - if let Err(e) = self.replacer.replace_file(p) { - Some((p.to_owned(), e)) - } else { - None - } - }) - .collect(); + if preview || self.source == Source::Stdin { + let mut handle = stdout().lock(); - if failed_jobs.is_empty() { - Ok(()) - } else { - let failed_jobs = - crate::error::FailedJobs::from(failed_jobs); - Err(Error::FailedProcessing(failed_jobs)) + for (path, _, replaced) in replaced { + if needs_separator { + writeln!(handle, "----- FILE {} -----", path.display())?; } + handle.write_all(replaced.as_ref())?; } - (Source::Files(paths), true) => { - let stdout = std::io::stdout(); - let mut handle = stdout.lock(); - let print_path = paths.len() > 1; - - paths.iter().try_for_each(|path| { - if Replacer::check_not_empty(File::open(path)?).is_err() { - return Ok(()); - } - let file = - unsafe { memmap2::Mmap::map(&File::open(path)?)? }; - if self.replacer.has_matches(&file) { - if print_path { - writeln!( - handle, - "----- FILE {} -----", - path.display() - )?; - } + } else { + for (path, _, replaced) in replaced { + let source = File::open(path)?; + let meta = fs::metadata(path)?; + drop(source); - handle - .write_all(&self.replacer.replace_preview(&file))?; - writeln!(handle)?; - } + let target = tempfile::NamedTempFile::new_in( + path.parent() + .ok_or_else(|| Error::InvalidPath(path.to_path_buf()))?, + )?; + let file = target.as_file(); + file.set_len(replaced.len() as u64)?; + file.set_permissions(meta.permissions())?; - Ok(()) - }) + if !replaced.is_empty() { + let mut mmap_target = unsafe { MmapMut::map_mut(file)? }; + mmap_target.deref_mut().write_all(&replaced)?; + mmap_target.flush_async()?; + } + + target.persist(fs::canonicalize(path)?)?; } } + + Ok(()) } } diff --git a/src/replacer/mod.rs b/src/replacer/mod.rs index d871323..e1b32a3 100644 --- a/src/replacer/mod.rs +++ b/src/replacer/mod.rs @@ -1,6 +1,6 @@ -use std::{borrow::Cow, fs, fs::File, io::prelude::*, path::Path}; +use std::borrow::Cow; -use crate::{utils, Error, Result}; +use crate::{utils, Result}; use regex::bytes::Regex; @@ -74,16 +74,6 @@ impl Replacer { }) } - pub(crate) fn has_matches(&self, content: &[u8]) -> bool { - self.regex.is_match(content) - } - - pub(crate) fn check_not_empty(mut file: File) -> Result<()> { - let mut buf: [u8; 1] = Default::default(); - file.read_exact(&mut buf)?; - Ok(()) - } - pub(crate) fn replace<'a>( &'a self, content: &'a [u8], @@ -148,65 +138,4 @@ impl Replacer { new.extend_from_slice(&haystack[last_match..]); Cow::Owned(new) } - - pub(crate) fn replace_preview<'a>( - &self, - content: &'a [u8], - ) -> std::borrow::Cow<'a, [u8]> { - let regex = &self.regex; - let limit = self.replacements; - // TODO: refine this condition more - let use_color = true; - if self.is_literal { - Self::replacen( - regex, - limit, - content, - use_color, - regex::bytes::NoExpand(&self.replace_with), - ) - } else { - Self::replacen( - regex, - limit, - content, - use_color, - &*self.replace_with, - ) - } - } - - pub(crate) fn replace_file(&self, path: &Path) -> Result<()> { - use memmap2::{Mmap, MmapMut}; - use std::ops::DerefMut; - - if Self::check_not_empty(File::open(path)?).is_err() { - return Ok(()); - } - - let source = File::open(path)?; - let meta = fs::metadata(path)?; - let mmap_source = unsafe { Mmap::map(&source)? }; - let replaced = self.replace(&mmap_source); - - let target = tempfile::NamedTempFile::new_in( - path.parent() - .ok_or_else(|| Error::InvalidPath(path.to_path_buf()))?, - )?; - let file = target.as_file(); - file.set_len(replaced.len() as u64)?; - file.set_permissions(meta.permissions())?; - - if !replaced.is_empty() { - let mut mmap_target = unsafe { MmapMut::map_mut(file)? }; - mmap_target.deref_mut().write_all(&replaced)?; - mmap_target.flush_async()?; - } - - drop(mmap_source); - drop(source); - - target.persist(fs::canonicalize(path)?)?; - Ok(()) - } } From 29d390da368d0e884b44791a9f2099ec2ae85e38 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 07:03:34 +0800 Subject: [PATCH 02/22] fmt: tab to spaces --- src/input.rs | 48 ++++++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/input.rs b/src/input.rs index b099bcc..395e6ce 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,8 +1,8 @@ use std::{ - fs::{File, self}, - io::{Write, stdin, stdout, Read}, - path::PathBuf, - ops::DerefMut, + fs::{self, File}, + io::{stdin, stdout, Read, Write}, + ops::DerefMut, + path::PathBuf, }; use crate::{Error, Replacer, Result}; @@ -28,37 +28,36 @@ impl App { pub(crate) fn run(&self, preview: bool) -> Result<()> { let sources: Vec<(PathBuf, Mmap)> = match &self.source { Source::Stdin => { - let mut handle = stdin().lock(); - let mut buf = Vec::new(); - handle.read_to_end(&mut buf)?; - let mut mmap = MmapOptions::new() - .len(buf.len()) - .map_anon()?; - mmap.copy_from_slice(&buf); - let mmap = mmap.make_read_only()?; - vec![(PathBuf::from("STDIN"), mmap)] - }, + let mut handle = stdin().lock(); + let mut buf = Vec::new(); + handle.read_to_end(&mut buf)?; + let mut mmap = MmapOptions::new().len(buf.len()).map_anon()?; + mmap.copy_from_slice(&buf); + let mmap = mmap.make_read_only()?; + vec![(PathBuf::from("STDIN"), mmap)] + } Source::Files(paths) => { - let mut refs = Vec::new(); + let mut refs = Vec::new(); for path in paths { if !path.exists() { return Err(Error::InvalidPath(path.clone())); } - let mmap = unsafe { Mmap::map(&File::open(path)?)? }; + let mmap = unsafe { Mmap::map(&File::open(path)?)? }; refs.push((path.clone(), mmap)); } refs - }, + } }; let needs_separator = sources.len() > 1; let replaced: Vec<_> = { use rayon::prelude::*; - sources.par_iter() + sources + .par_iter() .map(|(path, mmap)| { - let replaced = self.replacer.replace(mmap); - (path, mmap, replaced) - }) + let replaced = self.replacer.replace(mmap); + (path, mmap, replaced) + }) .collect() }; @@ -78,8 +77,9 @@ impl App { drop(source); let target = tempfile::NamedTempFile::new_in( - path.parent() - .ok_or_else(|| Error::InvalidPath(path.to_path_buf()))?, + path.parent().ok_or_else(|| { + Error::InvalidPath(path.to_path_buf()) + })?, )?; let file = target.as_file(); file.set_len(replaced.len() as u64)?; @@ -90,7 +90,7 @@ impl App { mmap_target.deref_mut().write_all(&replaced)?; mmap_target.flush_async()?; } - + target.persist(fs::canonicalize(path)?)?; } } From 7b1db4499c864f22570cd853288494e2d27cf059 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 09:51:06 +0800 Subject: [PATCH 03/22] feat: split out funcs for better error collection --- src/input.rs | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/src/input.rs b/src/input.rs index 395e6ce..5910436 100644 --- a/src/input.rs +++ b/src/input.rs @@ -61,7 +61,9 @@ impl App { .collect() }; - if preview || self.source == Source::Stdin { + let mut errors = Vec::new(); + + if preview || self.sources.get(0) == Some(&Source::Stdin) { let mut handle = stdout().lock(); for (path, _, replaced) in replaced { @@ -98,3 +100,42 @@ impl App { Ok(()) } } + +fn make_mmap(path: &PathBuf) -> Result { + Ok(unsafe { Mmap::map(&File::open(path)?)? }) +} + +fn make_mmap_stdin() -> Result { + let mut handle = stdin().lock(); + let mut buf = Vec::new(); + handle.read_to_end(&mut buf)?; + let mut mmap = MmapOptions::new().len(buf.len()).map_anon()?; + mmap.copy_from_slice(&buf); + let mmap = mmap.make_read_only()?; + Ok(mmap) +} + +fn write_with_temp(path: &PathBuf, data: &[u8]) -> Result<()> { + let path = fs::canonicalize(path)?; + + let temp = tempfile::NamedTempFile::new_in( + path.parent() + .ok_or_else(|| Error::InvalidPath(path.to_path_buf()))?, + )?; + + let file = temp.as_file(); + file.set_len(data.len() as u64)?; + if let Ok(metadata) = fs::metadata(&path) { + file.set_permissions(metadata.permissions()).ok(); + } + + if !data.is_empty() { + let mut mmap_temp = unsafe { MmapMut::map_mut(file)? }; + mmap_temp.deref_mut().write_all(data)?; + mmap_temp.flush_async()?; + } + + temp.persist(&path)?; + + Ok(()) +} From d66f8bd5674f6d225be0f87ecaa053898ab0bba2 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 09:54:57 +0800 Subject: [PATCH 04/22] feat: replace Vec-varianted Source with Vec --- src/input.rs | 106 +++++++++++++++++++++++++-------------------------- src/main.rs | 8 ++-- 2 files changed, 57 insertions(+), 57 deletions(-) diff --git a/src/input.rs b/src/input.rs index 5910436..108665a 100644 --- a/src/input.rs +++ b/src/input.rs @@ -12,52 +12,55 @@ use memmap2::{Mmap, MmapMut, MmapOptions}; #[derive(Debug, PartialEq)] pub(crate) enum Source { Stdin, - Files(Vec), + File(PathBuf), +} + +impl Source { + pub(crate) fn from_paths(paths: Vec) -> Vec { + paths.into_iter().map(Self::File).collect() + } + + pub(crate) fn from_stdin() -> Vec { + vec![Self::Stdin] + } + + fn display(&self) -> String { + match self { + Self::Stdin => "STDIN".to_string(), + Self::File(path) => format!("FILE {}", path.display()), + } + } } pub(crate) struct App { replacer: Replacer, - source: Source, + sources: Vec, } impl App { - pub(crate) fn new(source: Source, replacer: Replacer) -> Self { - Self { source, replacer } + pub(crate) fn new(sources: Vec, replacer: Replacer) -> Self { + Self { sources, replacer } } pub(crate) fn run(&self, preview: bool) -> Result<()> { - let sources: Vec<(PathBuf, Mmap)> = match &self.source { - Source::Stdin => { - let mut handle = stdin().lock(); - let mut buf = Vec::new(); - handle.read_to_end(&mut buf)?; - let mut mmap = MmapOptions::new().len(buf.len()).map_anon()?; - mmap.copy_from_slice(&buf); - let mmap = mmap.make_read_only()?; - vec![(PathBuf::from("STDIN"), mmap)] - } - Source::Files(paths) => { - let mut refs = Vec::new(); - for path in paths { - if !path.exists() { - return Err(Error::InvalidPath(path.clone())); - } - let mmap = unsafe { Mmap::map(&File::open(path)?)? }; - refs.push((path.clone(), mmap)); - } - refs - } - }; + let sources: Vec<(&Source, Result)> = self.sources + .iter() + .map(|source| (source, match source { + Source::File(path) => if path.exists() { + make_mmap(path) + } else { + Err(Error::InvalidPath(path.clone())) + }, + Source::Stdin => make_mmap_stdin() + })) + .collect(); let needs_separator = sources.len() > 1; let replaced: Vec<_> = { use rayon::prelude::*; sources .par_iter() - .map(|(path, mmap)| { - let replaced = self.replacer.replace(mmap); - (path, mmap, replaced) - }) + .map(|(source, maybe_mmap)| (source, maybe_mmap.as_ref().map(|mmap| self.replacer.replace(mmap)))) .collect() }; @@ -66,37 +69,34 @@ impl App { if preview || self.sources.get(0) == Some(&Source::Stdin) { let mut handle = stdout().lock(); - for (path, _, replaced) in replaced { + for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { if needs_separator { - writeln!(handle, "----- FILE {} -----", path.display())?; + writeln!(handle, "----- {} -----", source.display())?; } - handle.write_all(replaced.as_ref())?; + handle.write_all(replaced.as_ref().unwrap())?; } } else { - for (path, _, replaced) in replaced { - let source = File::open(path)?; - let meta = fs::metadata(path)?; - drop(source); - - let target = tempfile::NamedTempFile::new_in( - path.parent().ok_or_else(|| { - Error::InvalidPath(path.to_path_buf()) - })?, - )?; - let file = target.as_file(); - file.set_len(replaced.len() as u64)?; - file.set_permissions(meta.permissions())?; - - if !replaced.is_empty() { - let mut mmap_target = unsafe { MmapMut::map_mut(file)? }; - mmap_target.deref_mut().write_all(&replaced)?; - mmap_target.flush_async()?; - } + for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { + let path = match source { + Source::File(path) => path, + Source::Stdin => { + unreachable!("stdin should only go previous branch") + } + }; - target.persist(fs::canonicalize(path)?)?; + if let Err(e) = write_with_temp(path, replaced.as_ref().unwrap()) { + errors.push((source, e)); + } } } + for (source, error) in replaced.iter().filter(|(_, r)| r.is_err()) { + eprintln!("error: {}: {:?}", source.display(), error); + } + for (source, error) in errors { + eprintln!("error: {}: {:?}", source.display(), error); + } + Ok(()) } } diff --git a/src/main.rs b/src/main.rs index 07c48b8..b978707 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,14 +24,14 @@ fn main() { fn try_main() -> Result<()> { let options = cli::Options::parse(); - let source = if !options.files.is_empty() { - Source::Files(options.files) + let sources = if !options.files.is_empty() { + Source::from_paths(options.files) } else { - Source::Stdin + Source::from_stdin() }; App::new( - source, + sources, Replacer::new( options.find, options.replace_with, From 5e7d0e4beac43f7c7e0827de47423eac87907456 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 09:57:25 +0800 Subject: [PATCH 05/22] docs: more generic description for Error::InvalidPath --- src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index ad5fe31..0955d02 100644 --- a/src/error.rs +++ b/src/error.rs @@ -10,7 +10,7 @@ pub enum Error { File(#[from] std::io::Error), #[error("failed to move file: {0}")] TempfilePersist(#[from] tempfile::PersistError), - #[error("file doesn't have parent path: {0}")] + #[error("invalid path: {0}")] InvalidPath(PathBuf), #[error("{0}")] InvalidReplaceCapture(#[from] InvalidReplaceCapture), From 69094aea3883c1b76af1890d324fb7b4bbec5c17 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 09:57:57 +0800 Subject: [PATCH 06/22] fmt: reduce signature verbosity --- src/replacer/mod.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/replacer/mod.rs b/src/replacer/mod.rs index e1b32a3..fa65e6c 100644 --- a/src/replacer/mod.rs +++ b/src/replacer/mod.rs @@ -74,10 +74,7 @@ impl Replacer { }) } - pub(crate) fn replace<'a>( - &'a self, - content: &'a [u8], - ) -> std::borrow::Cow<'a, [u8]> { + pub(crate) fn replace<'a>(&'a self, content: &'a [u8]) -> Cow<'a, [u8]> { let regex = &self.regex; let limit = self.replacements; let use_color = false; From 4530e0881a2c6b07fb02249f16dcee9225aa29ef Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 10:06:55 +0800 Subject: [PATCH 07/22] internal: decolorize to prepare for proper colorization --- Cargo.lock | 32 -------------------------------- Cargo.toml | 1 - src/main.rs | 3 +-- src/replacer/mod.rs | 12 +----------- src/replacer/validate.rs | 18 +++++++----------- tests/cli.rs | 21 ++++----------------- 6 files changed, 13 insertions(+), 74 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b15c7f2..2123179 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -21,15 +21,6 @@ dependencies = [ "thiserror", ] -[[package]] -name = "ansi_term" -version = "0.12.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2" -dependencies = [ - "winapi", -] - [[package]] name = "anstream" version = "0.6.4" @@ -633,7 +624,6 @@ name = "sd" version = "1.0.0" dependencies = [ "ansi-to-html", - "ansi_term", "anyhow", "assert_cmd", "clap", @@ -787,28 +777,6 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" -[[package]] -name = "winapi" -version = "0.3.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" -dependencies = [ - "winapi-i686-pc-windows-gnu", - "winapi-x86_64-pc-windows-gnu", -] - -[[package]] -name = "winapi-i686-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" - -[[package]] -name = "winapi-x86_64-pc-windows-gnu" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" - [[package]] name = "windows-sys" version = "0.45.0" diff --git a/Cargo.toml b/Cargo.toml index c9eef4d..e44aede 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,6 @@ unescape = "0.1.0" memmap2 = "0.9.0" tempfile = "3.8.0" thiserror = "1.0.50" -ansi_term = "0.12.1" clap.workspace = true [dev-dependencies] diff --git a/src/main.rs b/src/main.rs index b978707..0a16e4b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,6 @@ pub(crate) mod utils; use std::process; pub(crate) use self::input::{App, Source}; -use ansi_term::{Color, Style}; pub(crate) use error::{Error, Result}; use replacer::Replacer; @@ -16,7 +15,7 @@ use clap::Parser; fn main() { if let Err(e) = try_main() { - eprintln!("{}: {}", Style::from(Color::Red).bold().paint("error"), e); + eprintln!("{}: {}", "error", e); process::exit(1); } } diff --git a/src/replacer/mod.rs b/src/replacer/mod.rs index fa65e6c..2bc3608 100644 --- a/src/replacer/mod.rs +++ b/src/replacer/mod.rs @@ -103,7 +103,7 @@ impl Replacer { regex: ®ex::bytes::Regex, limit: usize, haystack: &'haystack [u8], - use_color: bool, + _use_color: bool, mut rep: R, ) -> Cow<'haystack, [u8]> { let mut it = regex.captures_iter(haystack).enumerate().peekable(); @@ -116,17 +116,7 @@ impl Replacer { // unwrap on 0 is OK because captures only reports matches let m = cap.get(0).unwrap(); new.extend_from_slice(&haystack[last_match..m.start()]); - if use_color { - new.extend_from_slice( - ansi_term::Color::Blue.prefix().to_string().as_bytes(), - ); - } rep.replace_append(&cap, &mut new); - if use_color { - new.extend_from_slice( - ansi_term::Color::Blue.suffix().to_string().as_bytes(), - ); - } last_match = m.end(); if limit > 0 && i >= limit - 1 { break; diff --git a/src/replacer/validate.rs b/src/replacer/validate.rs index da5cc71..bbaf8c1 100644 --- a/src/replacer/validate.rs +++ b/src/replacer/validate.rs @@ -1,7 +1,5 @@ use std::{error::Error, fmt, str::CharIndices}; -use ansi_term::{Color, Style}; - #[derive(Debug)] pub struct InvalidReplaceCapture { original_replace: String, @@ -53,21 +51,19 @@ impl fmt::Display for InvalidReplaceCapture { // Build up the error to show the user let mut formatted = String::new(); let mut arrows_start = Span::start_at(0); - let special = Style::new().bold(); - let error = Style::from(Color::Red).bold(); for (byte_index, c) in original_replace.char_indices() { let (prefix, suffix, text) = match SpecialChar::new(c) { Some(c) => { - (Some(special.prefix()), Some(special.suffix()), c.render()) + (Some("" /* special prefix */), Some("" /* special suffix */), c.render()) } None => { let (prefix, suffix) = if byte_index == invalid_ident.start { - (Some(error.prefix()), None) + (Some("" /* error prefix */), None) } else if byte_index == invalid_ident.end.checked_sub(1).unwrap() { - (None, Some(error.suffix())) + (None, Some("" /* error suffix */)) } else { (None, None) }; @@ -99,7 +95,7 @@ impl fmt::Display for InvalidReplaceCapture { let mut arrows = " ".repeat(arrows_span.start); arrows.push_str(&format!( "{}", - Style::new().bold().paint("^".repeat(arrows_span.len())) + "^".repeat(arrows_span.len()) )); let ident = invalid_ident.slice(original_replace); @@ -107,12 +103,12 @@ impl fmt::Display for InvalidReplaceCapture { let disambiguous = format!("${{{number}}}{the_rest}"); let error_message = format!( "The numbered capture group `{}` in the replacement text is ambiguous.", - Style::new().bold().paint(format!("${}", number).to_string()) + format!("${}", number).to_string() ); let hint_message = format!( "{}: Use curly braces to disambiguate it `{}`.", - Style::from(Color::Blue).bold().paint("hint"), - Style::new().bold().paint(disambiguous) + "hint", + disambiguous ); writeln!(f, "{}", error_message)?; diff --git a/tests/cli.rs b/tests/cli.rs index 6bf78cc..696aa43 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -81,11 +81,7 @@ mod cli { sd().args(["-p", "abc\\d+", "", file.path().to_str().unwrap()]) .assert() .success() - .stdout(format!( - "{}{}def\n", - ansi_term::Color::Blue.prefix(), - ansi_term::Color::Blue.suffix() - )); + .stdout("def"); assert_file(file.path(), "abc123def"); @@ -113,13 +109,7 @@ mod cli { fn bad_replace_helper_plain(replace: &str) -> String { let stderr = bad_replace_helper_styled(replace); - - // TODO: no easy way to toggle off styling yet. Add a `--color ` - // flag, and respect things like `$NO_COLOR`. `ansi_term` is - // unmaintained, so we should migrate off of it anyways - console::AnsiCodeIterator::new(&stderr) - .filter_map(|(s, is_ansi)| (!is_ansi).then_some(s)) - .collect() + stderr } #[test] @@ -182,7 +172,7 @@ mod cli { // NOTE: styled terminal output is platform dependent, so convert to a // common format, in this case HTML, to check - #[test] + //#[test] fn ambiguous_replace_ensure_styling() { let styled_stderr = bad_replace_helper_styled("\t$1bad after"); let html_stderr = @@ -225,10 +215,7 @@ mod cli { ]) .assert() .success() - .stdout(format!( - "{}\nfoo\nfoo\n", - ansi_term::Color::Blue.paint("bar") - )); + .stdout("bar\nfoo\nfoo"); Ok(()) } From 857c630c7c3fcc9907a55a7924dc1f18591f951d Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 10:08:32 +0800 Subject: [PATCH 08/22] fmt: tab to spaces (again) --- .editorconfig | 4 ++++ src/input.rs | 26 +++++++++++++------------- tests/cli.rs | 2 +- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/.editorconfig b/.editorconfig index 649cad9..5a09788 100644 --- a/.editorconfig +++ b/.editorconfig @@ -1,2 +1,6 @@ +[*] +indent_style = space +indent_size = 4 + [*.rs] max_line_length = 80 diff --git a/src/input.rs b/src/input.rs index 108665a..619eba8 100644 --- a/src/input.rs +++ b/src/input.rs @@ -24,12 +24,12 @@ impl Source { vec![Self::Stdin] } - fn display(&self) -> String { - match self { - Self::Stdin => "STDIN".to_string(), - Self::File(path) => format!("FILE {}", path.display()), - } - } + fn display(&self) -> String { + match self { + Self::Stdin => "STDIN".to_string(), + Self::File(path) => format!("FILE {}", path.display()), + } + } } pub(crate) struct App { @@ -47,9 +47,9 @@ impl App { .iter() .map(|source| (source, match source { Source::File(path) => if path.exists() { - make_mmap(path) - } else { - Err(Error::InvalidPath(path.clone())) + make_mmap(path) + } else { + Err(Error::InvalidPath(path.clone())) }, Source::Stdin => make_mmap_stdin() })) @@ -116,7 +116,7 @@ fn make_mmap_stdin() -> Result { } fn write_with_temp(path: &PathBuf, data: &[u8]) -> Result<()> { - let path = fs::canonicalize(path)?; + let path = fs::canonicalize(path)?; let temp = tempfile::NamedTempFile::new_in( path.parent() @@ -125,9 +125,9 @@ fn write_with_temp(path: &PathBuf, data: &[u8]) -> Result<()> { let file = temp.as_file(); file.set_len(data.len() as u64)?; - if let Ok(metadata) = fs::metadata(&path) { - file.set_permissions(metadata.permissions()).ok(); - } + if let Ok(metadata) = fs::metadata(&path) { + file.set_permissions(metadata.permissions()).ok(); + } if !data.is_empty() { let mut mmap_temp = unsafe { MmapMut::map_mut(file)? }; diff --git a/tests/cli.rs b/tests/cli.rs index 696aa43..9e193b7 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -109,7 +109,7 @@ mod cli { fn bad_replace_helper_plain(replace: &str) -> String { let stderr = bad_replace_helper_styled(replace); - stderr + stderr } #[test] From 7fcfdde1bce2ee6135f02c8c9eab09835e6e27bf Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 12:29:13 +0800 Subject: [PATCH 09/22] refactor: break up App into main.rs, rm utils.rs --- src/input.rs | 112 ++------------------------------------ src/main.rs | 130 +++++++++++++++++++++++++++++++++++--------- src/replacer/mod.rs | 4 +- src/utils.rs | 3 - 4 files changed, 113 insertions(+), 136 deletions(-) delete mode 100644 src/utils.rs diff --git a/src/input.rs b/src/input.rs index 619eba8..4cb9e62 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,13 +1,7 @@ -use std::{ - fs::{self, File}, - io::{stdin, stdout, Read, Write}, - ops::DerefMut, - path::PathBuf, -}; +use std::{path::PathBuf, fs::File, io::{stdin, Read}}; +use memmap2::{Mmap, MmapOptions}; -use crate::{Error, Replacer, Result}; - -use memmap2::{Mmap, MmapMut, MmapOptions}; +use crate::error::Result; #[derive(Debug, PartialEq)] pub(crate) enum Source { @@ -24,7 +18,7 @@ impl Source { vec![Self::Stdin] } - fn display(&self) -> String { + pub(crate) fn display(&self) -> String { match self { Self::Stdin => "STDIN".to_string(), Self::File(path) => format!("FILE {}", path.display()), @@ -32,80 +26,11 @@ impl Source { } } -pub(crate) struct App { - replacer: Replacer, - sources: Vec, -} - -impl App { - pub(crate) fn new(sources: Vec, replacer: Replacer) -> Self { - Self { sources, replacer } - } - - pub(crate) fn run(&self, preview: bool) -> Result<()> { - let sources: Vec<(&Source, Result)> = self.sources - .iter() - .map(|source| (source, match source { - Source::File(path) => if path.exists() { - make_mmap(path) - } else { - Err(Error::InvalidPath(path.clone())) - }, - Source::Stdin => make_mmap_stdin() - })) - .collect(); - let needs_separator = sources.len() > 1; - - let replaced: Vec<_> = { - use rayon::prelude::*; - sources - .par_iter() - .map(|(source, maybe_mmap)| (source, maybe_mmap.as_ref().map(|mmap| self.replacer.replace(mmap)))) - .collect() - }; - - let mut errors = Vec::new(); - - if preview || self.sources.get(0) == Some(&Source::Stdin) { - let mut handle = stdout().lock(); - - for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { - if needs_separator { - writeln!(handle, "----- {} -----", source.display())?; - } - handle.write_all(replaced.as_ref().unwrap())?; - } - } else { - for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { - let path = match source { - Source::File(path) => path, - Source::Stdin => { - unreachable!("stdin should only go previous branch") - } - }; - - if let Err(e) = write_with_temp(path, replaced.as_ref().unwrap()) { - errors.push((source, e)); - } - } - } - - for (source, error) in replaced.iter().filter(|(_, r)| r.is_err()) { - eprintln!("error: {}: {:?}", source.display(), error); - } - for (source, error) in errors { - eprintln!("error: {}: {:?}", source.display(), error); - } - - Ok(()) - } -} - -fn make_mmap(path: &PathBuf) -> Result { +pub(crate) fn make_mmap(path: &PathBuf) -> Result { Ok(unsafe { Mmap::map(&File::open(path)?)? }) } -fn make_mmap_stdin() -> Result { +pub(crate) fn make_mmap_stdin() -> Result { let mut handle = stdin().lock(); let mut buf = Vec::new(); handle.read_to_end(&mut buf)?; @@ -114,28 +39,3 @@ fn make_mmap_stdin() -> Result { let mmap = mmap.make_read_only()?; Ok(mmap) } - -fn write_with_temp(path: &PathBuf, data: &[u8]) -> Result<()> { - let path = fs::canonicalize(path)?; - - let temp = tempfile::NamedTempFile::new_in( - path.parent() - .ok_or_else(|| Error::InvalidPath(path.to_path_buf()))?, - )?; - - let file = temp.as_file(); - file.set_len(data.len() as u64)?; - if let Ok(metadata) = fs::metadata(&path) { - file.set_permissions(metadata.permissions()).ok(); - } - - if !data.is_empty() { - let mut mmap_temp = unsafe { MmapMut::map_mut(file)? }; - mmap_temp.deref_mut().write_all(data)?; - mmap_temp.flush_async()?; - } - - temp.persist(&path)?; - - Ok(()) -} diff --git a/src/main.rs b/src/main.rs index 0a16e4b..13f383b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,42 +3,122 @@ mod error; mod input; pub(crate) mod replacer; -pub(crate) mod utils; - -use std::process; - -pub(crate) use self::input::{App, Source}; -pub(crate) use error::{Error, Result}; -use replacer::Replacer; +use std::{ + fs, + process, + path::PathBuf, + io::{stdout, Write}, + ops::DerefMut, +}; +use memmap2::{Mmap, MmapMut}; use clap::Parser; -fn main() { - if let Err(e) = try_main() { - eprintln!("{}: {}", "error", e); - process::exit(1); - } -} +pub(crate) use self::input::Source; +pub(crate) use self::error::{Error, Result}; +use self::replacer::Replacer; +use self::input::{make_mmap, make_mmap_stdin}; -fn try_main() -> Result<()> { +fn main() -> Result<()> { let options = cli::Options::parse(); + let replacer = Replacer::new( + options.find, + options.replace_with, + options.literal_mode, + options.flags, + options.replacements, + )?; + let sources = if !options.files.is_empty() { Source::from_paths(options.files) } else { Source::from_stdin() }; - App::new( - sources, - Replacer::new( - options.find, - options.replace_with, - options.literal_mode, - options.flags, - options.replacements, - )?, - ) - .run(options.preview)?; + let sources: Vec<(&Source, Result)> = sources + .iter() + .map(|source| (source, match source { + Source::File(path) => if path.exists() { + make_mmap(path) + } else { + Err(Error::InvalidPath(path.clone())) + }, + Source::Stdin => make_mmap_stdin() + })) + .collect(); + let needs_separator = sources.len() > 1; + + let replaced: Vec<_> = { + use rayon::prelude::*; + sources + .par_iter() + .map(|(source, maybe_mmap)| (source, maybe_mmap.as_ref().map(|mmap| replacer.replace(mmap)))) + .collect() + }; + + let mut errors = Vec::new(); + let mut has_error = !errors.is_empty(); + + if options.preview || sources.get(0).map(|s| s.0) == Some(&Source::Stdin) { + let mut handle = stdout().lock(); + + for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { + if needs_separator { + writeln!(handle, "----- {} -----", source.display())?; + } + handle.write_all(replaced.as_ref().unwrap())?; + } + } else { + for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { + let path = match source { + Source::File(path) => path, + Source::Stdin => { + unreachable!("stdin should only go previous branch") + } + }; + + if let Err(e) = write_with_temp(path, replaced.as_ref().unwrap()) { + errors.push((source, e)); + } + } + } + + for (source, error) in replaced.iter().filter(|(_, r)| r.is_err()) { + eprintln!("error: {}: {:?}", source.display(), error); + has_error = true; + } + for (source, error) in errors { + eprintln!("error: {}: {:?}", source.display(), error); + } + + if has_error { + process::exit(1); + } + Ok(()) +} + +fn write_with_temp(path: &PathBuf, data: &[u8]) -> Result<()> { + let path = fs::canonicalize(path)?; + + let temp = tempfile::NamedTempFile::new_in( + path.parent() + .ok_or_else(|| Error::InvalidPath(path.to_path_buf()))?, + )?; + + let file = temp.as_file(); + file.set_len(data.len() as u64)?; + if let Ok(metadata) = fs::metadata(&path) { + file.set_permissions(metadata.permissions()).ok(); + } + + if !data.is_empty() { + let mut mmap_temp = unsafe { MmapMut::map_mut(file)? }; + mmap_temp.deref_mut().write_all(data)?; + mmap_temp.flush_async()?; + } + + temp.persist(&path)?; + Ok(()) } diff --git a/src/replacer/mod.rs b/src/replacer/mod.rs index 2bc3608..8db902e 100644 --- a/src/replacer/mod.rs +++ b/src/replacer/mod.rs @@ -1,6 +1,6 @@ use std::borrow::Cow; -use crate::{utils, Result}; +use crate::Result; use regex::bytes::Regex; @@ -32,7 +32,7 @@ impl Replacer { ( look_for, - utils::unescape(&replace_with) + unescape::unescape(&replace_with) .unwrap_or(replace_with) .into_bytes(), ) diff --git a/src/utils.rs b/src/utils.rs deleted file mode 100644 index 72ad146..0000000 --- a/src/utils.rs +++ /dev/null @@ -1,3 +0,0 @@ -pub(crate) fn unescape(s: &str) -> Option { - unescape::unescape(s) -} From e78ce1541b00135e685b94194b4ce37d5abdf67f Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 17:04:39 +0800 Subject: [PATCH 10/22] test: only run cli::in_place_following_symlink on Unix, symlinks are privileged on Windows --- tests/cli.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/cli.rs b/tests/cli.rs index 9e193b7..ceff066 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -13,14 +13,12 @@ mod cli { assert_eq!(content, std::fs::read_to_string(path).unwrap()); } + #[cfg(target_family = "unix")] fn create_soft_link>( src: &P, dst: &P, ) -> Result<()> { - #[cfg(target_family = "unix")] std::os::unix::fs::symlink(src, dst)?; - #[cfg(target_family = "windows")] - std::os::windows::fs::symlink_file(src, dst)?; Ok(()) } @@ -53,6 +51,7 @@ mod cli { Ok(()) } + #[cfg(target_family = "unix")] #[test] fn in_place_following_symlink() -> Result<()> { let dir = tempfile::tempdir()?; From a8f4e1c76e4dc4d5adb3e91ec3b7eecc76080676 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 17:05:32 +0800 Subject: [PATCH 11/22] test: update insta snapshots due to Replacer::new()? in main() --- tests/cli.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/cli.rs b/tests/cli.rs index ceff066..98dfed9 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -128,7 +128,7 @@ mod cli { fn ambiguous_replace_basic() { let plain_stderr = bad_replace_helper_plain("before $1bad after"); insta::assert_snapshot!(plain_stderr, @r###" - error: The numbered capture group `$1` in the replacement text is ambiguous. + Error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}bad`. before $1bad after ^^^^ @@ -139,7 +139,7 @@ mod cli { fn ambiguous_replace_variable_width() { let plain_stderr = bad_replace_helper_plain("\r\n\t$1bad\r"); insta::assert_snapshot!(plain_stderr, @r###" - error: The numbered capture group `$1` in the replacement text is ambiguous. + Error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}bad`. ␍␊␉$1bad␍ ^^^^ @@ -150,7 +150,7 @@ mod cli { fn ambiguous_replace_multibyte_char() { let plain_stderr = bad_replace_helper_plain("😈$1bad😇"); insta::assert_snapshot!(plain_stderr, @r###" - error: The numbered capture group `$1` in the replacement text is ambiguous. + Error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}bad`. 😈$1bad😇 ^^^^ @@ -162,7 +162,7 @@ mod cli { let plain_stderr = bad_replace_helper_plain("$1Call $2($5, GetFM20ReturnKey(), $6)"); insta::assert_snapshot!(plain_stderr, @r###" - error: The numbered capture group `$1` in the replacement text is ambiguous. + Error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}Call`. $1Call $2($5, GetFM20ReturnKey(), $6) ^^^^^ From 100db825abc72dd64d332fea500e5a99b59507d8 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 9 Nov 2023 17:06:38 +0800 Subject: [PATCH 12/22] fix: restructure logic, Windows requires closing mmap before write --- src/main.rs | 67 +++++++++++++++++++++++++++++------------------------ 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/src/main.rs b/src/main.rs index 13f383b..a0e4bb4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,7 +11,7 @@ use std::{ io::{stdout, Write}, ops::DerefMut, }; -use memmap2::{Mmap, MmapMut}; +use memmap2::MmapMut; use clap::Parser; pub(crate) use self::input::Source; @@ -36,65 +36,72 @@ fn main() -> Result<()> { Source::from_stdin() }; - let sources: Vec<(&Source, Result)> = sources - .iter() - .map(|source| (source, match source { + let mut errors = Vec::new(); + + let mut mmaps = Vec::new(); + for source in sources.iter() { + let maybe_mmap = match source { Source::File(path) => if path.exists() { - make_mmap(path) + make_mmap(&path) } else { Err(Error::InvalidPath(path.clone())) }, Source::Stdin => make_mmap_stdin() - })) - .collect(); + }; + + match maybe_mmap { + Ok(mmap) => mmaps.push(Some(mmap)), + Err(e) => { + mmaps.push(None); + errors.push((source, e)); + }, + }; + } + let needs_separator = sources.len() > 1; let replaced: Vec<_> = { use rayon::prelude::*; - sources - .par_iter() - .map(|(source, maybe_mmap)| (source, maybe_mmap.as_ref().map(|mmap| replacer.replace(mmap)))) + mmaps.par_iter() + .map(|maybe_mmap| (maybe_mmap.as_ref().map(|mmap| replacer.replace(&mmap)))) .collect() }; - let mut errors = Vec::new(); - let mut has_error = !errors.is_empty(); - - if options.preview || sources.get(0).map(|s| s.0) == Some(&Source::Stdin) { + if options.preview || sources.first() == Some(&Source::Stdin) { let mut handle = stdout().lock(); - for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { + for (source, replaced) in sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) { if needs_separator { writeln!(handle, "----- {} -----", source.display())?; } handle.write_all(replaced.as_ref().unwrap())?; } } else { - for (source, replaced) in replaced.iter().filter(|(_, r)| r.is_ok()) { + // Windows requires closing mmap before writing: + // > The requested operation cannot be performed on a file with a user-mapped section open + #[cfg(target_family = "windows")] + let replaced: Vec>> = replaced.into_iter().map(|r| r.map(|r| r.to_vec())).collect(); + #[cfg(target_family = "windows")] + drop(mmaps); + + for (source, replaced) in sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) { let path = match source { Source::File(path) => path, - Source::Stdin => { - unreachable!("stdin should only go previous branch") - } + _ => unreachable!("stdin should go previous branch"), }; - - if let Err(e) = write_with_temp(path, replaced.as_ref().unwrap()) { + if let Err(e) = write_with_temp(path, &replaced.unwrap()) { errors.push((source, e)); } } } - for (source, error) in replaced.iter().filter(|(_, r)| r.is_err()) { - eprintln!("error: {}: {:?}", source.display(), error); - has_error = true; - } - for (source, error) in errors { - eprintln!("error: {}: {:?}", source.display(), error); - } - - if has_error { + if !errors.is_empty() { + for (source, error) in errors { + eprintln!("error: {}: {:?}", source.display(), error); + } process::exit(1); } + Ok(()) } From 9f0c476b09486ca54fad6d75d18e2a9efc2c902f Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Fri, 10 Nov 2023 17:43:00 +0800 Subject: [PATCH 13/22] test: properly mark no-Windows test --- tests/cli.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/cli.rs b/tests/cli.rs index 98dfed9..03b5491 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -13,11 +13,14 @@ mod cli { assert_eq!(content, std::fs::read_to_string(path).unwrap()); } - #[cfg(target_family = "unix")] + // This should really be cfg_attr(target_family = "windows"), but wasi impl + // is nightly for now, and other impls are not part of std + #[cfg_attr(not(target_family = "unix"), ignore = "Windows symlinks are privileged")] fn create_soft_link>( src: &P, dst: &P, ) -> Result<()> { + #[cfg(target_family = "unix")] std::os::unix::fs::symlink(src, dst)?; Ok(()) @@ -51,7 +54,7 @@ mod cli { Ok(()) } - #[cfg(target_family = "unix")] + #[cfg_attr(target_family = "windows", ignore = "Windows symlinks are privileged")] #[test] fn in_place_following_symlink() -> Result<()> { let dir = tempfile::tempdir()?; From 601db39cae34b3a3a6a22bc393e49fe724de0cbb Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Fri, 10 Nov 2023 17:43:22 +0800 Subject: [PATCH 14/22] test: properly mark temp. ignored test --- tests/cli.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/cli.rs b/tests/cli.rs index 03b5491..1122a80 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -174,7 +174,8 @@ mod cli { // NOTE: styled terminal output is platform dependent, so convert to a // common format, in this case HTML, to check - //#[test] + #[ignore = "TODO: wait for proper colorization"] + #[test] fn ambiguous_replace_ensure_styling() { let styled_stderr = bad_replace_helper_styled("\t$1bad after"); let html_stderr = From 559dfcb949961d43a6deff7ef262016eb0f3c810 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Thu, 16 Nov 2023 19:56:40 +0800 Subject: [PATCH 15/22] fix: retain unsafe property of Mmap::map in separate function --- src/input.rs | 7 +++++-- src/main.rs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/input.rs b/src/input.rs index 4cb9e62..b078686 100644 --- a/src/input.rs +++ b/src/input.rs @@ -26,8 +26,11 @@ impl Source { } } -pub(crate) fn make_mmap(path: &PathBuf) -> Result { - Ok(unsafe { Mmap::map(&File::open(path)?)? }) +// TODO: memmap2 docs state that users should implement proper +// procedures to avoid problems the `unsafe` keyword indicate. +// This would be in a later PR. +pub(crate) unsafe fn make_mmap(path: &PathBuf) -> Result { + Ok(Mmap::map(&File::open(path)?)?) } pub(crate) fn make_mmap_stdin() -> Result { diff --git a/src/main.rs b/src/main.rs index a0e4bb4..3f00f4d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -42,7 +42,7 @@ fn main() -> Result<()> { for source in sources.iter() { let maybe_mmap = match source { Source::File(path) => if path.exists() { - make_mmap(&path) + unsafe { make_mmap(&path) } } else { Err(Error::InvalidPath(path.clone())) }, From 8fc2aa47703197311212c3cd9ea15f821332e930 Mon Sep 17 00:00:00 2001 From: Cosmic Horror Date: Thu, 16 Nov 2023 18:19:09 -0700 Subject: [PATCH 16/22] chore: `cargo fmt` --- src/input.rs | 6 ++++- src/main.rs | 50 +++++++++++++++++++++++++--------------- src/replacer/validate.rs | 14 +++++------ tests/cli.rs | 10 ++++++-- 4 files changed, 51 insertions(+), 29 deletions(-) diff --git a/src/input.rs b/src/input.rs index b078686..fb98e62 100644 --- a/src/input.rs +++ b/src/input.rs @@ -1,5 +1,9 @@ -use std::{path::PathBuf, fs::File, io::{stdin, Read}}; use memmap2::{Mmap, MmapOptions}; +use std::{ + fs::File, + io::{stdin, Read}, + path::PathBuf, +}; use crate::error::Result; diff --git a/src/main.rs b/src/main.rs index 3f00f4d..32df841 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,20 +4,20 @@ mod input; pub(crate) mod replacer; +use clap::Parser; +use memmap2::MmapMut; use std::{ fs, - process, - path::PathBuf, io::{stdout, Write}, ops::DerefMut, + path::PathBuf, + process, }; -use memmap2::MmapMut; -use clap::Parser; -pub(crate) use self::input::Source; pub(crate) use self::error::{Error, Result}; -use self::replacer::Replacer; +pub(crate) use self::input::Source; use self::input::{make_mmap, make_mmap_stdin}; +use self::replacer::Replacer; fn main() -> Result<()> { let options = cli::Options::parse(); @@ -37,16 +37,18 @@ fn main() -> Result<()> { }; let mut errors = Vec::new(); - + let mut mmaps = Vec::new(); for source in sources.iter() { let maybe_mmap = match source { - Source::File(path) => if path.exists() { - unsafe { make_mmap(&path) } - } else { - Err(Error::InvalidPath(path.clone())) - }, - Source::Stdin => make_mmap_stdin() + Source::File(path) => { + if path.exists() { + unsafe { make_mmap(&path) } + } else { + Err(Error::InvalidPath(path.clone())) + } + } + Source::Stdin => make_mmap_stdin(), }; match maybe_mmap { @@ -54,7 +56,7 @@ fn main() -> Result<()> { Err(e) => { mmaps.push(None); errors.push((source, e)); - }, + } }; } @@ -62,15 +64,20 @@ fn main() -> Result<()> { let replaced: Vec<_> = { use rayon::prelude::*; - mmaps.par_iter() - .map(|maybe_mmap| (maybe_mmap.as_ref().map(|mmap| replacer.replace(&mmap)))) + mmaps + .par_iter() + .map(|maybe_mmap| { + (maybe_mmap.as_ref().map(|mmap| replacer.replace(&mmap))) + }) .collect() }; if options.preview || sources.first() == Some(&Source::Stdin) { let mut handle = stdout().lock(); - for (source, replaced) in sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) { + for (source, replaced) in + sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) + { if needs_separator { writeln!(handle, "----- {} -----", source.display())?; } @@ -80,11 +87,16 @@ fn main() -> Result<()> { // Windows requires closing mmap before writing: // > The requested operation cannot be performed on a file with a user-mapped section open #[cfg(target_family = "windows")] - let replaced: Vec>> = replaced.into_iter().map(|r| r.map(|r| r.to_vec())).collect(); + let replaced: Vec>> = replaced + .into_iter() + .map(|r| r.map(|r| r.to_vec())) + .collect(); #[cfg(target_family = "windows")] drop(mmaps); - for (source, replaced) in sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) { + for (source, replaced) in + sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) + { let path = match source { Source::File(path) => path, _ => unreachable!("stdin should go previous branch"), diff --git a/src/replacer/validate.rs b/src/replacer/validate.rs index bbaf8c1..a7ce765 100644 --- a/src/replacer/validate.rs +++ b/src/replacer/validate.rs @@ -54,7 +54,11 @@ impl fmt::Display for InvalidReplaceCapture { for (byte_index, c) in original_replace.char_indices() { let (prefix, suffix, text) = match SpecialChar::new(c) { Some(c) => { - (Some("" /* special prefix */), Some("" /* special suffix */), c.render()) + ( + Some("" /* special prefix */), + Some("" /* special suffix */), + c.render(), + ) } None => { let (prefix, suffix) = if byte_index == invalid_ident.start @@ -93,10 +97,7 @@ impl fmt::Display for InvalidReplaceCapture { // This relies on all non-curly-braced capture chars being 1 byte let arrows_span = arrows_start.end_offset(invalid_ident.len()); let mut arrows = " ".repeat(arrows_span.start); - arrows.push_str(&format!( - "{}", - "^".repeat(arrows_span.len()) - )); + arrows.push_str(&format!("{}", "^".repeat(arrows_span.len()))); let ident = invalid_ident.slice(original_replace); let (number, the_rest) = ident.split_at(*num_leading_digits); @@ -107,8 +108,7 @@ impl fmt::Display for InvalidReplaceCapture { ); let hint_message = format!( "{}: Use curly braces to disambiguate it `{}`.", - "hint", - disambiguous + "hint", disambiguous ); writeln!(f, "{}", error_message)?; diff --git a/tests/cli.rs b/tests/cli.rs index 1122a80..15c367f 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -15,7 +15,10 @@ mod cli { // This should really be cfg_attr(target_family = "windows"), but wasi impl // is nightly for now, and other impls are not part of std - #[cfg_attr(not(target_family = "unix"), ignore = "Windows symlinks are privileged")] + #[cfg_attr( + not(target_family = "unix"), + ignore = "Windows symlinks are privileged" + )] fn create_soft_link>( src: &P, dst: &P, @@ -54,7 +57,10 @@ mod cli { Ok(()) } - #[cfg_attr(target_family = "windows", ignore = "Windows symlinks are privileged")] + #[cfg_attr( + target_family = "windows", + ignore = "Windows symlinks are privileged" + )] #[test] fn in_place_following_symlink() -> Result<()> { let dir = tempfile::tempdir()?; From 5168104eec6eb1ccde7aee18b7f414a601b51627 Mon Sep 17 00:00:00 2001 From: Cosmic Horror Date: Fri, 17 Nov 2023 19:50:25 -0700 Subject: [PATCH 17/22] chore: Resolve lone warning --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 32df841..f3945af 100644 --- a/src/main.rs +++ b/src/main.rs @@ -67,7 +67,7 @@ fn main() -> Result<()> { mmaps .par_iter() .map(|maybe_mmap| { - (maybe_mmap.as_ref().map(|mmap| replacer.replace(&mmap))) + maybe_mmap.as_ref().map(|mmap| replacer.replace(&mmap)) }) .collect() }; From 56f4d251835bf898ba4d1e00b9291fe56989146a Mon Sep 17 00:00:00 2001 From: Cosmic Horror Date: Sat, 18 Nov 2023 12:33:42 -0700 Subject: [PATCH 18/22] test: Test a variety of fs failure cases --- tests/cli.rs | 156 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 155 insertions(+), 1 deletion(-) diff --git a/tests/cli.rs b/tests/cli.rs index 15c367f..d33b923 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -3,7 +3,7 @@ mod cli { use anyhow::Result; use assert_cmd::Command; - use std::io::prelude::*; + use std::{fs, io::prelude::*, path::Path}; fn sd() -> Command { Command::cargo_bin(env!("CARGO_PKG_NAME")).expect("Error invoking sd") @@ -246,4 +246,158 @@ mod cli { .success() .stdout("bar\nfoo\nfoo"); } + + const UNTOUCHED_CONTENTS: &str = "untouched"; + + fn assert_fails_correctly( + command: &mut Command, + valid: &Path, + test_home: &Path, + snap_name: &str, + ) { + let failed_command = command.assert().failure().code(1); + + assert_eq!(fs::read_to_string(&valid).unwrap(), UNTOUCHED_CONTENTS); + + let stderr_orig = + std::str::from_utf8(&failed_command.get_output().stderr).unwrap(); + // Normalize unstable path bits + let stderr_norm = stderr_orig + .replace(test_home.to_str().unwrap(), "") + .replace('\\', "/"); + insta::assert_snapshot!(snap_name, stderr_norm); + } + + #[test] + fn correctly_fails_on_missing_file() -> Result<()> { + let test_dir = tempfile::Builder::new().prefix("sd-test-").tempdir()?; + let test_home = test_dir.path(); + + let valid = test_home.join("valid"); + fs::write(&valid, UNTOUCHED_CONTENTS)?; + let missing = test_home.join("missing"); + + assert_fails_correctly( + sd().args([".*", ""]).arg(&valid).arg(&missing), + &valid, + test_home, + "correctly_fails_on_missing_file", + ); + + Ok(()) + } + + #[cfg_attr(not(target_family = "unix"), ignore = "only runs on unix")] + #[test] + fn correctly_fails_on_unreadable_file() -> Result<()> { + #[cfg(not(target_family = "unix"))] + { + unreachable!("This test should be ignored"); + } + #[cfg(target_family = "unix")] + { + use std::os::unix::fs::OpenOptionsExt; + + let test_dir = + tempfile::Builder::new().prefix("sd-test-").tempdir()?; + let test_home = test_dir.path(); + + let valid = test_home.join("valid"); + fs::write(&valid, UNTOUCHED_CONTENTS)?; + let write_only = { + let path = test_home.join("write_only"); + let mut write_only_file = std::fs::OpenOptions::new() + .mode(0o333) + .create(true) + .write(true) + .open(&path)?; + write!(write_only_file, "unreadable")?; + path + }; + + assert_fails_correctly( + sd().args([".*", ""]).arg(&valid).arg(&write_only), + &valid, + test_home, + "correctly_fails_on_unreadable_file", + ); + + Ok(()) + } + } + + // Failing to create a temporary file in the same directory as the input is + // one of the failure cases that is past the "point of no return" (after we + // already start making replacements). This means that any files that could + // be modified are, and we report any failure cases + #[cfg_attr(not(target_family = "unix"), ignore = "only runs on unix")] + #[test] + fn reports_errors_on_atomic_file_swap_creation_failure() -> Result<()> { + #[cfg(not(target_family = "unix"))] + { + unreachable!("This test should be ignored"); + } + #[cfg(target_family = "unix")] + { + use std::os::unix::fs::PermissionsExt; + + const FIND_REPLACE: [&str; 2] = ["able", "ed"]; + const ORIG_TEXT: &str = "modifiable"; + const MODIFIED_TEXT: &str = "modified"; + + let test_dir = + tempfile::Builder::new().prefix("sd-test-").tempdir()?; + let test_home = test_dir.path(); + + let writable_dir = test_home.join("writable"); + fs::create_dir(&writable_dir)?; + let writable_dir_file = writable_dir.join("foo"); + fs::write(&writable_dir_file, ORIG_TEXT)?; + + let unwritable_dir = test_home.join("unwritable"); + fs::create_dir(&unwritable_dir)?; + let unwritable_dir_file1 = unwritable_dir.join("bar"); + fs::write(&unwritable_dir_file1, ORIG_TEXT)?; + let unwritable_dir_file2 = unwritable_dir.join("baz"); + fs::write(&unwritable_dir_file2, ORIG_TEXT)?; + let mut perms = fs::metadata(&unwritable_dir)?.permissions(); + perms.set_mode(0o555); + fs::set_permissions(&unwritable_dir, perms)?; + + let failed_command = sd() + .args(FIND_REPLACE) + .arg(&writable_dir_file) + .arg(&unwritable_dir_file1) + .arg(&unwritable_dir_file2) + .assert() + .failure() + .code(1); + + // Confirm that we modified the one file that we were able to + assert_eq!(fs::read_to_string(&writable_dir_file)?, MODIFIED_TEXT); + assert_eq!(fs::read_to_string(&unwritable_dir_file1)?, ORIG_TEXT); + assert_eq!(fs::read_to_string(&unwritable_dir_file2)?, ORIG_TEXT); + + let stderr_orig = + std::str::from_utf8(&failed_command.get_output().stderr) + .unwrap(); + // Normalize unstable path bits + let stderr_partial_norm = stderr_orig + .replace(test_home.to_str().unwrap(), "") + .replace('\\', "/"); + let tmp_file_rep = regex::Regex::new(r"\.tmp\w+")?; + let stderr_norm = + tmp_file_rep.replace_all(&stderr_partial_norm, ""); + insta::assert_snapshot!(stderr_norm); + + // Make the unwritable dir writable again, so it can be cleaned up + // when dropping the temp dir + let mut perms = fs::metadata(&unwritable_dir)?.permissions(); + perms.set_mode(0o777); + fs::set_permissions(&unwritable_dir, perms)?; + test_dir.close()?; + + Ok(()) + } + } } From fcd631d92bdbe255148a92b500e03a0ebff1dd7a Mon Sep 17 00:00:00 2001 From: Cosmic Horror Date: Fri, 17 Nov 2023 20:49:42 -0700 Subject: [PATCH 19/22] refactor: Add back `try_main()` --- src/main.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index f3945af..b4f7252 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,7 +19,14 @@ pub(crate) use self::input::Source; use self::input::{make_mmap, make_mmap_stdin}; use self::replacer::Replacer; -fn main() -> Result<()> { +fn main() { + if let Err(e) = try_main() { + eprintln!("error: {e}"); + process::exit(1); + } +} + +fn try_main() -> Result<()> { let options = cli::Options::parse(); let replacer = Replacer::new( From 8af2effd8a643bc8253da8bf250c6440b45bb102 Mon Sep 17 00:00:00 2001 From: Cosmic Horror Date: Fri, 17 Nov 2023 21:07:16 -0700 Subject: [PATCH 20/22] refactor: Rework error handling --- src/error.rs | 27 ++++++++++++++++++++--- src/main.rs | 60 ++++++++++++++++++---------------------------------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/src/error.rs b/src/error.rs index 0955d02..3a06758 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::{fmt, path::PathBuf}; use crate::replacer::InvalidReplaceCapture; @@ -14,13 +14,34 @@ pub enum Error { InvalidPath(PathBuf), #[error("{0}")] InvalidReplaceCapture(#[from] InvalidReplaceCapture), + #[error("{0}")] + FailedJobs(FailedJobs), } // pretty-print the error -impl std::fmt::Debug for Error { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Debug for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self) } } pub type Result = std::result::Result; + +pub struct FailedJobs(pub Vec<(PathBuf, Error)>); + +impl fmt::Display for FailedJobs { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("Failed processing some inputs\n")?; + for (source, error) in &self.0 { + writeln!(f, " {}: {}", source.display(), error)?; + } + + Ok(()) + } +} + +impl fmt::Debug for FailedJobs { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self) + } +} diff --git a/src/main.rs b/src/main.rs index b4f7252..c346ab6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -14,7 +14,7 @@ use std::{ process, }; -pub(crate) use self::error::{Error, Result}; +pub(crate) use self::error::{Error, FailedJobs, Result}; pub(crate) use self::input::Source; use self::input::{make_mmap, make_mmap_stdin}; use self::replacer::Replacer; @@ -43,28 +43,20 @@ fn try_main() -> Result<()> { Source::from_stdin() }; - let mut errors = Vec::new(); - let mut mmaps = Vec::new(); for source in sources.iter() { - let maybe_mmap = match source { + let mmap = match source { Source::File(path) => { if path.exists() { - unsafe { make_mmap(&path) } + unsafe { make_mmap(&path)? } } else { - Err(Error::InvalidPath(path.clone())) + return Err(Error::InvalidPath(path.to_owned())); } } - Source::Stdin => make_mmap_stdin(), + Source::Stdin => make_mmap_stdin()?, }; - match maybe_mmap { - Ok(mmap) => mmaps.push(Some(mmap)), - Err(e) => { - mmaps.push(None); - errors.push((source, e)); - } - }; + mmaps.push(mmap); } let needs_separator = sources.len() > 1; @@ -73,52 +65,42 @@ fn try_main() -> Result<()> { use rayon::prelude::*; mmaps .par_iter() - .map(|maybe_mmap| { - maybe_mmap.as_ref().map(|mmap| replacer.replace(&mmap)) - }) + .map(|mmap| replacer.replace(&mmap)) .collect() }; if options.preview || sources.first() == Some(&Source::Stdin) { let mut handle = stdout().lock(); - for (source, replaced) in - sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) - { + for (source, replaced) in sources.iter().zip(replaced) { if needs_separator { writeln!(handle, "----- {} -----", source.display())?; } - handle.write_all(replaced.as_ref().unwrap())?; + handle.write_all(&replaced)?; } } else { // Windows requires closing mmap before writing: // > The requested operation cannot be performed on a file with a user-mapped section open #[cfg(target_family = "windows")] - let replaced: Vec>> = replaced - .into_iter() - .map(|r| r.map(|r| r.to_vec())) - .collect(); + let replaced: Vec> = + replaced.into_iter().map(|r| r.to_vec()).collect(); #[cfg(target_family = "windows")] drop(mmaps); - for (source, replaced) in - sources.iter().zip(replaced).filter(|(_, r)| r.is_some()) - { - let path = match source { - Source::File(path) => path, + let mut failed_jobs = Vec::new(); + for (source, replaced) in sources.iter().zip(replaced) { + match source { + Source::File(path) => { + if let Err(e) = write_with_temp(path, &replaced) { + failed_jobs.push((path.to_owned(), e)); + } + } _ => unreachable!("stdin should go previous branch"), - }; - if let Err(e) = write_with_temp(path, &replaced.unwrap()) { - errors.push((source, e)); } } - } - - if !errors.is_empty() { - for (source, error) in errors { - eprintln!("error: {}: {:?}", source.display(), error); + if !failed_jobs.is_empty() { + return Err(Error::FailedJobs(FailedJobs(failed_jobs))); } - process::exit(1); } Ok(()) From c1cb8795d1d2bfa8b31cdc54663e8969936078d9 Mon Sep 17 00:00:00 2001 From: Cosmic Horror Date: Fri, 17 Nov 2023 21:13:00 -0700 Subject: [PATCH 21/22] test: Update snapshots --- tests/cli.rs | 8 ++++---- .../cli__cli__correctly_fails_on_missing_file.snap | 6 ++++++ .../cli__cli__correctly_fails_on_unreadable_file.snap | 6 ++++++ ...orts_errors_on_atomic_file_swap_creation_failure.snap | 9 +++++++++ 4 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 tests/snapshots/cli__cli__correctly_fails_on_missing_file.snap create mode 100644 tests/snapshots/cli__cli__correctly_fails_on_unreadable_file.snap create mode 100644 tests/snapshots/cli__cli__reports_errors_on_atomic_file_swap_creation_failure.snap diff --git a/tests/cli.rs b/tests/cli.rs index d33b923..8c207c0 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -137,7 +137,7 @@ mod cli { fn ambiguous_replace_basic() { let plain_stderr = bad_replace_helper_plain("before $1bad after"); insta::assert_snapshot!(plain_stderr, @r###" - Error: The numbered capture group `$1` in the replacement text is ambiguous. + error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}bad`. before $1bad after ^^^^ @@ -148,7 +148,7 @@ mod cli { fn ambiguous_replace_variable_width() { let plain_stderr = bad_replace_helper_plain("\r\n\t$1bad\r"); insta::assert_snapshot!(plain_stderr, @r###" - Error: The numbered capture group `$1` in the replacement text is ambiguous. + error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}bad`. ␍␊␉$1bad␍ ^^^^ @@ -159,7 +159,7 @@ mod cli { fn ambiguous_replace_multibyte_char() { let plain_stderr = bad_replace_helper_plain("😈$1bad😇"); insta::assert_snapshot!(plain_stderr, @r###" - Error: The numbered capture group `$1` in the replacement text is ambiguous. + error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}bad`. 😈$1bad😇 ^^^^ @@ -171,7 +171,7 @@ mod cli { let plain_stderr = bad_replace_helper_plain("$1Call $2($5, GetFM20ReturnKey(), $6)"); insta::assert_snapshot!(plain_stderr, @r###" - Error: The numbered capture group `$1` in the replacement text is ambiguous. + error: The numbered capture group `$1` in the replacement text is ambiguous. hint: Use curly braces to disambiguate it `${1}Call`. $1Call $2($5, GetFM20ReturnKey(), $6) ^^^^^ diff --git a/tests/snapshots/cli__cli__correctly_fails_on_missing_file.snap b/tests/snapshots/cli__cli__correctly_fails_on_missing_file.snap new file mode 100644 index 0000000..eb2a82e --- /dev/null +++ b/tests/snapshots/cli__cli__correctly_fails_on_missing_file.snap @@ -0,0 +1,6 @@ +--- +source: tests/cli.rs +expression: stderr_norm +--- +error: invalid path: /missing + diff --git a/tests/snapshots/cli__cli__correctly_fails_on_unreadable_file.snap b/tests/snapshots/cli__cli__correctly_fails_on_unreadable_file.snap new file mode 100644 index 0000000..3dcab19 --- /dev/null +++ b/tests/snapshots/cli__cli__correctly_fails_on_unreadable_file.snap @@ -0,0 +1,6 @@ +--- +source: tests/cli.rs +expression: stderr_norm +--- +error: Permission denied (os error 13) + diff --git a/tests/snapshots/cli__cli__reports_errors_on_atomic_file_swap_creation_failure.snap b/tests/snapshots/cli__cli__reports_errors_on_atomic_file_swap_creation_failure.snap new file mode 100644 index 0000000..07930dc --- /dev/null +++ b/tests/snapshots/cli__cli__reports_errors_on_atomic_file_swap_creation_failure.snap @@ -0,0 +1,9 @@ +--- +source: tests/cli.rs +expression: stderr_norm +--- +error: Failed processing some inputs + /unwritable/bar: Permission denied (os error 13) at path "/unwritable/" + /unwritable/baz: Permission denied (os error 13) at path "/unwritable/" + + From f8613a9b7e88587aaad598aff363445adcc8ae45 Mon Sep 17 00:00:00 2001 From: Blair Noctis Date: Sun, 19 Nov 2023 04:01:19 +0800 Subject: [PATCH 22/22] test: fix path inconsistency --- tests/cli.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli.rs b/tests/cli.rs index 8c207c0..1030dc1 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -347,7 +347,7 @@ mod cli { let test_dir = tempfile::Builder::new().prefix("sd-test-").tempdir()?; - let test_home = test_dir.path(); + let test_home = test_dir.path().canonicalize()?; let writable_dir = test_home.join("writable"); fs::create_dir(&writable_dir)?;