Improve diff with "\r\n" and "\n"

[Mathieu-Lala]

The output produced by a difference with new lines is not very clear.

To reproduce :

#[test]
fn f() {
    pretty_assertions::assert_eq!("\n", "\r\n");
}

thread 'test::f' panicked at 'assertion failed: `(left == right)`

Diff < left / right > :
 

I don't know if that has not been thought or if I am doing something wrong.

[tommilligan]

Thanks for the report. This looks like an issue in the underlying diff, and transitively, std crates, whereby 'lines' are created by splitting on \n characters, and then any trailing \r characters are stripped:

• diff::lines
• std::str::lines

I think a resolution to this issue would take one of two approaches:

• Don't actually fix the underlying issue - detect when pretty_assertions can't print a good diff, and show a warning to the user.
  • For example: "Warning: values were different, but no line diff was generated. Do your values have the same line endings?"
• Handle differing line endings correctly.
  • Detect the line ending type used by one value.
    • optional: run it on both values, and print a user warning if we think the values have different line endings
  • Use that to split both values explicitly, don't strip trailing \rs print a diff as normal

I know that line ending detection is possibly Hard, and would probably require a linear pass over the diffed values. Might not be a big a deal as I initially think though.

One benefit of the first approach is that it would be a catch-all to warn about other edge cases such as this, which don't produce good output. We could even make it an error-report style message, like "please open an issue at github.com/... with the following details", but that might actually be annoying to users if seen repetitively during testing.

[tommilligan]

For anyone wanting to pick up the first approach (print a warning on no diff), the following diff may be useful:

• At the point we iterate through changes, accumulate some stats about number of Both, Left and Right changes. Return this from the printing function.
• The caller of the printing function can then decide what to do if, e.g. there are only Both changes (no diff).

diff --git a/pretty_assertions/src/printer.rs b/pretty_assertions/src/printer.rs
index af3e3e6..6edba1e 100644
--- a/pretty_assertions/src/printer.rs
+++ b/pretty_assertions/src/printer.rs
@@ -89,7 +89,7 @@ pub(crate) fn write_lines<TWrite: fmt::Write>(
     let mut previous_deletion = LatentDeletion::default();

     while let Some(change) = changes.next() {
-        match (change, changes.peek()) {
+        match (dbg!(change), changes.peek()) {
             // If the text is unchanged, just print it plain
             (::diff::Result::Both(value, _), _) => {
                 previous_deletion.flush(f)?;
@@ -594,5 +594,23 @@ Cabbage"#;

             check_printer(write_lines, left, right, &expected);
         }
+
+        /// Test diffing newlines from different conventions.
+        ///
+        /// See: https://github.com/colin-kiegel/rust-pretty-assertions/issues/100
+        #[test]
+        fn different_newlines() {
+            // The below inputs caused an output with no visible diff
+            let left = "\r\n";
+            let right = "\n";
+            let expected = format!(
+                "
+
+Warning: No diff generated for different values.
+",
+            );
+
+            check_printer(write_lines, left, right, &expected);
+        }
     }
 }

[lemolatoon]

I'm gonna try the first approach.