Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions sample_files/compare.expected
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ f44f0dffe3617ddafca3d65e94a21613 -
sample_files/devicetree_1.dts sample_files/devicetree_2.dts
64b33c6d65dbc85777ee30cf3893fbdb -

sample_files/displaypanic_a_1.txt sample_files/displaypanic_a_2.txt
34442b044ab320973fc989efda46e9d6 -

sample_files/displaypanic_b_1.nix sample_files/displaypanic_b_2.nix
474dcbe72b3c62d6acc5eb8ed80da52a -

sample_files/elisp_1.el sample_files/elisp_2.el
98571b62f7407e90e9441e4f47ea85ae -

Expand Down
14 changes: 14 additions & 0 deletions sample_files/displaypanic_a_1.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@


exit:
mov x0, #0
mov x8, #93
svc #0








13 changes: 13 additions & 0 deletions sample_files/displaypanic_a_2.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@




exit_success:
mov x0, #0
mov x8, #93
svc #0

exit_error:
mov x0, #1
mov x8, #93
svc #0
40 changes: 40 additions & 0 deletions sample_files/displaypanic_b_1.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
flake.darwinModules.communication = {
homebrew.casks = [
"signal"
"vesktop"
"whatsapp"
"zulip"
];
};

flake.homeModules.communication =
{
osConfig,
lib,
pkgs,
...
}:
let
inherit (lib.lists) optionals;
isLinux = osConfig.nixpkgs.hostPlatform.isLinux;
in
{
packages = [
pkgs.cinny-desktop
pkgs.thunderbird
]
++ optionals isLinux [
pkgs.signal-desktop
(
pkgs.discord.overrideAttrs
(old: {
postFixup = ''
wrapProgram $out/opt/Discord/Discord \
--add-flags "x"
'';
})
)
];
};
}
39 changes: 39 additions & 0 deletions sample_files/displaypanic_b_2.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
flake.darwinModules.discord = {
homebrew.casks = [ "vesktop" ];
};

flake.darwinModules.signal-desktop = {
homebrew.casks = [ "signal" ];
};

flake.darwinModules.whatsapp = {
homebrew.casks = [ "whatsapp" ];
};

flake.darwinModules.zulip = {
homebrew.casks = [ "zulip" ];
};

flake.homeModules.discord =
{
osConfig,
lib,
pkgs,
...
}:
let
inherit (lib.lists) optional;
in
{
packages = optional osConfig.nixpkgs.hostPlatform.isLinux (
pkgs.discord.overrideAttrs
(old: {
postFixup = ''
wrapProgram $out/opt/Discord/Discord \
--add-flags "x"
'';
})
);
};
}
54 changes: 46 additions & 8 deletions src/display/hunks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,11 +577,16 @@ fn matched_novel_lines(
true
};
if should_append {
lines.push((
Some(self_line),
next_opposite(self_line, &opposite_to_lhs, highest_rhs),
));
let opposite = next_opposite(self_line, &opposite_to_lhs, highest_rhs);
lines.push((Some(self_line), opposite));
highest_lhs = Some(self_line);
// Also update highest_rhs if we used an RHS line as opposite,
// to prevent that RHS line from being added again when processed.
if let Some(opp) = opposite {
if highest_rhs.map_or(true, |h| opp > h) {
highest_rhs = Some(opp);
}
}
}
}
Side::Right => {
Expand All @@ -591,11 +596,16 @@ fn matched_novel_lines(
true
};
if should_append {
lines.push((
next_opposite(self_line, &opposite_to_rhs, highest_rhs),
Some(self_line),
));
let opposite = next_opposite(self_line, &opposite_to_rhs, highest_lhs);
lines.push((opposite, Some(self_line)));
highest_rhs = Some(self_line);
// Also update highest_lhs if we used an LHS line as opposite,
// to prevent that LHS line from being added again when processed.
if let Some(opp) = opposite {
if highest_lhs.map_or(true, |h| opp > h) {
highest_lhs = Some(opp);
}
}
}
}
}
Expand Down Expand Up @@ -694,6 +704,7 @@ mod tests {

use super::*;
use crate::{
display::test_util::{novel, unchanged},
hash::DftHashMap,
syntax::{MatchKind, TokenKind},
};
Expand Down Expand Up @@ -868,4 +879,31 @@ mod tests {
]
);
}

#[test]
fn test_matched_novel_lines_opposite_threshold() {
// LHS novels at lines 3, 6; RHS novel at line 2, then at line 10
// RHS line 10 has LHS opposites {4, 8}
// Should pick LHS 8 (first > highest_lhs=6), not LHS 4
let lhs_mps = [novel(3), novel(6), unchanged(4, 10), unchanged(8, 10)];
let rhs_mps = [novel(2), unchanged(10, 4), unchanged(10, 8), novel(10)];

let result = matched_novel_lines(&lhs_mps, &rhs_mps);
let (lhs, _) = result.iter().find(|(_, r)| *r == Some(10.into())).unwrap();
assert_eq!(*lhs, Some(8.into()));
}

