Adjust force-update on inline snapshots to only view within the string

CHANGELOG.md

@@ -4,6 +4,11 @@ All notable changes to insta and cargo-insta are documented here.
 
 ## 1.41.0
 
+- `--force-update-snapshots` has more conservative and consistent behavior for
+  inline snapshots. As a side-effect of this, only the content within the inline
+  snapshot delimiters are assessed for changes, not the delimiters (e.g. `###`).
+  #581
+
 ## 1.40.0
 
 - `cargo-insta` no longer panics when running `cargo insta test --accept --workspace`

cargo-insta/src/cli.rs

@@ -1077,12 +1077,14 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
         let is_inline = snapshot_container.snapshot_file().is_none();
         for snapshot_ref in snapshot_container.iter_snapshots() {
             if cmd.as_json {
+                let old_snapshot = snapshot_ref.old.as_ref().map(|x| x.contents_string());
+                let new_snapshot = snapshot_ref.new.contents_string();
                 let info = if is_inline {
                     SnapshotKey::InlineSnapshot {
                         path: &target_file,
                         line: snapshot_ref.line.unwrap(),
-                        old_snapshot: snapshot_ref.old.as_ref().map(|x| x.contents_str()),
-                        new_snapshot: snapshot_ref.new.contents_str(),
+                        old_snapshot: old_snapshot.as_deref(),
+                        new_snapshot: &new_snapshot,
                         expression: snapshot_ref.new.metadata().expression(),
                     }
                 } else {

cargo-insta/src/container.rs

@@ -3,6 +3,7 @@ use std::fs;
 use std::path::{Path, PathBuf};
 
 use insta::Snapshot;
+pub(crate) use insta::SnapshotKind;
 use insta::_cargo_insta_support::{ContentError, PendingInlineSnapshot};
 
 use crate::inline::FilePatcher;
@@ -14,12 +15,6 @@ pub(crate) enum Operation {
     Skip,
 }
 
-#[derive(Debug)]
-pub(crate) enum SnapshotContainerKind {
-    Inline,
-    External,
-}
-
 #[derive(Debug)]
 pub(crate) struct PendingSnapshot {
     #[allow(dead_code)]
@@ -54,7 +49,7 @@ impl PendingSnapshot {
 pub(crate) struct SnapshotContainer {
     snapshot_path: PathBuf,
     target_path: PathBuf,
-    kind: SnapshotContainerKind,
+    kind: SnapshotKind,
     snapshots: Vec<PendingSnapshot>,
     patcher: Option<FilePatcher>,
 }
@@ -63,11 +58,11 @@ impl SnapshotContainer {
     pub(crate) fn load(
         snapshot_path: PathBuf,
         target_path: PathBuf,
-        kind: SnapshotContainerKind,
+        kind: SnapshotKind,
     ) -> Result<SnapshotContainer, Box<dyn Error>> {
         let mut snapshots = Vec::new();
         let patcher = match kind {
-            SnapshotContainerKind::External => {
+            SnapshotKind::File => {
                 let old = if fs::metadata(&target_path).is_err() {
                     None
                 } else {
@@ -83,7 +78,7 @@ impl SnapshotContainer {
                 });
                 None
             }
-            SnapshotContainerKind::Inline => {
+            SnapshotKind::Inline => {
                 let mut pending_vec = PendingInlineSnapshot::load_batch(&snapshot_path)?;
                 let mut have_new = false;
 
@@ -139,8 +134,8 @@ impl SnapshotContainer {
 
     pub(crate) fn snapshot_file(&self) -> Option<&Path> {
         match self.kind {
-            SnapshotContainerKind::External => Some(&self.target_path),
-            SnapshotContainerKind::Inline => None,
+            SnapshotKind::File => Some(&self.target_path),
+            SnapshotKind::Inline => None,
         }
     }
 

cargo-insta/src/walk.rs

@@ -5,7 +5,7 @@ use std::path::Path;
 use ignore::overrides::OverrideBuilder;
 use ignore::{DirEntry, Walk, WalkBuilder};
 
-use crate::container::{SnapshotContainer, SnapshotContainerKind};
+use crate::container::{SnapshotContainer, SnapshotKind};
 
 #[derive(Debug, Copy, Clone)]
 pub(crate) struct FindFlags {
@@ -37,15 +37,15 @@ pub(crate) fn find_pending_snapshots<'a>(
                 Some(SnapshotContainer::load(
                     new_path,
                     old_path,
-                    SnapshotContainerKind::External,
+                    SnapshotKind::File,
                 ))
             } else if fname.starts_with('.') && fname.ends_with(".pending-snap") {
                 let mut target_path = e.path().to_path_buf();
                 target_path.set_file_name(&fname[1..fname.len() - 13]);
                 Some(SnapshotContainer::load(
                     e.path().to_path_buf(),
                     target_path,
-                    SnapshotContainerKind::Inline,
+                    SnapshotKind::Inline,
                 ))
             } else {
                 None

cargo-insta/tests/main.rs

@@ -905,13 +905,13 @@ Hello, world!
 }
 
 #[test]
-fn test_force_update_inline_snapshot() {
+fn test_force_update_inline_snapshot_linebreaks() {
     let test_project = TestFiles::new()
         .add_file(
             "Cargo.toml",
             r#"
 [package]
-name = "force-update-inline"
+name = "force-update-inline-linebreaks"
 version = "0.1.0"
 edition = "2021"
 
@@ -924,8 +924,11 @@ insta = { path = '$PROJECT_PATH' }
             "src/lib.rs",
             r#####"
 #[test]
-fn test_excessive_hashes() {
-    insta::assert_snapshot!("foo", @r####"foo"####);
+fn test_linebreaks() {
+    insta::assert_snapshot!("foo", @r####"
+    foo
+
+    "####);
 }
 "#####
                 .to_string(),
@@ -950,16 +953,143 @@ fn test_excessive_hashes() {
     assert_snapshot!(test_project.diff("src/lib.rs"), @r#####"
     --- Original: src/lib.rs
     +++ Updated: src/lib.rs
-    @@ -1,5 +1,5 @@
+    @@ -1,8 +1,5 @@
 
      #[test]
-     fn test_excessive_hashes() {
-    -    insta::assert_snapshot!("foo", @r####"foo"####);
+     fn test_linebreaks() {
+    -    insta::assert_snapshot!("foo", @r####"
+    -    foo
+    -    
+    -    "####);
     +    insta::assert_snapshot!("foo", @"foo");
      }
     "#####);
 }
 
+#[test]
+fn test_force_update_inline_snapshot_hashes() {
+    let test_project = TestFiles::new()
+        .add_file(
+            "Cargo.toml",
+            r#"
+[package]
+name = "force-update-inline-hashes"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+insta = { path = '$PROJECT_PATH' }
+"#
+            .to_string(),
+        )
+        .add_file(
+            "src/lib.rs",
+            r#####"
+#[test]
+fn test_excessive_hashes() {
+    insta::assert_snapshot!("foo", @r####"foo"####);
+}
+"#####
+                .to_string(),
+        )
+        .create_project();
+
+    // Run the test with --force-update-snapshots and --accept
+    let output = test_project
+        .cmd()
+        .args([
+            "test",
+            "--force-update-snapshots",
+            "--accept",
+            "--",
+            "--nocapture",
+        ])
+        .output()
+        .unwrap();
+
+    assert_success(&output);
+
+    // TODO: we would like to update the number of hashes, but that's not easy
+    // given the reasons at https://github.com/mitsuhiko/insta/pull/573. So this
+    // result asserts the current state rather than the desired state.
+    assert_snapshot!(test_project.diff("src/lib.rs"), @"");
+}
+
+#[test]
+fn test_inline_snapshot_indent() {
+    let test_project = TestFiles::new()
+        .add_file(
+            "Cargo.toml",
+            r#"
+[package]
+name = "inline-indent"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+insta = { path = '$PROJECT_PATH' }
+"#
+            .to_string(),
+        )
+        .add_file(
+            "src/lib.rs",
+            r#####"
+#[test]
+fn test_wrong_indent_force() {
+    insta::assert_snapshot!(r#"
+    foo
+    foo
+    "#, @r#"
+                foo
+                foo
+    "#);
+}
+"#####
+                .to_string(),
+        )
+        .create_project();
+
+    // Confirm the test passes despite the indent
+    let output = test_project
+        .cmd()
+        .args(["test", "--check", "--", "--nocapture"])
+        .output()
+        .unwrap();
+    assert_success(&output);
+
+    // Then run the test with --force-update-snapshots and --accept to confirm
+    // the new snapshot is written
+    let output = test_project
+        .cmd()
+        .args([
+            "test",
+            "--force-update-snapshots",
+            "--accept",
+            "--",
+            "--nocapture",
+        ])
+        .output()
+        .unwrap();
+    assert_success(&output);
+
+    // https://github.com/mitsuhiko/insta/pull/563 will fix the starting &
+    // ending newlines
+    assert_snapshot!(test_project.diff("src/lib.rs"), @r##"
+    --- Original: src/lib.rs
+    +++ Updated: src/lib.rs
+    @@ -5,7 +5,7 @@
+         foo
+         foo
+         "#, @r#"
+    -                foo
+    -                foo
+    +    foo
+    +    foo
+         "#);
+     }
+    "##);
+}
+
 #[test]
 fn test_hashtag_escape_in_inline_snapshot() {
     let test_project = TestFiles::new()
@@ -981,7 +1111,7 @@ insta = { path = '$PROJECT_PATH' }
             r#"
 #[test]
 fn test_hashtag_escape() {
-    insta::assert_snapshot!("Value with #### hashtags\n", @"");
+    insta::assert_snapshot!("Value with\n#### hashtags\n", @"");
 }
 "#
             .to_string(),
@@ -999,13 +1129,14 @@ fn test_hashtag_escape() {
     assert_snapshot!(test_project.diff("src/main.rs"), @r######"
     --- Original: src/main.rs
     +++ Updated: src/main.rs
-    @@ -1,5 +1,7 @@
+    @@ -1,5 +1,8 @@
 
      #[test]
      fn test_hashtag_escape() {
-    -    insta::assert_snapshot!("Value with #### hashtags\n", @"");
-    +    insta::assert_snapshot!("Value with #### hashtags\n", @r#####"
-    +    Value with #### hashtags
+    -    insta::assert_snapshot!("Value with\n#### hashtags\n", @"");
+    +    insta::assert_snapshot!("Value with\n#### hashtags\n", @r#####"
+    +    Value with
+    +    #### hashtags
     +    "#####);
      }
     "######);

insta/src/lib.rs

@@ -287,7 +287,7 @@ mod glob;
 mod test;
 
 pub use crate::settings::Settings;
-pub use crate::snapshot::{MetaData, Snapshot};
+pub use crate::snapshot::{MetaData, Snapshot, SnapshotKind};
 
 /// Exposes some library internals.
 ///

insta/src/output.rs

@@ -103,7 +103,7 @@ impl<'a> SnapshotPrinter<'a> {
     fn print_snapshot(&self) {
         print_line(term_width());
 
-        let new_contents = self.new_snapshot.contents_str();
+        let new_contents = self.new_snapshot.contents_string();
 
         let width = term_width();
         if self.show_info {
@@ -118,15 +118,17 @@ impl<'a> SnapshotPrinter<'a> {
     }
 
     fn print_changeset(&self) {
-        let old = self.old_snapshot.as_ref().map_or("", |x| x.contents_str());
-        let new = self.new_snapshot.contents_str();
-        let newlines_matter = newlines_matter(old, new);
+        let old: String = self
+            .old_snapshot
+            .map_or("".to_string(), |x| x.contents_string());
+        let new = self.new_snapshot.contents_string();
+        let newlines_matter = newlines_matter(old.as_str(), new.as_str());
 
         let width = term_width();
         let diff = TextDiff::configure()
             .algorithm(Algorithm::Patience)
             .timeout(Duration::from_millis(500))
-            .diff_lines(old, new);
+            .diff_lines(old.as_str(), new.as_str());
         print_line(width);
 
         if self.show_info {