Skip to content

Commit

Permalink
Always consider newlines when diffing lines
Browse files Browse the repository at this point in the history
StringIgnoringNewline caused crashes in code that assumed that two
equal values would hav the same string length.

This reverts 8661279. A better
approach would be to normalise line endings before diffing, but
additionally print whether files have/lack trailing newlines.

Fixes #755
  • Loading branch information
Wilfred committed Oct 15, 2024
1 parent 2dd4f7e commit 7edd2a8
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 50 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
`~/.gitconfig` to match the manual, regardless of your difftastic
version.**

### Diffing

Fixed a crash (introduced in 0.60) when the final line in a file does
not have a trailing newline and occurs more than once in the file.

## 0.60 (released 1st August 2024)

### Diffing
Expand Down
7 changes: 5 additions & 2 deletions sample_files/compare.expected
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ sample_files/ada_1.adb sample_files/ada_2.adb
eb3b0e12e239ae33789136380e81406b -

sample_files/added_line_1.txt sample_files/added_line_2.txt
8a1587e6b5fc53f2ec2ac665a5d00372 -
f935214c79833318d39364145d36d8e2 -

sample_files/align_footer_1.txt sample_files/align_footer_2.txt
d640bd2de31e56a39f0efb92aff0f379 -
Expand All @@ -23,7 +23,7 @@ sample_files/bad_combine_1.rs sample_files/bad_combine_2.rs
f5051bf7d2b8afa3a677388cbd458891 -

sample_files/big_text_hunk_1.txt sample_files/big_text_hunk_2.txt
fc26d41a5ff771670e04033b177973d2 -
dcc22684c2a200afdd583d621b47dfa3 -

sample_files/change_outer_1.el sample_files/change_outer_2.el
2b9334a4cc72da63bba28eff958f0038 -
Expand Down Expand Up @@ -214,6 +214,9 @@ sample_files/r_1.R sample_files/r_2.R
sample_files/racket_1.rkt sample_files/racket_2.rkt
605aec93fa7b89d08e5d8ed56ad3c1be -

sample_files/repeated_line_no_eol_1.txt sample_files/repeated_line_no_eol_2.txt
ac714893a2d28dc0204855d308972ccd -

sample_files/ruby_1.rb sample_files/ruby_2.rb
d4d591902030355656f5c18c78f965a6 -

Expand Down
1 change: 1 addition & 0 deletions sample_files/repeated_line_no_eol_1.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
abc
2 changes: 2 additions & 0 deletions sample_files/repeated_line_no_eol_2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
abc
abc
53 changes: 5 additions & 48 deletions src/line_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
use lazy_static::lazy_static;
use line_numbers::{LinePositions, SingleLineSpan};
use regex::Regex;
use std::hash::Hash;

use crate::words::split_words;
use crate::{
Expand Down Expand Up @@ -74,66 +73,24 @@ fn merge_novel<'a>(
res
}

#[derive(Debug, Clone)]
struct StringIgnoringNewline<'a>(&'a str);

impl PartialEq for StringIgnoringNewline<'_> {
fn eq(&self, other: &Self) -> bool {
let mut s = self.0;
if s.ends_with('\n') {
s = &s[..s.len() - 1];
}

let mut other_s = other.0;
if other_s.ends_with('\n') {
other_s = &other_s[..other_s.len() - 1];
}

s == other_s
}
}

impl Eq for StringIgnoringNewline<'_> {}

impl Hash for StringIgnoringNewline<'_> {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
let mut s = self.0;
if s.ends_with('\n') {
s = &s[..s.len() - 1];
}

s.hash(state);
}
}

fn changed_parts<'a>(
src: &'a str,
opposite_src: &'a str,
) -> Vec<(TextChangeKind, Vec<&'a str>, Vec<&'a str>)> {
let src_lines = split_lines_keep_newline(src)
.into_iter()
.map(StringIgnoringNewline)
.collect::<Vec<_>>();
let opposite_src_lines = split_lines_keep_newline(opposite_src)
.into_iter()
.map(StringIgnoringNewline)
.collect::<Vec<_>>();
let src_lines = split_lines_keep_newline(src);
let opposite_src_lines = split_lines_keep_newline(opposite_src);

let mut res: Vec<(TextChangeKind, Vec<&'a str>, Vec<&'a str>)> = vec![];
for diff_res in myers_diff::slice_unique_by_hash(&src_lines, &opposite_src_lines) {
match diff_res {
myers_diff::DiffResult::Left(line) => {
res.push((TextChangeKind::Novel, vec![line.0], vec![]));
res.push((TextChangeKind::Novel, vec![line], vec![]));
}
myers_diff::DiffResult::Both(line, opposite_line) => {
res.push((
TextChangeKind::Unchanged,
vec![line.0],
vec![opposite_line.0],
));
res.push((TextChangeKind::Unchanged, vec![line], vec![opposite_line]));
}
myers_diff::DiffResult::Right(opposite_line) => {
res.push((TextChangeKind::Novel, vec![], vec![opposite_line.0]));
res.push((TextChangeKind::Novel, vec![], vec![opposite_line]));
}
}
}
Expand Down

0 comments on commit 7edd2a8

Please sign in to comment.