Repository: mitsuhiko/insta
Pull request #581 (2024-09-01 23:39:42)

Adjust force-update on inline snapshots to only view within the string
This solves the issue in #573 for the moment:
- When `--force-update-snapshots` in passed, we only update inline snapshots when there's some difference _within_ the string, such as an additional linebreak at the start or the end
- We no longer update the surrounding hashes. In the existing code, this happened because we were writing too many inline snapshots, not because we were actually checking the number of hashes
  - Checking the number of hashes isn't easy to do — reason outlined at https://github.com/mitsuhiko/insta/pull/573#issuecomment-2323500808
- The main change in the code is that we store the unnormalized snapshot and then normalize when we need to. The existing code stored the normalized version, which meant we couldn't evaluate whether the unnormalized versions were different

It's a decent number of changes, but will integrate nicely with https://github.com/mitsuhiko/insta/pull/563.

~(FYI we currently don't look at the indentation, but we could adjust this)~ Now indentation works too

max-sixty: OK, I think this is in a pretty good state now — again, more changes than ideal — but I think moving the library to be simpler, fewer corners
max-sixty: @mitsuhiko this one is top of my list for you to review when you get the chance to review things; think it's a much better experience for users; somewhat to your point when merging the PR that enabled forcing inline snapshot updates
max-sixty: @mitsuhiko I think the benefits of this are large enough, and I would like to minimize conflicts with #489, that it's worth me merging without waiting longer for a review. The main drawback is potentially one of performance — the current code does many more allocations, albeit in non-perf-sensitive code.

I recognize you gave me merge permissions with the understanding I would be careful, so:
- For the future, was merging this OK or would you have preferred I wait?
- I'll address any objections
- If you strongly object, I'll own reverting and rectifying any conflicts

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1081998..cd5b152 100644
--- a/CHANGELOG.md
+++ b/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`
diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs
index 03c775b..e3eafaf 100644
--- a/cargo-insta/src/cli.rs
+++ b/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 {
diff --git a/cargo-insta/src/container.rs b/cargo-insta/src/container.rs
index 6532058..6a2ded5 100644
--- a/cargo-insta/src/container.rs
+++ b/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,
         }
     }
 
diff --git a/cargo-insta/src/walk.rs b/cargo-insta/src/walk.rs
index 5ff9f75..d94c651 100644
--- a/cargo-insta/src/walk.rs
+++ b/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,7 +37,7 @@ 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();
@@ -45,7 +45,7 @@ pub(crate) fn find_pending_snapshots<'a>(
                 Some(SnapshotContainer::load(
                     e.path().to_path_buf(),
                     target_path,
-                    SnapshotContainerKind::Inline,
+                    SnapshotKind::Inline,
                 ))
             } else {
                 None
diff --git a/insta/src/lib.rs b/insta/src/lib.rs
index 4de7fc2..574d380 100644
--- a/insta/src/lib.rs
+++ b/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.
 ///
diff --git a/insta/src/output.rs b/insta/src/output.rs
index 075e914..5c9d103 100644
--- a/insta/src/output.rs
+++ b/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 {
diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs
index 22deb33..ad8a1dd 100644
--- a/insta/src/runtime.rs
+++ b/insta/src/runtime.rs
@@ -8,14 +8,17 @@ use std::str;
 use std::sync::{Arc, Mutex};
 use std::{borrow::Cow, env};
 
-use crate::env::{
-    get_cargo_workspace, get_tool_config, memoize_snapshot_file, snapshot_update_behavior,
-    OutputBehavior, SnapshotUpdateBehavior, ToolConfig,
-};
 use crate::output::SnapshotPrinter;
 use crate::settings::Settings;
 use crate::snapshot::{MetaData, PendingInlineSnapshot, Snapshot, SnapshotContents};
 use crate::utils::{path_to_storage, style};
+use crate::{
+    env::{
+        get_cargo_workspace, get_tool_config, memoize_snapshot_file, snapshot_update_behavior,
+        OutputBehavior, SnapshotUpdateBehavior, ToolConfig,
+    },
+    snapshot::SnapshotKind,
+};
 
 lazy_static::lazy_static! {
     static ref TEST_NAME_COUNTERS: Mutex<BTreeMap<String, usize>> =
@@ -78,6 +81,7 @@ impl<'a> From<&'a str> for ReferenceValue<'a> {
     }
 }
 
+#[derive(Debug)]
 /// A reference to a snapshot
 pub enum ReferenceValue<'a> {
     /// A file snapshot, where the inner value is the snapshot name.
@@ -289,7 +293,7 @@ impl<'a> SnapshotAssertionContext<'a> {
                     module_path.replace("::", "__"),
                     None,
                     MetaData::default(),
-                    SnapshotContents::from_inline(contents),
+                    SnapshotContents::new(contents.to_string(), SnapshotKind::Inline),
                 ));
             }
         };
@@ -643,7 +647,12 @@ pub fn assert_snapshot(
     let new_snapshot_value =
         Settings::with(|settings| settings.filters().apply_to(new_snapshot_value));
 
-    let new_snapshot = ctx.new_snapshot(new_snapshot_value.into(), expr);
+    let kind = match ctx.snapshot_file {
+        Some(_) => SnapshotKind::File,
+        None => SnapshotKind::Inline,
+    };
+    let new_snapshot =
+        ctx.new_snapshot(SnapshotContents::new(new_snapshot_value.into(), kind), expr);
 
     // memoize the snapshot file if requested.
     if let Some(ref snapshot_file) = ctx.snapshot_file {
@@ -678,7 +687,22 @@ pub fn assert_snapshot(
             tool_config.snapshot_update(),
             crate::env::SnapshotUpdate::Force
         ) {
-            ctx.update_snapshot(new_snapshot)?;
+            // Avoid creating new files if contents match exactly. In
+            // particular, this would otherwise create lots of unneeded files
+            // for inline snapshots
+            //
+            // Note that there's a check down the stack on whether the file
+            // contents match exactly for file snapshots; probably we should
+            // combine that check with `matches_fully` and then use a single
+            // check for whether we force update snapshots.
+            let matches_fully = &ctx
+                .old_snapshot
+                .as_ref()
+                .map(|x| x.matches_fully(&new_snapshot))
+                .unwrap_or(false);
+            if !matches_fully {
+                ctx.update_snapshot(new_snapshot)?;
+            }
         }
     // otherwise print information and update snapshots.
     } else {
diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs
index 9ff600b..05463c5 100644
--- a/insta/src/snapshot.rs
+++ b/insta/src/snapshot.rs
@@ -1,10 +1,10 @@
-use std::borrow::Cow;
 use std::env;
 use std::error::Error;
 use std::fs;
 use std::io::{BufRead, BufReader, Write};
 use std::path::{Path, PathBuf};
 use std::time::{SystemTime, UNIX_EPOCH};
+use std::{borrow::Cow, fmt};
 
 use crate::{
     content::{self, json, yaml, Content},
@@ -93,8 +93,12 @@ impl PendingInlineSnapshot {
                 match key.as_str() {
                     Some("run_id") => run_id = value.as_str().map(|x| x.to_string()),
                     Some("line") => line = value.as_u64().map(|x| x as u32),
-                    Some("old") if !value.is_nil() => old = Some(Snapshot::from_content(value)?),
-                    Some("new") if !value.is_nil() => new = Some(Snapshot::from_content(value)?),
+                    Some("old") if !value.is_nil() => {
+                        old = Some(Snapshot::from_content(value, SnapshotKind::Inline)?)
+                    }
+                    Some("new") if !value.is_nil() => {
+                        new = Some(Snapshot::from_content(value, SnapshotKind::Inline)?)
+                    }
                     _ => {}
                 }
             }
@@ -261,7 +265,8 @@ impl MetaData {
         // snapshot. But we don't know that from here (notably
         // `self.input_file.is_none()` is not a correct approach). Given that
         // `--require-full-match` is experimental and we're working on making
-        // inline & file snapshots more coherent, I'm leaving this as is for now.
+        // inline & file snapshots more coherent, I'm leaving this as is for
+        // now.
         if self.assertion_line.is_some() {
             let mut rv = self.clone();
             rv.assertion_line = None;
@@ -272,6 +277,12 @@ impl MetaData {
     }
 }
 
+#[derive(Debug, Clone, Copy)]
+pub enum SnapshotKind {
+    Inline,
+    File,
+}
+
 /// A helper to work with file snapshots.
 #[derive(Debug, Clone)]
 pub struct Snapshot {
@@ -304,6 +315,8 @@ impl Snapshot {
             let content = yaml::parse_str(&buf, p)?;
             MetaData::from_content(content)?
         // legacy format
+        // (but not viable to move into `match_legacy` given it's more than
+        // just the snapshot value itself...)
         } else {
             let mut rv = MetaData::default();
             loop {
@@ -344,7 +357,10 @@ impl Snapshot {
             module_name,
             Some(snapshot_name),
             metadata,
-            buf.into(),
+            SnapshotContents {
+                contents: buf,
+                kind: SnapshotKind::File,
+            },
         ))
     }
 
@@ -364,7 +380,7 @@ impl Snapshot {
     }
 
     #[cfg(feature = "_cargo_insta_internal")]
-    fn from_content(content: Content) -> Result<Snapshot, Box<dyn Error>> {
+    fn from_content(content: Content, kind: SnapshotKind) -> Result<Snapshot, Box<dyn Error>> {
         if let Content::Map(map) = content {
             let mut module_name = None;
             let mut snapshot_name = None;
@@ -377,12 +393,13 @@ impl Snapshot {
                     Some("snapshot_name") => snapshot_name = value.as_str().map(|x| x.to_string()),
                     Some("metadata") => metadata = Some(MetaData::from_content(value)?),
                     Some("snapshot") => {
-                        snapshot = Some(SnapshotContents(
-                            value
+                        snapshot = Some(SnapshotContents {
+                            contents: value
                                 .as_str()
                                 .ok_or(content::Error::UnexpectedDataType)?
                                 .to_string(),
-                        ))
+                            kind,
+                        });
                     }
                     _ => {}
                 }
@@ -405,7 +422,7 @@ impl Snapshot {
             fields.push(("snapshot_name", Content::from(name)));
         }
         fields.push(("metadata", self.metadata.as_content()));
-        fields.push(("snapshot", Content::from(self.snapshot.0.as_str())));
+        fields.push(("snapshot", Content::from(self.snapshot.to_string())));
 
         Content::Struct("Content", fields)
     }
@@ -435,21 +452,34 @@ impl Snapshot {
         self.contents() == other.contents()
     }
 
-    /// Snapshot contents _and_ metadata match another snapshot's.
+    pub fn kind(&self) -> SnapshotKind {
+        self.snapshot.kind
+    }
+
+    /// Both the exact snapshot contents and the persisted metadata match another snapshot's.
+    // (could rename to `matches_exact` for consistency, after some current
+    // pending merge requests are merged)
     pub fn matches_fully(&self, other: &Snapshot) -> bool {
-        self.snapshot.matches_fully(&other.snapshot)
-            && self.metadata.trim_for_persistence() == other.metadata.trim_for_persistence()
+        let contents_match_exact =
+            self.contents().as_str_exact() == other.contents().as_str_exact();
+        match self.kind() {
+            SnapshotKind::File => {
+                self.metadata.trim_for_persistence() == other.metadata.trim_for_persistence()
+                    && contents_match_exact
+            }
+            SnapshotKind::Inline => contents_match_exact,
+        }
     }
 
-    /// The snapshot contents as a &str
-    pub fn contents_str(&self) -> &str {
-        self.snapshot.as_str()
+    /// The normalized snapshot contents as a String
+    pub fn contents_string(&self) -> String {
+        self.snapshot.normalize()
     }
 
     fn serialize_snapshot(&self, md: &MetaData) -> String {
         let mut buf = yaml::to_string(&md.as_content());
         buf.push_str("---\n");
-        buf.push_str(self.contents_str());
+        buf.push_str(self.contents_string().as_str());
         buf.push('\n');
         buf
     }
@@ -517,29 +547,23 @@ impl Snapshot {
 /// The contents of a Snapshot
 // Could be Cow, but I think limited savings
 #[derive(Debug, Clone)]
-pub struct SnapshotContents(String);
+pub struct SnapshotContents {
+    contents: String,
+    pub kind: SnapshotKind,
+}
 
 impl SnapshotContents {
-    pub fn from_inline(value: &str) -> SnapshotContents {
-        SnapshotContents(get_inline_snapshot_value(value))
-    }
-
-    /// Returns the snapshot contents as a normalized string (for example,
-    /// removing surrounding whitespace)
-    pub fn as_str(&self) -> &str {
-        let out = self.0.trim_start_matches(['\r', '\n']).trim_end();
-        // Old inline snapshots have `---` at the start, so this strips that if
-        // it exists. Soon we can start printing a warning and then eventually
-        // remove it in the next version.
-        match out.strip_prefix("---\n") {
-            Some(s) => s,
-            None => out,
-        }
+    pub fn new(contents: String, kind: SnapshotKind) -> SnapshotContents {
+        // We could store a normalized version of the string as part of `new`;
+        // it would avoid allocating a new `String` when we get the normalized
+        // versions, which we may do a few times. (We want to store the
+        // unnormalized version because it allows us to use `matches_fully`.)
+        SnapshotContents { contents, kind }
     }
 
     /// Returns the snapshot contents as string without any normalization
     pub fn as_str_exact(&self) -> &str {
-        self.0.as_str()
+        self.contents.as_str()
     }
 
     /// Matches another snapshot without any normalization
@@ -549,26 +573,26 @@ impl SnapshotContents {
 
     /// Snapshot matches based on the latest format.
     pub fn matches_latest(&self, other: &SnapshotContents) -> bool {
-        self.as_str() == other.as_str()
+        self.to_string() == other.to_string()
     }
 
     pub fn matches_legacy(&self, other: &SnapshotContents) -> bool {
-        fn as_str_legacy(sc: &SnapshotContents) -> &str {
-            let out = sc.as_str();
+        fn as_str_legacy(sc: &SnapshotContents) -> String {
+            let out = sc.to_string();
             // Legacy inline snapshots have `---` at the start, so this strips that if
             // it exists.
             match out.strip_prefix("---\n") {
-                Some(old_snapshot) => old_snapshot,
+                Some(old_snapshot) => old_snapshot.to_string(),
                 None => out,
             }
         }
         as_str_legacy(self) == as_str_legacy(other)
     }
 
-    /// Create the literal string to write inline, adding `#` to escape the
-    /// value, indentation, delimiters
+    /// Returns the string literal, including `#` delimiters, to insert into a
+    /// Rust source file.
     pub fn to_inline(&self, indentation: usize) -> String {
-        let contents = &self.0;
+        let contents = self.normalize();
         let mut out = String::new();
 
         // We don't technically need to escape on newlines, but it reduces diffs
@@ -612,7 +636,7 @@ impl SnapshotContents {
                     .chain(Some(format!("\n{:width$}", "", width = indentation))),
             );
         } else {
-            out.push_str(contents);
+            out.push_str(contents.as_str());
         }
 
         out.push('"');
@@ -620,34 +644,25 @@ impl SnapshotContents {
 
         out
     }
-}
-
-impl<'a> From<Cow<'a, str>> for SnapshotContents {
-    fn from(value: Cow<'a, str>) -> Self {
-        match value {
-            Cow::Borrowed(s) => SnapshotContents::from(s),
-            Cow::Owned(s) => SnapshotContents::from(s),
-        }
-    }
-}
-
-impl From<&str> for SnapshotContents {
-    fn from(value: &str) -> SnapshotContents {
-        // make sure we have unix newlines consistently
-        SnapshotContents(value.replace("\r\n", "\n"))
-    }
-}
 
