fix: avoid repeated scope labels within diff excerpts
Compare each side with its preceding source row within the same excerpt, not the adjacent aligned pair: alignment holes must not restart labels. Retain sticky context independently on each side, including leading holes. Regression: merged Python insert/delete hunks emitted five labels instead of one. Verify deduplication, folded excerpt restart, and sticky-gap context. Verified: cargo test (950 passed), cargo build, and real PTY smoke covering split/unified, wrap, sticky gaps, and a short horizontal pane source caret.
This commit is contained in:
+85
-11
@@ -4544,8 +4544,9 @@ impl DiffWrapViewport {
|
||||
return;
|
||||
}
|
||||
let mut position = self.top;
|
||||
let mut sticky = [true; 2];
|
||||
while self.rows.len() < height {
|
||||
for scope in syntax.labels(diff, position, self.rows.is_empty()) {
|
||||
for scope in syntax.labels(diff, position, sticky) {
|
||||
if scope.iter().any(|side| *side) && self.rows.len() + 1 < height {
|
||||
self.rows.push(WrappedDiffRow {
|
||||
position,
|
||||
@@ -4556,6 +4557,11 @@ impl DiffWrapViewport {
|
||||
});
|
||||
}
|
||||
}
|
||||
for (side, focus) in [Focus::Before, Focus::After].into_iter().enumerate() {
|
||||
if diff.display_source_row(focus, position.display).is_some() {
|
||||
sticky[side] = false;
|
||||
}
|
||||
}
|
||||
let row = self.row(diff, file, position);
|
||||
self.rows.push(row);
|
||||
let next = self.step(diff, file, position, true);
|
||||
@@ -4669,7 +4675,7 @@ impl DiffWrapViewport {
|
||||
let target = row.min(height.saturating_sub(1));
|
||||
let label_count = |position, sticky| {
|
||||
syntax
|
||||
.labels(diff, position, sticky)
|
||||
.labels(diff, position, [sticky; 2])
|
||||
.iter()
|
||||
.filter(|scope| scope.iter().any(|side| *side))
|
||||
.count()
|
||||
@@ -4751,20 +4757,37 @@ impl DiffSyntaxCache {
|
||||
})
|
||||
}
|
||||
|
||||
fn labels(&self, diff: &DiffState, position: DiffWrapPosition, sticky: bool) -> [[bool; 2]; 2] {
|
||||
fn labels(
|
||||
&self,
|
||||
diff: &DiffState,
|
||||
position: DiffWrapPosition,
|
||||
sticky: [bool; 2],
|
||||
) -> [[bool; 2]; 2] {
|
||||
let scopes = self.scopes(diff, position.display);
|
||||
if !sticky
|
||||
&& (position.continuation > 0
|
||||
|| position.display > 0 && scopes == self.scopes(diff, position.display - 1))
|
||||
{
|
||||
return [[false; 2]; 2];
|
||||
}
|
||||
let show = [Focus::Before, Focus::After].map(|focus| {
|
||||
let side = usize::from(focus == Focus::After);
|
||||
if scopes[side].is_none() || (!sticky[side] && position.continuation > 0) {
|
||||
return false;
|
||||
}
|
||||
if sticky[side] {
|
||||
return true;
|
||||
}
|
||||
// Alignment holes do not end a scope; only an omitted excerpt resets it.
|
||||
let previous = (0..position.display)
|
||||
.rev()
|
||||
.take_while(|&display| {
|
||||
!matches!(diff.display_rows[display], DisplayRow::Fold { .. })
|
||||
})
|
||||
.find_map(|display| diff.display_source_row(focus, display));
|
||||
let syntax = if side == 0 { &self.before } else { &self.after };
|
||||
scopes[side] != previous.and_then(|source| syntax.scope_at(source))
|
||||
});
|
||||
if diff.view_mode == ViewMode::Unified
|
||||
&& scopes[0].map(|scope| scope.label) != scopes[1].map(|scope| scope.label)
|
||||
{
|
||||
[[scopes[0].is_some(), false], [false, scopes[1].is_some()]]
|
||||
[[show[0], false], [false, show[1]]]
|
||||
} else {
|
||||
[[scopes[0].is_some(), scopes[1].is_some()], [false; 2]]
|
||||
[show, [false; 2]]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7894,6 +7917,57 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_scope_labels_do_not_repeat_across_hunks_in_one_excerpt() {
|
||||
let before = "def retry_cancellation_email():\n first()\n shared_one()\n old()\n shared_two()\n shared_three()\n tail()\n";
|
||||
let after = "def retry_cancellation_email():\n first()\n inserted_one()\n inserted_two()\n shared_one()\n shared_two()\n inserted_three()\n shared_three()\n tail()\n";
|
||||
for mode in [ViewMode::Split, ViewMode::Unified] {
|
||||
let mut file = syntax_diff_file("retry.py", before, after);
|
||||
file.hunks = crate::diff::line_hunks(&file.before, false, &file.after, false);
|
||||
let mut diff = diff_comparison(vec![file]);
|
||||
diff.files_visible = false;
|
||||
diff.set_view_mode(mode);
|
||||
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 40)).unwrap();
|
||||
let mut panels = WheelPanels::default();
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
if mode == ViewMode::Split {
|
||||
for top in [0, diff.source_display_row(Focus::After, 3).unwrap()] {
|
||||
diff.top = top;
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
for side in [1, 2] {
|
||||
let area = panels.diff.as_ref().unwrap().regions[side].0;
|
||||
assert_eq!(
|
||||
pane_screen(&terminal, area)
|
||||
.matches("› retry_cancellation_email")
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
}
|
||||
}
|
||||
diff.set_context_options(0, 15);
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
for (side, expected) in [(1, 1), (2, 2)] {
|
||||
let area = panels.diff.as_ref().unwrap().regions[side].0;
|
||||
assert_eq!(
|
||||
pane_screen(&terminal, area)
|
||||
.matches("› retry_cancellation_email")
|
||||
.count(),
|
||||
expected
|
||||
);
|
||||
}
|
||||
} else {
|
||||
assert_eq!(
|
||||
screen_text(&terminal)
|
||||
.matches("± retry_cancellation_email")
|
||||
.count(),
|
||||
1
|
||||
);
|
||||
assert!(!screen_text(&terminal).contains("+ retry_cancellation_email"));
|
||||
assert!(!screen_text(&terminal).contains("− retry_cancellation_email"));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_scope_labels_identify_multiple_scopes_and_sticky_renamed_sides() {
|
||||
let before = "impl Review {\n fn old_name(&self) {\n let first = 1;\n let second = 2;\n }\n fn next(&self) {\n let third = 3;\n }\n}";
|
||||
|
||||
Reference in New Issue
Block a user