fix: hide diff scope labels for visible declarations
The visible declaration already provides scope context. Cache the actual identifier point separately from the declaration extent/decorators and hide labels when it is visible in that side of the projected viewport. Refill reclaimed rows and anchor carets against the resulting layout. Retain sticky context when the symbol scrolls or wraps out of view. Verified: 951 tests passed, cargo build, and real PTY smoke for declaration visibility/scroll restoration, wrapping, horizontal clipping, unified view, and short horizontal-pane caret reflow.
This commit is contained in:
+24
-9
@@ -46,12 +46,14 @@ pub struct SyntaxSpan {
|
||||
pub(crate) struct ScopeContext<'a> {
|
||||
pub start_row: usize,
|
||||
pub end_row: usize,
|
||||
pub name: Point,
|
||||
pub label: &'a str,
|
||||
}
|
||||
|
||||
struct CachedScope {
|
||||
start_row: usize,
|
||||
end_row: usize,
|
||||
name: Point,
|
||||
label: String,
|
||||
}
|
||||
|
||||
@@ -334,6 +336,7 @@ impl SyntaxCache {
|
||||
Some(ScopeContext {
|
||||
start_row: scope.start_row,
|
||||
end_row: scope.end_row,
|
||||
name: scope.name,
|
||||
label: &scope.label,
|
||||
})
|
||||
}
|
||||
@@ -581,8 +584,8 @@ fn collect_scopes(
|
||||
{
|
||||
ancestors.pop();
|
||||
}
|
||||
if let Some((extent, name)) =
|
||||
scope_declaration(node, language, source).filter(|(extent, _)| {
|
||||
if let Some((extent, name, label)) =
|
||||
scope_declaration(node, language, source).filter(|(extent, _, _)| {
|
||||
ranges.is_empty()
|
||||
|| ranges.iter().any(|range| {
|
||||
range.start_byte <= extent.start_byte()
|
||||
@@ -594,9 +597,9 @@ fn collect_scopes(
|
||||
.last()
|
||||
.map_or("", |(index, _)| scopes[*index].label.as_str());
|
||||
let label = if parent.is_empty() {
|
||||
name
|
||||
label
|
||||
} else {
|
||||
format!("{parent} › {name}")
|
||||
format!("{parent} › {label}")
|
||||
};
|
||||
let start_row = extent.start_position().row;
|
||||
let end = extent.end_position();
|
||||
@@ -605,6 +608,7 @@ fn collect_scopes(
|
||||
scopes.push(CachedScope {
|
||||
start_row,
|
||||
end_row,
|
||||
name,
|
||||
label,
|
||||
});
|
||||
let end_row = end_row.min(rows.len());
|
||||
@@ -633,7 +637,7 @@ fn scope_declaration<'a>(
|
||||
node: Node<'a>,
|
||||
language: &str,
|
||||
source: &str,
|
||||
) -> Option<(Node<'a>, String)> {
|
||||
) -> Option<(Node<'a>, Point, String)> {
|
||||
if !node.is_named() || node.is_missing() {
|
||||
return None;
|
||||
}
|
||||
@@ -646,7 +650,7 @@ fn scope_declaration<'a>(
|
||||
Some(trait_name) => scope_label(&["impl", trait_name, "for", target]),
|
||||
None => scope_label(&["impl", target]),
|
||||
};
|
||||
return Some((node, label));
|
||||
return Some((node, field("type")?.start_position(), label));
|
||||
}
|
||||
(
|
||||
"rust",
|
||||
@@ -686,8 +690,12 @@ fn scope_declaration<'a>(
|
||||
.named_child(0)?
|
||||
.child_by_field_name("type")?;
|
||||
let receiver = scope_text(receiver, source)?;
|
||||
let name = scope_text(field("name")?, source)?;
|
||||
return Some((node, scope_label(&[receiver, "›", name])));
|
||||
let name = field("name")?;
|
||||
return Some((
|
||||
node,
|
||||
name.start_position(),
|
||||
scope_label(&[receiver, "›", scope_text(name, source)?]),
|
||||
));
|
||||
}
|
||||
("go", "type_spec")
|
||||
if field("type")
|
||||
@@ -764,7 +772,11 @@ fn scope_declaration<'a>(
|
||||
{
|
||||
extent = parent;
|
||||
}
|
||||
Some((extent, scope_label(&[scope_text(name, source)?])))
|
||||
Some((
|
||||
extent,
|
||||
name.start_position(),
|
||||
scope_label(&[scope_text(name, source)?]),
|
||||
))
|
||||
}
|
||||
|
||||
fn scope_label(parts: &[&str]) -> String {
|
||||
@@ -1567,6 +1579,7 @@ mod tests {
|
||||
Some(ScopeContext {
|
||||
start_row: 1,
|
||||
end_row: 4,
|
||||
name: Point::new(1, 7),
|
||||
label: "impl State › old",
|
||||
})
|
||||
);
|
||||
@@ -1578,6 +1591,7 @@ mod tests {
|
||||
Some(ScopeContext {
|
||||
start_row: 6,
|
||||
end_row: 9,
|
||||
name: Point::new(6, 3),
|
||||
label: "next",
|
||||
})
|
||||
);
|
||||
@@ -1605,6 +1619,7 @@ mod tests {
|
||||
Some(ScopeContext {
|
||||
start_row: 1,
|
||||
end_row: 4,
|
||||
name: Point::new(2, 8),
|
||||
label: "State › run",
|
||||
})
|
||||
);
|
||||
|
||||
+200
-87
@@ -30,6 +30,7 @@ use ratatui::{
|
||||
text::{Line, Span},
|
||||
widgets::{Block, Borders, Clear, Paragraph, Wrap},
|
||||
};
|
||||
use tree_sitter::Point;
|
||||
use unicode_segmentation::UnicodeSegmentation;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
@@ -3698,32 +3699,6 @@ fn draw_diff(
|
||||
.then_some(height / 2)
|
||||
});
|
||||
let toggle_row = center_row.or(toggle_row);
|
||||
if let Some(file) = selected.filter(|file| file.loaded)
|
||||
&& let Some(syntax) = &panels.diff_syntax
|
||||
{
|
||||
let caret = editor
|
||||
.filter(|editor| editor.diff_interacting() && diff.follow_caret)
|
||||
.and_then(|editor| {
|
||||
editor
|
||||
.diff_buffer_focus()
|
||||
.map(|focus| (focus, editor.cursor()))
|
||||
});
|
||||
let viewport = panels
|
||||
.diff_wrap
|
||||
.get_or_insert_with(DiffWrapViewport::default);
|
||||
viewport.update(
|
||||
diff,
|
||||
file,
|
||||
[before_area, after_area],
|
||||
caret,
|
||||
toggle_row,
|
||||
editor.map_or(Some(0), Editor::scroll_distance),
|
||||
syntax,
|
||||
);
|
||||
top = viewport.top.display;
|
||||
} else {
|
||||
panels.diff_wrap = None;
|
||||
}
|
||||
if let Some(editor) =
|
||||
editor.filter(|editor| !wrapping && editor.diff_interacting() && diff.follow_caret)
|
||||
&& let Some(focus) = editor.diff_buffer_focus()
|
||||
@@ -3745,6 +3720,33 @@ fn draw_diff(
|
||||
let column = display_column(&editor.lines()[editor.cursor().row], editor.cursor().column);
|
||||
*left = scroll_offset(*left, column, width, editor.scroll_distance());
|
||||
}
|
||||
if let Some(file) = selected.filter(|file| file.loaded)
|
||||
&& let Some(syntax) = &panels.diff_syntax
|
||||
{
|
||||
let caret = editor
|
||||
.filter(|editor| editor.diff_interacting() && diff.follow_caret)
|
||||
.and_then(|editor| {
|
||||
editor
|
||||
.diff_buffer_focus()
|
||||
.map(|focus| (focus, editor.cursor()))
|
||||
});
|
||||
let viewport = panels
|
||||
.diff_wrap
|
||||
.get_or_insert_with(DiffWrapViewport::default);
|
||||
viewport.lefts = [before_left, after_left];
|
||||
viewport.update(
|
||||
diff,
|
||||
file,
|
||||
[before_area, after_area],
|
||||
caret,
|
||||
toggle_row,
|
||||
editor.map_or(Some(0), Editor::scroll_distance),
|
||||
syntax,
|
||||
);
|
||||
top = viewport.top.display;
|
||||
} else {
|
||||
panels.diff_wrap = None;
|
||||
}
|
||||
if let Some(id) = diff.comparison_id
|
||||
&& let Some(file) = selected.filter(|file| file.loaded)
|
||||
{
|
||||
@@ -4390,6 +4392,9 @@ struct DiffWrapViewport {
|
||||
before: crate::wrap::WrapCache,
|
||||
after: crate::wrap::WrapCache,
|
||||
widths: [usize; 2],
|
||||
lefts: [usize; 2],
|
||||
heights: [usize; 2],
|
||||
visible_names: [Vec<Point>; 2],
|
||||
top: DiffWrapPosition,
|
||||
rows: Vec<WrappedDiffRow>,
|
||||
}
|
||||
@@ -4532,6 +4537,37 @@ impl DiffWrapViewport {
|
||||
position
|
||||
}
|
||||
|
||||
fn declaration_visible(
|
||||
&self,
|
||||
diff: &DiffState,
|
||||
file: &ChangedFile,
|
||||
side: usize,
|
||||
name: Point,
|
||||
) -> bool {
|
||||
let focus = if side == 0 {
|
||||
Focus::Before
|
||||
} else {
|
||||
Focus::After
|
||||
};
|
||||
let source = if side == 0 { &file.before } else { &file.after };
|
||||
let column = display_column(source.get(name.row).map_or("", String::as_str), name.column);
|
||||
self.rows.iter().take(self.heights[side]).any(|row| {
|
||||
if row.scope != [false; 2]
|
||||
|| diff.display_source_row(focus, row.position.display) != Some(name.row)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
let (left, right) = if row.unwrapped {
|
||||
(self.lefts[side], self.lefts[side] + self.widths[side])
|
||||
} else if let Some(segment) = row.segments[side] {
|
||||
(segment.start, segment.end)
|
||||
} else {
|
||||
return false;
|
||||
};
|
||||
left <= column && column < right
|
||||
})
|
||||
}
|
||||
|
||||
fn fill(
|
||||
&mut self,
|
||||
diff: &DiffState,
|
||||
@@ -4539,36 +4575,69 @@ impl DiffWrapViewport {
|
||||
height: usize,
|
||||
syntax: &DiffSyntaxCache,
|
||||
) {
|
||||
self.rows.clear();
|
||||
if diff.display_rows.is_empty() {
|
||||
return;
|
||||
for names in &mut self.visible_names {
|
||||
names.clear();
|
||||
}
|
||||
let mut position = self.top;
|
||||
let mut sticky = [true; 2];
|
||||
while self.rows.len() < height {
|
||||
for scope in syntax.labels(diff, position, sticky) {
|
||||
if scope.iter().any(|side| *side) && self.rows.len() + 1 < height {
|
||||
self.rows.push(WrappedDiffRow {
|
||||
position,
|
||||
segments: [None; 2],
|
||||
final_segments: [false; 2],
|
||||
scope,
|
||||
unwrapped: false,
|
||||
});
|
||||
// ponytail: reflow only this viewport; index visual rows if tall terminals make it hot.
|
||||
loop {
|
||||
self.rows.clear();
|
||||
if diff.display_rows.is_empty() {
|
||||
return;
|
||||
}
|
||||
let mut position = self.top;
|
||||
let mut sticky = [true; 2];
|
||||
while self.rows.len() < height {
|
||||
let contexts = syntax.scopes(diff, position.display);
|
||||
for mut scope in syntax.labels(diff, position, sticky) {
|
||||
for side in 0..2 {
|
||||
scope[side] &= !contexts[side].is_some_and(|context| {
|
||||
self.visible_names[side].contains(&context.name)
|
||||
});
|
||||
}
|
||||
if scope.iter().any(|side| *side) && self.rows.len() + 1 < height {
|
||||
self.rows.push(WrappedDiffRow {
|
||||
position,
|
||||
segments: [None; 2],
|
||||
final_segments: [false; 2],
|
||||
scope,
|
||||
unwrapped: false,
|
||||
});
|
||||
}
|
||||
}
|
||||
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);
|
||||
if next == position {
|
||||
break;
|
||||
}
|
||||
position = next;
|
||||
}
|
||||
let mut changed = false;
|
||||
for index in 0..self.rows.len() {
|
||||
let row = self.rows[index];
|
||||
if row.scope == [false; 2] {
|
||||
continue;
|
||||
}
|
||||
let contexts = syntax.scopes(diff, row.position.display);
|
||||
for side in 0..2 {
|
||||
if let Some(context) = contexts[side]
|
||||
&& row.scope[side]
|
||||
&& !self.visible_names[side].contains(&context.name)
|
||||
&& self.declaration_visible(diff, file, side, context.name)
|
||||
{
|
||||
self.visible_names[side].push(context.name);
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
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);
|
||||
if next == position {
|
||||
if !changed {
|
||||
break;
|
||||
}
|
||||
position = next;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4631,6 +4700,7 @@ impl DiffWrapViewport {
|
||||
diff_pane_text_width(diff, file, areas[0], Focus::Before),
|
||||
diff_pane_text_width(diff, file, areas[1], Focus::After),
|
||||
];
|
||||
self.heights = areas.map(|area| usize::from(diff_inner(area).height));
|
||||
self.before.prepare((diff.file, id), self.widths[0]);
|
||||
self.after
|
||||
.prepare((diff.file, self.revision), self.widths[1]);
|
||||
@@ -4673,27 +4743,30 @@ impl DiffWrapViewport {
|
||||
if let Some(row) = anchor {
|
||||
self.top = position;
|
||||
let target = row.min(height.saturating_sub(1));
|
||||
let label_count = |position, sticky| {
|
||||
syntax
|
||||
.labels(diff, position, [sticky; 2])
|
||||
.iter()
|
||||
.filter(|scope| scope.iter().any(|side| *side))
|
||||
.count()
|
||||
.min(height.saturating_sub(1))
|
||||
};
|
||||
let mut screen = label_count(self.top, true);
|
||||
while screen < target {
|
||||
self.fill(diff, file, paint_height, syntax);
|
||||
while self
|
||||
.rows
|
||||
.iter()
|
||||
.position(|row| row.scope == [false; 2] && row.position == position)
|
||||
.is_some_and(|screen| screen < target)
|
||||
{
|
||||
let previous = self.step(diff, file, self.top, false);
|
||||
let next_screen =
|
||||
screen + 1 + label_count(self.top, false) + label_count(previous, true)
|
||||
- label_count(self.top, true);
|
||||
if previous == self.top || next_screen > target {
|
||||
if previous == self.top {
|
||||
break;
|
||||
}
|
||||
let top = self.top;
|
||||
self.top = previous;
|
||||
screen = next_screen;
|
||||
self.fill(diff, file, paint_height, syntax);
|
||||
let screen = self
|
||||
.rows
|
||||
.iter()
|
||||
.position(|row| row.scope == [false; 2] && row.position == position);
|
||||
if screen.is_none_or(|screen| screen > target) {
|
||||
self.top = top;
|
||||
self.fill(diff, file, paint_height, syntax);
|
||||
break;
|
||||
}
|
||||
}
|
||||
self.fill(diff, file, paint_height, syntax);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -7917,6 +7990,60 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn diff_scope_labels_follow_declaration_visibility() {
|
||||
let before = "@decorate()\nclass PolicyActiveStatusEnum(BaseEnum):\n ACTIVE = 1\n EXPIRED = 2\n FUTURE = 3\n UNKNOWN = 4\n";
|
||||
let after = before.replace(" FUTURE", " CANCELLED = 5\n FUTURE");
|
||||
for (mode, axis, wrap) in [
|
||||
(ViewMode::Split, SplitAxis::Vertical, false),
|
||||
(ViewMode::Split, SplitAxis::Horizontal, true),
|
||||
(ViewMode::Unified, SplitAxis::Vertical, true),
|
||||
] {
|
||||
let mut file = syntax_diff_file("status.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.split_axis = axis;
|
||||
diff.word_wrap = wrap;
|
||||
diff.set_view_mode(mode);
|
||||
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 32)).unwrap();
|
||||
let mut panels = WheelPanels::default();
|
||||
for (source, visible) in [(0, false), (2, true), (1, false), (2, true)] {
|
||||
diff.top = diff.source_display_row(Focus::After, source).unwrap();
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
let text = screen_text(&terminal);
|
||||
let count = ["› PolicyActiveStatusEnum", "± PolicyActiveStatusEnum"]
|
||||
.iter()
|
||||
.map(|label| text.matches(label).count())
|
||||
.sum::<usize>();
|
||||
assert_eq!(
|
||||
count,
|
||||
if visible {
|
||||
if mode == ViewMode::Split { 2 } else { 1 }
|
||||
} else {
|
||||
0
|
||||
}
|
||||
);
|
||||
assert!(text.contains("ACTIVE = 1"));
|
||||
}
|
||||
}
|
||||
let before = "def old_name():\n shared()\n tail()\n";
|
||||
let after = before.replace("old_name", "new_name");
|
||||
let mut file = syntax_diff_file("renamed.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(ViewMode::Unified);
|
||||
diff.top = diff.source_display_row(Focus::After, 0).unwrap();
|
||||
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(100, 16)).unwrap();
|
||||
let mut panels = WheelPanels::default();
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
let text = screen_text(&terminal);
|
||||
assert!(text.contains("def new_name():"));
|
||||
assert!(text.contains("− old_name"));
|
||||
assert!(!text.contains("+ new_name"));
|
||||
}
|
||||
|
||||
#[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";
|
||||
@@ -7927,11 +8054,12 @@ mod tests {
|
||||
let mut diff = diff_comparison(vec![file]);
|
||||
diff.files_visible = false;
|
||||
diff.set_view_mode(mode);
|
||||
diff.top = diff.source_display_row(Focus::After, 1).unwrap();
|
||||
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()] {
|
||||
for top in [diff.top, diff.source_display_row(Focus::After, 3).unwrap()] {
|
||||
diff.top = top;
|
||||
render_diff(&mut terminal, &diff, &mut panels);
|
||||
for side in [1, 2] {
|
||||
@@ -7980,6 +8108,7 @@ mod tests {
|
||||
diff.files_visible = false;
|
||||
diff.split_axis = axis;
|
||||
diff.word_wrap = wrapping;
|
||||
diff.top = diff.source_display_row(Focus::Before, 2).unwrap();
|
||||
let mut terminal =
|
||||
Terminal::new(ratatui::backend::TestBackend::new(160, 48)).unwrap();
|
||||
let mut panels = WheelPanels::default();
|
||||
@@ -8003,7 +8132,8 @@ mod tests {
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert!(labels.iter().any(|label| label.contains(name)));
|
||||
assert!(labels.iter().any(|label| label.contains("next")));
|
||||
assert!(!labels.iter().any(|label| label.contains("next")));
|
||||
assert!(pane_screen(&terminal, area).contains("fn next"));
|
||||
assert!(!labels.iter().any(|label| label.contains(other)));
|
||||
}
|
||||
let syntax = panels.diff_syntax.as_ref().unwrap();
|
||||
@@ -8027,6 +8157,7 @@ mod tests {
|
||||
}
|
||||
let mut narrow = diff_comparison(vec![syntax_diff_file("scope.rs", before, &after)]);
|
||||
narrow.files_visible = false;
|
||||
narrow.top = narrow.source_display_row(Focus::Before, 2).unwrap();
|
||||
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(40, 30)).unwrap();
|
||||
let mut panels = WheelPanels::default();
|
||||
render_diff(&mut terminal, &narrow, &mut panels);
|
||||
@@ -12825,24 +12956,6 @@ end
|
||||
diff.display_source_row(diff.focus, rows[caret_row].position.display),
|
||||
Some(source.row)
|
||||
);
|
||||
assert!(rows.iter().any(|row| row.scope != [false; 2]));
|
||||
if diff.view_mode == ViewMode::Unified {
|
||||
let labels = rows
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter(|(_, row)| row.scope != [false; 2])
|
||||
.map(|(screen, _)| {
|
||||
(area.x..area.right())
|
||||
.map(|x| {
|
||||
terminal.backend().buffer()[(x, area.y + screen as u16)]
|
||||
.symbol()
|
||||
})
|
||||
.collect::<String>()
|
||||
})
|
||||
.collect::<Vec<_>>();
|
||||
assert!(labels.iter().any(|label| label.contains("− old_name")));
|
||||
assert!(labels.iter().any(|label| label.contains("+ new_name")));
|
||||
}
|
||||
for (screen, row) in rows.iter().take(usize::from(area.height)).enumerate() {
|
||||
if row.scope == [false; 2] {
|
||||
continue;
|
||||
|
||||
Reference in New Issue
Block a user