Overall reorganization (#265)

* refactor!: overall reorganization

* fmt: tab to spaces

* feat: split out funcs for better error collection

* feat: replace Vec-varianted Source with Vec<Source>

* docs: more generic description for Error::InvalidPath

* fmt: reduce signature verbosity

* internal: decolorize to prepare for proper colorization

* fmt: tab to spaces (again)

* refactor: break up App into main.rs, rm utils.rs

* test: only run cli::in_place_following_symlink on Unix, symlinks are privileged on Windows

* test: update insta snapshots due to Replacer::new()? in main()

* fix: restructure logic, Windows requires closing mmap before write

* test: properly mark no-Windows test

* test: properly mark temp. ignored test

* fix: retain unsafe property of Mmap::map in separate function

* chore: `cargo fmt`

* chore: Resolve lone warning

* test: Test a variety of fs failure cases

* refactor: Add back `try_main()`

* refactor: Rework error handling

* test: Update snapshots

* test: fix path inconsistency

---------

Co-authored-by: Cosmic Horror <CosmicHorrorDev@pm.me>

.editorconfig

@@ -1,2 +1,6 @@
+[*]
+indent_style = space
+indent_size = 4
+
 [*.rs]
 max_line_length = 80

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"
@@ -324,12 +315,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 +328,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"
@@ -650,14 +624,12 @@ name = "sd"
 version = "1.0.0"
 dependencies = [
  "ansi-to-html",
- "ansi_term",
  "anyhow",
  "assert_cmd",
  "clap",
  "clap_mangen",
  "console",
  "insta",
- "is-terminal",
  "memmap2",
  "proptest",
  "rayon",
@@ -805,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"

Cargo.toml

@@ -33,8 +33,6 @@ unescape = "0.1.0"
 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]

src/error.rs

@@ -1,7 +1,4 @@
-use std::{
+use std::{fmt, path::PathBuf};
-    fmt::{self, Write},
-    path::PathBuf,
-};
 
 use crate::replacer::InvalidReplaceCapture;
 
@@ -13,37 +10,38 @@ 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("failed processing files:\n{0}")]
-    FailedProcessing(FailedJobs),
     #[error("{0}")]
     InvalidReplaceCapture(#[from] InvalidReplaceCapture),
-}
+    #[error("{0}")]
-
+    FailedJobs(FailedJobs),
-pub struct FailedJobs(Vec<(PathBuf, Error)>);
-
-impl From<Vec<(PathBuf, Error)>> 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 {
+impl fmt::Debug for Error {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", self)
     }
 }
 
 pub type Result<T, E = Error> = std::result::Result<T, E>;
+
+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)
+    }
+}

src/input.rs

@@ -1,98 +1,48 @@
-use std::{fs::File, io::prelude::*, path::PathBuf};
+use memmap2::{Mmap, MmapOptions};
+use std::{
+    fs::File,
+    io::{stdin, Read},
+    path::PathBuf,
+};
 
-use crate::{Error, Replacer, Result};
+use crate::error::Result;
 
-use is_terminal::IsTerminal;
+#[derive(Debug, PartialEq)]
-
-#[derive(Debug)]
 pub(crate) enum Source {
     Stdin,
-    Files(Vec<PathBuf>),
+    File(PathBuf),
 }
 
-pub(crate) struct App {
+impl Source {
-    replacer: Replacer,
+    pub(crate) fn from_paths(paths: Vec<PathBuf>) -> Vec<Self> {
-    source: Source,
+        paths.into_iter().map(Self::File).collect()
-}
-
-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 {
+    pub(crate) fn from_stdin() -> Vec<Self> {
-        Self { source, replacer }
+        vec![Self::Stdin]
     }
-    pub(crate) fn run(&self, preview: bool) -> Result<()> {
-        let is_tty = std::io::stdout().is_terminal();
 
-        match (&self.source, preview) {
+    pub(crate) fn display(&self) -> String {
-            (Source::Stdin, true) => self.stdin_replace(is_tty),
+        match self {
-            (Source::Stdin, false) => self.stdin_replace(is_tty),
+            Self::Stdin => "STDIN".to_string(),
-            (Source::Files(paths), false) => {
+            Self::File(path) => format!("FILE {}", path.display()),
-                use rayon::prelude::*;
-
-                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 failed_jobs.is_empty() {
-                    Ok(())
-                } else {
-                    let failed_jobs =
-                        crate::error::FailedJobs::from(failed_jobs);
-                    Err(Error::FailedProcessing(failed_jobs))
-                }
-            }
-            (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()
-                            )?;
-                        }
-
-                        handle
-                            .write_all(&self.replacer.replace_preview(&file))?;
-                        writeln!(handle)?;
-                    }
-
-                    Ok(())
-                })
-            }
         }
     }
 }
+
+// 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<Mmap> {
+    Ok(Mmap::map(&File::open(path)?)?)
+}
+
+pub(crate) fn make_mmap_stdin() -> Result<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()?;
+    Ok(mmap)
+}

src/main.rs

@@ -3,20 +3,25 @@ mod error;
 mod input;
 
 pub(crate) mod replacer;
-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;
 
 use clap::Parser;
+use memmap2::MmapMut;
+use std::{
+    fs,
+    io::{stdout, Write},
+    ops::DerefMut,
+    path::PathBuf,
+    process,
+};
+
+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;
 
 fn main() {
     if let Err(e) = try_main() {
-        eprintln!("{}: {}", Style::from(Color::Red).bold().paint("error"), e);
+        eprintln!("error: {e}");
         process::exit(1);
     }
 }
