diff --git a/CHANGELOG.md b/CHANGELOG.md index 49b5a701c..ebc68b09a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## 0.3 (unreleased) +Diffs are now displayed with unchanged lines aligned to the other side. + Improved Rust parsing to recognise lifetime syntax `'foo`, character literals `'x'` and sequences of punctuation. diff --git a/img/difftastic.png b/img/difftastic.png index 4cbd884ce..dc03315e2 100644 Binary files a/img/difftastic.png and b/img/difftastic.png differ diff --git a/src/lines.rs b/src/lines.rs index c01f77091..86e1dab11 100644 --- a/src/lines.rs +++ b/src/lines.rs @@ -1,7 +1,8 @@ use crate::positions::SingleLineSpan; -use crate::syntax::{MatchKind, MatchedPos}; +use crate::syntax::{aligned_lines, MatchKind, MatchedPos}; use regex::Regex; use std::cmp::{max, min, Ordering}; +use std::collections::HashMap; use std::fmt; use std::ops::RangeInclusive; @@ -43,22 +44,30 @@ impl LineGroup { } } - // We can't iterate over a RangeInclusive in safe, stable Rust. See - // https://github.com/rust-lang/rust/issues/42168 - pub fn iter_lhs_lines(&self) -> RangeInclusive { - let empty_iter = 1usize..=0; + fn lhs_lines(&self) -> Vec { + let mut res = vec![]; match &self.lhs_lines { - Some(lhs_lines) => lhs_lines.start().number..=lhs_lines.end().number, - None => empty_iter, + Some(lhs_lines) => { + for line in lhs_lines.start().number..=lhs_lines.end().number { + res.push(line.into()); + } + } + None => {} } + res } - pub fn iter_rhs_lines(&self) -> RangeInclusive { - let empty_iter = 1usize..=0; + fn rhs_lines(&self) -> Vec { + let mut res = vec![]; match &self.rhs_lines { - Some(rhs_lines) => rhs_lines.start().number..=rhs_lines.end().number, - None => empty_iter, + Some(rhs_lines) => { + for line in rhs_lines.start().number..=rhs_lines.end().number { + res.push(line.into()); + } + } + None => {} } + res } pub fn pad(&mut self, amount: usize, max_lhs_line: LineNumber, max_rhs_line: LineNumber) { @@ -393,25 +402,45 @@ fn apply_group( lhs_lines: &[&str], rhs_lines: &[&str], group: &LineGroup, + lhs_line_matches: &HashMap, lhs_content_width: usize, lhs_column_width: usize, rhs_column_width: usize, ) -> String { let mut lhs_result = String::new(); - for lhs_line_num in group.iter_lhs_lines() { - lhs_result.push_str(&format_line_num_padded(lhs_line_num, lhs_column_width)); + let mut rhs_result = String::new(); - match lhs_lines.get(lhs_line_num) { - Some(line) => lhs_result.push_str(line), - None => lhs_result.push_str(&" ".repeat(lhs_content_width)), + for (lhs_line_num, rhs_line_num) in + aligned_lines(&group.lhs_lines(), &group.rhs_lines(), lhs_line_matches) + { + // TODO: we could build up a single string rather than + // horizontally concatenating afterwards. + + match lhs_line_num { + Some(lhs_line_num) => { + lhs_result.push_str(&format_line_num_padded( + lhs_line_num.number, + lhs_column_width, + )); + lhs_result.push_str(lhs_lines[lhs_line_num.number]); + } + None => { + lhs_result.push_str(&" ".repeat(lhs_column_width)); + lhs_result.push_str(&" ".repeat(lhs_content_width)); + } } lhs_result.push('\n'); - } - let mut rhs_result = String::new(); - for rhs_line_num in group.iter_rhs_lines() { - rhs_result.push_str(&format_line_num_padded(rhs_line_num, rhs_column_width)); - rhs_result.push_str(rhs_lines.get(rhs_line_num).unwrap_or(&"")); + match rhs_line_num { + Some(rhs_line_num) => { + rhs_result.push_str(&format_line_num_padded( + rhs_line_num.number, + rhs_column_width, + )); + rhs_result.push_str(rhs_lines[rhs_line_num.number]); + } + None => {} + } rhs_result.push('\n'); } @@ -429,6 +458,7 @@ pub fn apply_groups( lhs: &str, rhs: &str, groups: &[LineGroup], + lhs_line_matches: &HashMap, lhs_content_width: usize, lhs_column_width: usize, rhs_column_width: usize, @@ -445,6 +475,7 @@ pub fn apply_groups( &lhs_lines, &rhs_lines, group, + lhs_line_matches, lhs_content_width, lhs_column_width, rhs_column_width, diff --git a/src/main.rs b/src/main.rs index e898aad6d..8d0d1d5fa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -16,7 +16,7 @@ use crate::lines::{ }; use crate::parse::{find_lang, parse, parse_lines, read_or_die, ConfigDir}; use crate::style::apply_colors; -use crate::syntax::{change_positions, set_next}; +use crate::syntax::{change_positions, matching_lines, set_next}; fn term_width() -> Option { term_size::dimensions().map(|(w, _)| w) @@ -91,6 +91,8 @@ fn main() { let lhs_positions = change_positions(&lhs_src, &rhs_src, &lhs); let rhs_positions = change_positions(&rhs_src, &lhs_src, &rhs); + let lhs_matched_lines = matching_lines(&lhs); + let mut groups = visible_groups(&lhs_positions, &rhs_positions); for group in &mut groups { group.pad(3, lhs_src.max_line(), rhs_src.max_line()); @@ -127,6 +129,7 @@ fn main() { &lhs_colored, &rhs_colored, &groups, + &lhs_matched_lines, lhs_content_width, lhs_column_width, rhs_column_width, diff --git a/src/syntax.rs b/src/syntax.rs index cddc2f470..8d897869a 100644 --- a/src/syntax.rs +++ b/src/syntax.rs @@ -3,8 +3,9 @@ use itertools::{EitherOrBoth, Itertools}; use std::cell::Cell; +use std::cmp::min; use std::collections::hash_map::DefaultHasher; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fmt; use std::hash::{Hash, Hasher}; use typed_arena::Arena; @@ -520,6 +521,22 @@ fn change_positions_<'a>( } } +fn zip_pad_shorter(lhs: &[Tx], rhs: &[Ty]) -> Vec<(Option, Option)> { + let mut res = vec![]; + + let mut i = 0; + loop { + match (lhs.get(i), rhs.get(i)) { + (None, None) => break, + (x, y) => res.push((x.map(|v| *v), y.map(|v| *v))), + } + + i += 1; + } + + res +} + /// Given two slices of line positions, return a list of line number /// pairs. If the slices have different lengths, reuse the last item /// from the shorter slice. @@ -555,59 +572,53 @@ pub fn aligned_lines( lhs_lines: &[LineNumber], rhs_lines: &[LineNumber], lhs_line_matches: &HashMap, - rhs_line_matches: &HashMap, ) -> Vec<(Option, Option)> { - // Find RHS lines that we can match up. - let mut lhs_opposite_lines = vec![]; - for lhs_line in lhs_lines { - if let Some(lhs_opposite_line) = lhs_line_matches.get(lhs_line) { - lhs_opposite_lines.push(lhs_opposite_line); - } - } + let mut rhs_lines_available: HashSet<_> = rhs_lines.iter().collect(); - // Find LHS lines that we can match up. - let mut rhs_opposite_lines = vec![]; - for rhs_line in rhs_lines { - if let Some(rhs_opposite_line) = rhs_line_matches.get(rhs_line) { - rhs_opposite_lines.push(rhs_opposite_line); + // For every LHS line, if there is a RHS line that included in + // `rhs_lines` and hasn't yet been paired up, add it to matched_lines. + let mut matched_lines = vec![]; + for lhs_line in lhs_lines { + if let Some(rhs_line) = lhs_line_matches.get(lhs_line) { + if rhs_lines_available.remove(&rhs_line) { + matched_lines.push((lhs_line, rhs_line)); + } } } - // Sanity check: if LHS X matches RHS Y, then RHS Y should match - // LHS X and we should have the same number of opposite lines. - assert_eq!(lhs_opposite_lines.len(), rhs_opposite_lines.len()); - let mut res = vec![]; let mut lhs_i = 0; let mut rhs_i = 0; - // Build a vec of matched lines, padding the unmatched sequences with None. - for (lhs_matched_line, rhs_matched_line) in rhs_opposite_lines - .into_iter() - .zip(lhs_opposite_lines.into_iter()) - { + // Build a vec of matched line tuples. For lines without matches + // (novel lines, empty lines), just match lines up pairwise. Pad + // gaps if one side has more lines. + for (lhs_matched_line, rhs_matched_line) in matched_lines { + let mut lhs_prev_lines = vec![]; while lhs_i < lhs_lines.len() && lhs_lines[lhs_i] < *lhs_matched_line { - res.push((Some(lhs_lines[lhs_i]), None)); + lhs_prev_lines.push(lhs_lines[lhs_i]); lhs_i += 1; } + let mut rhs_prev_lines = vec![]; while rhs_i < rhs_lines.len() && rhs_lines[rhs_i] < *rhs_matched_line { - res.push((None, Some(rhs_lines[rhs_i]))); + rhs_prev_lines.push(rhs_lines[rhs_i]); rhs_i += 1; } - res.push((Some(*lhs_matched_line), Some(*rhs_matched_line))); - } - // If we have trailing unmatched lines on other side, add them now. - while lhs_i < lhs_lines.len() { - res.push((Some(lhs_lines[lhs_i]), None)); + res.extend(zip_pad_shorter(&lhs_prev_lines, &rhs_prev_lines)); + + res.push((Some(*lhs_matched_line), Some(*rhs_matched_line))); lhs_i += 1; - } - while rhs_i < rhs_lines.len() { - res.push((None, Some(rhs_lines[rhs_i]))); rhs_i += 1; } + // Handle unmatched lines after the last match. + res.extend(zip_pad_shorter( + &lhs_lines[min(lhs_i, lhs_lines.len())..], + &rhs_lines[min(rhs_i, rhs_lines.len())..], + )); + res } @@ -675,6 +686,70 @@ mod tests { use pretty_assertions::assert_eq; use AtomKind::Other; + #[test] + fn test_aligned_middle() { + let lhs_lines: Vec = vec![1.into(), 2.into()]; + let rhs_lines: Vec = vec![12.into(), 13.into()]; + + let mut line_matches: HashMap = HashMap::new(); + line_matches.insert(2.into(), 12.into()); + + assert_eq!( + aligned_lines(&lhs_lines, &rhs_lines, &line_matches), + vec![ + (Some(1.into()), None), + (Some(2.into()), Some(12.into())), + (None, Some(13.into())) + ] + ); + } + + #[test] + fn test_aligned_all() { + let lhs_lines: Vec = vec![1.into(), 2.into()]; + let rhs_lines: Vec = vec![11.into(), 12.into()]; + + let mut line_matches: HashMap = HashMap::new(); + line_matches.insert(1.into(), 2.into()); + line_matches.insert(2.into(), 12.into()); + + assert_eq!( + aligned_lines(&lhs_lines, &rhs_lines, &line_matches), + vec![ + (Some(1.into()), Some(11.into())), + (Some(2.into()), Some(12.into())), + ] + ); + } + + #[test] + fn test_aligned_none() { + let lhs_lines: Vec = vec![1.into()]; + let rhs_lines: Vec = vec![11.into()]; + + let line_matches: HashMap = HashMap::new(); + + assert_eq!( + aligned_lines(&lhs_lines, &rhs_lines, &line_matches), + vec![(Some(1.into()), Some(11.into()))] + ); + } + + #[test] + fn test_aligned_line_overlap() { + let lhs_lines: Vec = vec![1.into(), 2.into()]; + let rhs_lines: Vec = vec![11.into()]; + + let mut line_matches: HashMap = HashMap::new(); + line_matches.insert(1.into(), 11.into()); + line_matches.insert(2.into(), 11.into()); + + assert_eq!( + aligned_lines(&lhs_lines, &rhs_lines, &line_matches), + vec![(Some(1.into()), Some(11.into())), (Some(2.into()), None)] + ); + } + /// Ensure that we assign prev_opposite_pos even if the change is on the first node. #[test] fn test_prev_opposite_pos_first_node() {