Fix crash when the last hunk includes the trailing newline at EOF

Input cleaning should happen before we diff the content, not during
display. Previously display would crash due to line numbers
referencing the line that had been stripped.

Fixes #713
Fixes #739
Fixes #742

CHANGELOG.md

@@ -1,5 +1,10 @@
 ## 0.60 (unreleased)
 
+### Diffing
+
+Fixed a crash (introduced in 0.59) when the final changed hunk
+included the last line of the file.
+
 ### Display
 
 Fixed an issue where files with no common content would show duplicate

sample_files/cli_tests/changes_at_end_1.txt

@@ -0,0 +1,21 @@
+              TOKEN_PATH@129..133 "/d6/"
+          TOKEN_WHITESPACE@133..135 "\u{c}\u{c}"
+          NODE_IDENTIFIER@135..137
+            TOKEN_IDENTIFIER@135..137 "o2"
+        NODE_PATH@137..140
+          TOKEN_PATH@137..140 "/Aa"
+    TOKEN_WHITESPACE@140..142 "\u{c}\u{c}"
+    TOKEN_DOLLAR@142..143 "$"
+    NODE_INFIX_OPERATION@143..155
+      NODE_IDENTIFIER@143..144
+        TOKEN_IDENTIFIER@143..144 "d"
+      TOKEN_PLUS@144..145 "+"
+      NODE_APPLICATION@145..155
+        NODE_APPLICATION@145..152
+          NODE_PATH@145..148
+            TOKEN_PATH@145..148 "/6/"
+          TOKEN_WHITESPACE@148..150 "\u{c}\u{c}"
+          NODE_IDENTIFIER@150..152
+            TOKEN_IDENTIFIER@150..152 "o2"
+        NODE_PATH@152..155
+          TOKEN_PATH@152..155 "/Aa"

sample_files/cli_tests/changes_at_end_2.txt

@@ -0,0 +1,30 @@
+                    TOKEN_INTEGER@125..127 "77"
+                  TOKEN_DOLLAR@127..128 "$"
+                  NODE_INFIX_OPERATION@128..155
+                    NODE_APPLICATION@128..140
+                      NODE_APPLICATION@128..137
+                        NODE_APPLICATION@128..133
+                          NODE_NUMBER@128..129
+                            TOKEN_INTEGER@128..129 "7"
+                          NODE_PATH@129..133
+                            TOKEN_PATH@129..133 "/d6/"
+                        TOKEN_WHITESPACE@133..135 "\u{c}\u{c}"
+                        NODE_IDENTIFIER@135..137
+                          TOKEN_IDENTIFIER@135..137 "o2"
+                      NODE_PATH@137..140
+                        TOKEN_PATH@137..140 "/Aa"
+                    TOKEN_WHITESPACE@140..142 "\u{c}\u{c}"
+                    TOKEN_DOLLAR@142..143 "$"
+                    NODE_INFIX_OPERATION@143..155
+                      NODE_IDENTIFIER@143..144
+                        TOKEN_IDENTIFIER@143..144 "d"
+                      TOKEN_PLUS@144..145 "+"
+                      NODE_APPLICATION@145..155
+                        NODE_APPLICATION@145..152
+                          NODE_PATH@145..148
+                            TOKEN_PATH@145..148 "/6/"
+                          TOKEN_WHITESPACE@148..150 "\u{c}\u{c}"
+                          NODE_IDENTIFIER@150..152
+                            TOKEN_IDENTIFIER@150..152 "o2"
+                        NODE_PATH@152..155
+                          TOKEN_PATH@152..155 "/Aa"

src/display/side_by_side.rs

@@ -407,20 +407,8 @@ pub(crate) fn print(
     let mut prev_lhs_line_num = None;
     let mut prev_rhs_line_num = None;
 
-    let mut lhs_lines = split_on_newlines(lhs_src).collect::<Vec<_>>();
-    let mut rhs_lines = split_on_newlines(rhs_src).collect::<Vec<_>>();
-
-    // If "foo" is one line, is "foo\n" two lines? Generally we want
-    // to care about newlines when deciding whether content differs.
-    //
-    // Ending a file with a trailing newline is extremely common
-    // though. If both files have a trailing newline, consider "foo\n"
-    // to be "foo" so we don't end up displaying a blank line on both
-    // sides.
-    if lhs_lines.last() == Some(&"") && rhs_lines.last() == Some(&"") {
-        lhs_lines.pop();
-        rhs_lines.pop();
-    }
+    let lhs_lines = split_on_newlines(lhs_src).collect::<Vec<_>>();
+    let rhs_lines = split_on_newlines(rhs_src).collect::<Vec<_>>();
 
     let matched_lines = all_matched_lines_filled(lhs_mps, rhs_mps, &lhs_lines, &rhs_lines);
     let mut matched_lines_to_print = &matched_lines[..];

src/main.rs

@@ -373,6 +373,18 @@ fn diff_file(
         rhs_src.retain(|c| c != '\r');
     }
 
+    // If "foo" is one line, is "foo\n" two lines? Generally we want
+    // to care about newlines when deciding whether content differs.
+    //
+    // Ending a file with a trailing newline is extremely common
+    // though. If both files have a trailing newline, consider "foo\n"
+    // to be "foo" so we don't end up displaying a blank line on both
+    // sides.
+    if lhs_src.ends_with('\n') && rhs_src.ends_with('\n') {
+        lhs_src.pop();
+        rhs_src.pop();
+    }
+
     let mut extra_info = renamed;
     if let (Some(lhs_perms), Some(rhs_perms)) = (lhs_permissions, rhs_permissions) {
         if lhs_perms != rhs_perms {

tests/cli.rs

@@ -116,6 +116,16 @@ fn check_only_text_file() {
     cmd.assert().stdout(predicate_fn);
 }
 
+#[test]
+fn text_changes_at_end_doesnt_crash() {
+    let mut cmd = get_base_command();
+
+    cmd.arg("sample_files/cli_tests/changes_at_end_1.txt")
+        .arg("sample_files/cli_tests/changes_at_end_2.txt");
+
+    cmd.assert().success();
+}
+
 #[test]
 fn makefile_text_as_atom() {
     let mut cmd = get_base_command();