@@ -24,22 +29,104 @@ fn main() {
 fn try_main() -> Result<()> {
     let options = cli::Options::parse();
 
-    let source = if !options.files.is_empty() {
+    let replacer = Replacer::new(
-        Source::Files(options.files)
+        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::Stdin
+        Source::from_stdin()
     };
 
-    App::new(
+    let mut mmaps = Vec::new();
-        source,
+    for source in sources.iter() {
-        Replacer::new(
+        let mmap = match source {
-            options.find,
+            Source::File(path) => {
-            options.replace_with,
+                if path.exists() {
-            options.literal_mode,
+                    unsafe { make_mmap(&path)? }
-            options.flags,
+                } else {
-            options.replacements,
+                    return Err(Error::InvalidPath(path.to_owned()));
-        )?,
+                }
-    )
+            }
-    .run(options.preview)?;
+            Source::Stdin => make_mmap_stdin()?,
+        };
+
+        mmaps.push(mmap);
+    }
+
+    let needs_separator = sources.len() > 1;
+
+    let replaced: Vec<_> = {
+        use rayon::prelude::*;
+        mmaps
+            .par_iter()
+            .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) {
+            if needs_separator {
+                writeln!(handle, "----- {} -----", source.display())?;
+            }
+            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<Vec<u8>> =
+            replaced.into_iter().map(|r| r.to_vec()).collect();
+        #[cfg(target_family = "windows")]
+        drop(mmaps);
+
+        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 !failed_jobs.is_empty() {
+            return Err(Error::FailedJobs(FailedJobs(failed_jobs)));
+        }
+    }
+
+    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(())
 }

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::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(),
             )
@@ -74,20 +74,7 @@ impl Replacer {
         })
     }
 
-    pub(crate) fn has_matches(&self, content: &[u8]) -> bool {
+    pub(crate) fn replace<'a>(&'a self, content: &'a [u8]) -> Cow<'a, [u8]> {
-        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],
-    ) -> std::borrow::Cow<'a, [u8]> {
         let regex = &self.regex;
         let limit = self.replacements;
         let use_color = false;
@@ -116,7 +103,7 @@ impl Replacer {
         regex: &regex::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();
@@ -129,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;
@@ -148,65 +125,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(())
-    }
 }

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,23 @@ 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)
                     };
@@ -97,22 +97,18 @@ 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!(
+        arrows.push_str(&format!("{}", "^".repeat(arrows_span.len())));
-            "{}",
-            Style::new().bold().paint("^".repeat(arrows_span.len()))
-        ));
 
         let ident = invalid_ident.slice(original_replace);
         let (number, the_rest) = ident.split_at(*num_leading_digits);
         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"),
+            "hint", disambiguous
-            Style::new().bold().paint(disambiguous)
         );
 
         writeln!(f, "{}", error_message)?;

src/utils.rs

@@ -1,3 +0,0 @@
-pub(crate) fn unescape(s: &str) -> Option<String> {
-    unescape::unescape(s)
-}

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")
@@ -13,14 +13,18 @@ mod cli {
         assert_eq!(content, std::fs::read_to_string(path).unwrap());
     }
 
+    // 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<P: AsRef<std::path::Path>>(
         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 +57,10 @@ mod cli {
         Ok(())
     }
 
+    #[cfg_attr(
+        target_family = "windows",
+        ignore = "Windows symlinks are privileged"
+    )]
     #[test]
     fn in_place_following_symlink() -> Result<()> {
         let dir = tempfile::tempdir()?;
@@ -81,11 +89,7 @@ mod cli {
         sd().args(["-p", "abc\\d+", "", file.path().to_str().unwrap()])
             .assert()
             .success()
-            .stdout(format!(
+            .stdout("def");
-                "{}{}def\n",
-                ansi_term::Color::Blue.prefix(),
-                ansi_term::Color::Blue.suffix()
-            ));
 
         assert_file(file.path(), "abc123def");
 
@@ -113,13 +117,7 @@ mod cli {
 
     fn bad_replace_helper_plain(replace: &str) -> String {
         let stderr = bad_replace_helper_styled(replace);
-
+        stderr
-        // TODO: no easy way to toggle off styling yet. Add a `--color <when>`
-        // 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()
     }
 
     #[test]
@@ -182,6 +180,7 @@ mod cli {
 
     // NOTE: styled terminal output is platform dependent, so convert to a
     // common format, in this case HTML, to check
+    #[ignore = "TODO: wait for proper colorization"]
     #[test]
     fn ambiguous_replace_ensure_styling() {
         let styled_stderr = bad_replace_helper_styled("\t$1bad after");
@@ -225,10 +224,7 @@ mod cli {
         ])
         .assert()
         .success()
-        .stdout(format!(
+        .stdout("bar\nfoo\nfoo");
-            "{}\nfoo\nfoo\n",
-            ansi_term::Color::Blue.paint("bar")
-        ));
 
         Ok(())
     }
@@ -250,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(), "<test_home>")
+            .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().canonicalize()?;
+
+            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(), "<test_home>")
+                .replace('\\', "/");
+            let tmp_file_rep = regex::Regex::new(r"\.tmp\w+")?;
+            let stderr_norm =
+                tmp_file_rep.replace_all(&stderr_partial_norm, "<tmp_file>");
+            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(())
+        }
+    }
 }

tests/snapshots/cli__cli__correctly_fails_on_missing_file.snap

@@ -0,0 +1,6 @@
+---
+source: tests/cli.rs
+expression: stderr_norm
+---
+error: invalid path: <test_home>/missing
+

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)
+

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
+    <test_home>/unwritable/bar: Permission denied (os error 13) at path "<test_home>/unwritable/<tmp_file>"
+    <test_home>/unwritable/baz: Permission denied (os error 13) at path "<test_home>/unwritable/<tmp_file>"
+
+