Repository: mitsuhiko/insta
Pull request #603 (2024-09-21 19:38:15)

Only add `#` characters when required
My previous implementation added them too liberally



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index cd5b152..910f005 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,8 @@ All notable changes to insta and cargo-insta are documented here.
   snapshot delimiters are assessed for changes, not the delimiters (e.g. `###`).
   #581
 
+- Inline snapshots only use `#` characters as delimiters when required.  #603
+
 ## 1.40.0
 
 - `cargo-insta` no longer panics when running `cargo insta test --accept --workspace`
diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs
index 93569f8..7d487ea 100644
--- a/insta/src/snapshot.rs
+++ b/insta/src/snapshot.rs
@@ -570,16 +570,10 @@ impl SnapshotContents {
 
         // We don't technically need to escape on newlines, but it reduces diffs
         let is_escape = contents.contains(['\\', '"', '\n']);
-        // Escape the string if needed, with `r#`, using with 1 more `#` than
-        // the maximum number of existing contiguous `#`.
+        // Escape the string if needed, with `r#`, using the required number of `#`s
         let delimiter = if is_escape {
-            let max_contiguous_hash = contents
-                .split(|c| c != '#')
-                .map(|group| group.len())
-                .max()
-                .unwrap_or(0);
             out.push('r');
-            "#".repeat(max_contiguous_hash + 1)
+            "#".repeat(required_hashes(&contents))
         } else {
             "".to_string()
         };
@@ -653,6 +647,34 @@ impl PartialEq for SnapshotContents {
     }
 }
 
+/// The number of `#` we need to surround a raw string literal with.
+fn required_hashes(text: &str) -> usize {
+    let splits = text.split('"');
+    if splits.clone().count() <= 1 {
+        return 0;
+    }
+
+    splits
+        .map(|s| s.chars().take_while(|&c| c == '#').count() + 1)
+        .max()
+        .unwrap()
+}
+
+#[test]
+fn test_required_hashes() {
+    assert_snapshot!(required_hashes(""), @"0");
+    assert_snapshot!(required_hashes("Hello, world!"), @"0");
+    assert_snapshot!(required_hashes("\"\""), @"1");
+    assert_snapshot!(required_hashes("##"), @"0");
+    assert_snapshot!(required_hashes("\"#\"#"), @"2");
+    assert_snapshot!(required_hashes(r##""#"##), @"2");
+    assert_snapshot!(required_hashes(r######"foo ""##### bar "###" baz"######), @"6");
+    assert_snapshot!(required_hashes("\"\"\""), @"1");
+    assert_snapshot!(required_hashes("####"), @"0");
+    assert_snapshot!(required_hashes(r###"\"\"##\"\""###), @"3");
+    assert_snapshot!(required_hashes(r###"r"#"Raw string"#""###), @"2");
+}
+
 fn count_leading_spaces(value: &str) -> usize {
     value.chars().take_while(|x| x.is_whitespace()).count()
 }
@@ -782,35 +804,35 @@ fn test_snapshot_contents() {
 
     assert_eq!(
         SnapshotContents::new("\na\nb".to_string(), SnapshotKind::Inline).to_inline(0),
-        r##"r#"
+        r##"r"
 a
 b
-"#"##
+""##
     );
 
     assert_eq!(
         SnapshotContents::new("a\nb".to_string(), SnapshotKind::Inline).to_inline(4),
-        r##"r#"
+        r##"r"
     a
     b
-    "#"##
+    ""##
     );
 
     assert_eq!(
         SnapshotContents::new("\n    a\n    b".to_string(), SnapshotKind::Inline).to_inline(0),
-        r##"r#"
+        r##"r"
 a
 b
-"#"##
+""##
     );
 
     assert_eq!(
         SnapshotContents::new("\na\n\nb".to_string(), SnapshotKind::Inline).to_inline(4),
-        r##"r#"
+        r##"r"
     a
 
     b
-    "#"##
+    ""##
     );
 
     assert_eq!(
@@ -833,10 +855,10 @@ fn test_snapshot_contents_hashes() {
 
     assert_eq!(
         SnapshotContents::new("a\n\\###b".to_string(), SnapshotKind::Inline).to_inline(0),
-        r#####"r####"
+        r#####"r"
 a
 \###b
-"####"#####
+""#####
     );
 }
 


Test patch:
diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs
index f1c6552..db0ca74 100644
--- a/cargo-insta/tests/main.rs
+++ b/cargo-insta/tests/main.rs
@@ -1077,15 +1077,18 @@ fn test_wrong_indent_force() {
     assert_snapshot!(test_project.diff("src/lib.rs"), @r##"
     --- Original: src/lib.rs
     +++ Updated: src/lib.rs
-    @@ -5,7 +5,7 @@
+    @@ -4,8 +4,8 @@
+         insta::assert_snapshot!(r#"
          foo
          foo
-         "#, @r#"
+    -    "#, @r#"
     -                foo
     -                foo
+    -    "#);
+    +    "#, @r"
     +    foo
     +    foo
-         "#);
+    +    ");
      }
     "##);
 }
@@ -1108,13 +1111,14 @@ insta = { path = '$PROJECT_PATH' }
         )
         .add_file(
             "src/main.rs",
-            r#"
+            r#####"
 #[test]
 fn test_hashtag_escape() {
-    insta::assert_snapshot!("Value with\n#### hashtags\n", @"");
+    insta::assert_snapshot!(r###"Value with
+    "## hashtags\n"###, @"");
 }
-"#
-            .to_string(),
+"#####
+                .to_string(),
         )
         .create_project();
 
@@ -1126,18 +1130,18 @@ fn test_hashtag_escape() {
 
     assert_success(&output);
 
-    assert_snapshot!(test_project.diff("src/main.rs"), @r######"
+    assert_snapshot!(test_project.diff("src/main.rs"), @r####"
     --- Original: src/main.rs
     +++ Updated: src/main.rs
-    @@ -1,5 +1,8 @@
-     
+    @@ -2,5 +2,8 @@
      #[test]
      fn test_hashtag_escape() {
-    -    insta::assert_snapshot!("Value with\n#### hashtags\n", @"");
-    +    insta::assert_snapshot!("Value with\n#### hashtags\n", @r#####"
+         insta::assert_snapshot!(r###"Value with
+    -    "## hashtags\n"###, @"");
+    +    "## hashtags\n"###, @r###"
     +    Value with
-    +    #### hashtags
-    +    "#####);
+    +        "## hashtags\n
+    +    "###);
      }
-    "######);
+    "####);
 }