#[test]
fn test_matched_novel_lines_no_duplicate_lines() {
// LHS novel at line 1 pairs with RHS line 2 as opposite.
// RHS novel at line 2 should not create another entry.
let lhs_mps = [novel(1), unchanged(1, 2)];
let rhs_mps = [novel(2), unchanged(2, 1)];

let result = matched_novel_lines(&lhs_mps, &rhs_mps);

// RHS line 2 should appear exactly once
let rhs_2_count = result.iter().filter(|(_, r)| *r == Some(2.into())).count();
assert_eq!(rhs_2_count, 1);
}
}
3 changes: 3 additions & 0 deletions src/display/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@ pub(crate) mod inline;
pub(crate) mod json;
pub(crate) mod side_by_side;
pub(crate) mod style;

#[cfg(test)]
pub(crate) mod test_util;
52 changes: 45 additions & 7 deletions src/display/side_by_side.rs
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ pub(crate) fn print(
}

let matched_lines = all_matched_lines_filled(lhs_mps, rhs_mps, &lhs_lines, &rhs_lines);
let mut matched_lines_to_print = &matched_lines[..];
let matched_lines_to_print = &matched_lines[..];

let mut lhs_max_visible_line = 1.into();
let mut rhs_max_visible_line = 1.into();
Expand Down Expand Up @@ -612,12 +612,11 @@ pub(crate) fn print(
display_options.num_context_lines as usize,
);
let aligned_lines = &matched_lines_to_print[start_i..end_i];
// We iterate through hunks in order, so we know the next hunk
// must appear after start_i. This makes
// `matched_lines_indexes_for_hunk` faster on later
// iterations, and this function is hot on large textual
// diffs.
matched_lines_to_print = &matched_lines_to_print[start_i..];
// Note: We previously sliced matched_lines_to_print here as an
// optimization, assuming hunks appear in order. However, hunks
// from matched_novel_lines may not be strictly ordered by their
// position in matched_lines when there are interleaved LHS-only
// and RHS-only novel sections. See issue #770.

let no_lhs_changes = hunk.novel_lhs.is_empty();
let no_rhs_changes = hunk.novel_rhs.is_empty();
Expand Down Expand Up @@ -782,6 +781,10 @@ mod tests {

use super::*;
use crate::{
display::{
hunks::matched_pos_to_hunks,
test_util::{novel, unchanged},
},
options::DEFAULT_TERMINAL_WIDTH,
parse::guess_language::Language,
syntax::{AtomKind, MatchKind, TokenKind},
Expand Down Expand Up @@ -907,4 +910,39 @@ mod tests {
&rhs_mps,
);
}

/// Regression test for issue #770: slicing optimization assumed hunks appear
/// in matched_lines order, but LHS-only hunks may appear before RHS-only hunks
/// even when RHS lines come first. Would panic before fix.
#[test]
fn test_display_hunks_out_of_order_issue_770() {
// LHS novels at 10-12, RHS novels at 3-5. Hunks: [LHS, RHS] but
// matched_lines has RHS first. Slicing after LHS hunk would remove RHS lines.
let lhs_src = (0..15)
.map(|i| format!("L{i}"))
.collect::<Vec<_>>()
.join("\n");
let rhs_src = (0..10)
.map(|i| format!("R{i}"))
.collect::<Vec<_>>()
.join("\n");

let lhs_mps = vec![unchanged(0, 0), novel(10), novel(11), novel(12)];
let rhs_mps = vec![unchanged(0, 0), novel(3), novel(4), novel(5)];

let hunks = matched_pos_to_hunks(&lhs_mps, &rhs_mps);

// Would panic before fix with "Hunk lines should be present in matched lines"
print(
&hunks,
&DisplayOptions::default(),
"t.txt",
None,
&FileFormat::PlainText,
&lhs_src,
&rhs_src,
&lhs_mps,
&rhs_mps,
);
}
}
36 changes: 36 additions & 0 deletions src/display/test_util.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
//! Test helpers for creating [`MatchedPos`] values with minimal boilerplate.

use line_numbers::SingleLineSpan;

use crate::parse::syntax::{MatchKind, MatchedPos, TokenKind};

/// Creates a [`SingleLineSpan`] at the given line with column range 0..1.
pub(crate) fn span(line: u32) -> SingleLineSpan {
SingleLineSpan {
line: line.into(),
start_col: 0,
end_col: 1,
}
}

/// Creates a novel [`MatchedPos`] at the given line.
pub(crate) fn novel(line: u32) -> MatchedPos {
MatchedPos {
kind: MatchKind::Novel {
highlight: TokenKind::Delimiter,
},
pos: span(line),
}
}

/// Creates an unchanged [`MatchedPos`] linking `self_line` to `opposite_line`.
pub(crate) fn unchanged(self_line: u32, opposite_line: u32) -> MatchedPos {
MatchedPos {
kind: MatchKind::UnchangedToken {
highlight: TokenKind::Delimiter,
self_pos: vec![span(self_line)],
opposite_pos: vec![span(opposite_line)],
},
pos: span(self_line),
}
}
Loading