-impl From<String> for SnapshotContents {
-    fn from(value: String) -> SnapshotContents {
-        // make sure we have unix newlines consistently
-        SnapshotContents(value.replace("\r\n", "\n"))
+    fn normalize(&self) -> String {
+        let kind_specific_normalization = match self.kind {
+            SnapshotKind::Inline => get_inline_snapshot_value(&self.contents),
+            SnapshotKind::File => self.contents.clone(),
+        };
+        // Then this we do for both kinds
+        let out = kind_specific_normalization
+            .trim_start_matches(['\r', '\n'])
+            .trim_end();
+        out.replace("\r\n", "\n")
     }
 }
 
-impl From<SnapshotContents> for String {
-    fn from(value: SnapshotContents) -> String {
-        value.0
+impl fmt::Display for SnapshotContents {
+    /// Returns the snapshot contents as a normalized string (for example,
+    /// removing surrounding whitespace)
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", self.normalize())
     }
 }
 
@@ -657,7 +672,7 @@ impl PartialEq for SnapshotContents {
         if self.matches_latest(other) {
             true
         } else if self.matches_legacy(other) {
-            elog!("{} {}\n{}",style("Snapshot passes but is a legacy format. Please run `cargo insta test --force-update-snapshots --accept` to update to a newer format.").yellow().bold(),"Snapshot contents:", self.as_str());
+            elog!("{} {}\n{}",style("Snapshot passes but is a legacy format. Please run `cargo insta test --force-update-snapshots --accept` to update to a newer format.").yellow().bold(),"Snapshot contents:", self.to_string());
             true
         } else {
             false
@@ -689,9 +704,7 @@ fn min_indentation(snapshot: &str) -> usize {
 fn normalize_inline_snapshot(snapshot: &str) -> String {
     let indentation = min_indentation(snapshot);
     snapshot
-        .trim_end()
         .lines()
-        .skip_while(|l| l.is_empty())
         .map(|l| l.get(indentation..).unwrap_or(""))
         .collect::<Vec<&str>>()
         .join("\n")
@@ -751,13 +764,15 @@ fn test_names_of_path() {
 ///
 /// This also changes all newlines to \n
 fn get_inline_snapshot_value(frozen_value: &str) -> String {
-    // TODO: could move this into the SnapshotContents `from_inline` method
+    // TODO: could move this into the SnapshotContents struct
     // (the only call site)
 
     if frozen_value.trim_start().starts_with('⋮') {
         elog!("A snapshot uses an old snapshot format; please update it to the new format with `cargo insta test --force-update-snapshots --accept`.\n\nSnapshot is at: {}", frozen_value);
 
         // legacy format - retain so old snapshots still work
+        // TODO: move this into `matches_legacy` after the current merge
+        // requests have settled.
         let mut buf = String::new();
         let mut line_iter = frozen_value.lines();
         let mut indentation = 0;
@@ -801,14 +816,13 @@ fn get_inline_snapshot_value(frozen_value: &str) -> String {
 #[test]
 fn test_snapshot_contents() {
     use similar_asserts::assert_eq;
-    let snapshot_contents = SnapshotContents("testing".to_string());
-    assert_eq!(snapshot_contents.to_inline(0), r#""testing""#);
+    assert_eq!(
+        SnapshotContents::new("testing".to_string(), SnapshotKind::Inline).to_inline(0),
+        r#""testing""#
+    );
 
-    let t = &"
-a
-b"[1..];
     assert_eq!(
-        SnapshotContents(t.to_string()).to_inline(0),
+        SnapshotContents::new("\na\nb".to_string(), SnapshotKind::Inline).to_inline(0),
         r##"r#"
 a
 b
@@ -816,35 +830,23 @@ b
     );
 
     assert_eq!(
-        SnapshotContents(
-            "a
-b"
-            .to_string()
-        )
-        .to_inline(4),
+        SnapshotContents::new("a\nb".to_string(), SnapshotKind::Inline).to_inline(4),
         r##"r#"
     a
     b
     "#"##
     );
 
-    let t = &"
-    a
-    b"[1..];
     assert_eq!(
-        SnapshotContents(t.to_string()).to_inline(0),
+        SnapshotContents::new("\n    a\n    b".to_string(), SnapshotKind::Inline).to_inline(0),
         r##"r#"
-    a
-    b
+a
+b
 "#"##
     );
 
-    let t = &"
-a
-
-b"[1..];
     assert_eq!(
-        SnapshotContents(t.to_string()).to_inline(4),
+        SnapshotContents::new("\na\n\nb".to_string(), SnapshotKind::Inline).to_inline(4),
         r##"r#"
     a
 
@@ -852,28 +854,26 @@ b"[1..];
     "#"##
     );
 
-    let t = &"
-    ab
-"[1..];
     assert_eq!(
-        SnapshotContents(t.to_string()).to_inline(0),
-        r##"r#"
-    ab
-"#"##
+        SnapshotContents::new("\n    ab\n".to_string(), SnapshotKind::Inline).to_inline(0),
+        r##""ab""##
     );
 
-    let t = "ab";
-    assert_eq!(SnapshotContents(t.to_string()).to_inline(0), r#""ab""#);
+    assert_eq!(
+        SnapshotContents::new("ab".to_string(), SnapshotKind::Inline).to_inline(0),
+        r#""ab""#
+    );
 }
 
 #[test]
 fn test_snapshot_contents_hashes() {
-    let t = "a###b";
-    assert_eq!(SnapshotContents(t.to_string()).to_inline(0), r#""a###b""#);
+    assert_eq!(
+        SnapshotContents::new("a###b".to_string(), SnapshotKind::Inline).to_inline(0),
+        r#""a###b""#
+    );
 
-    let t = "a\n\\###b";
     assert_eq!(
-        SnapshotContents(t.to_string()).to_inline(0),
+        SnapshotContents::new("a\n\\###b".to_string(), SnapshotKind::Inline).to_inline(0),
         r#####"r####"
 a
 \###b
@@ -891,10 +891,12 @@ fn test_normalize_inline_snapshot() {
             r#"
    1
    2
-    "#
+   "#
         ),
-        r###"1
-2"###
+        r###"
+1
+2
+"###
     );
 
     assert_eq!(
@@ -903,7 +905,8 @@ fn test_normalize_inline_snapshot() {
             1
     2"#
         ),
-        r###"        1
+        r###"
+        1
 2"###
     );
 
@@ -914,8 +917,10 @@ fn test_normalize_inline_snapshot() {
             2
     "#
         ),
-        r###"1
-2"###
+        r###"
+1
+2
+"###
     );
 
     assert_eq!(
@@ -925,7 +930,8 @@ fn test_normalize_inline_snapshot() {
    2
 "#
         ),
-        r###"1
+        r###"
+1
 2"###
     );
 
@@ -935,7 +941,9 @@ fn test_normalize_inline_snapshot() {
         a
     "#
         ),
-        "a"
+        "
+a
+"
     );
 
     assert_eq!(normalize_inline_snapshot(""), "");
@@ -948,9 +956,11 @@ fn test_normalize_inline_snapshot() {
 c
     "#
         ),
-        r###"    a
+        r###"
+    a
     b
-c"###
+c
+    "###
     );
 
     assert_eq!(
@@ -959,7 +969,9 @@ c"###
 a
     "#
         ),
-        "a"
+        "
+a
+    "
     );
 
     assert_eq!(
@@ -967,7 +979,8 @@ a
             "
     a"
         ),
-        "a"
+        "
+a"
     );
 
     assert_eq!(


Test patch:
diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs
index 3d8c277..f1c6552 100644
--- a/cargo-insta/tests/main.rs
+++ b/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
     +    "#####);
      }
     "######);