fix: retain numbered declarations in reserved sticky rows

This commit is contained in:
Valdemar Lorenzen
2026-10-16 11:29:24 +02:00
parent 7b950210b8
commit c689d9e782
3 changed files with 101 additions and 7 deletions
+1
View File
@@ -15,6 +15,7 @@
- [x] file picker file search should also be fuzzy (we should be able to use the same fuzzy implemented for diff view search, i believe) - [x] file picker file search should also be fuzzy (we should be able to use the same fuzzy implemented for diff view search, i believe)
- [x] diff view: labels are currently attached also when scrolling the view port horizontally (such that e.g. a function name is not fully visible in the viewport), which is good, but it currently attaches at that line, thus pushing down the function by one line, which causes a one-line jump when scrolling horizontally. - [x] diff view: labels are currently attached also when scrolling the view port horizontally (such that e.g. a function name is not fully visible in the viewport), which is good, but it currently attaches at that line, thus pushing down the function by one line, which causes a one-line jump when scrolling horizontally.
- horizontal labels overlay the row above the declaration without shifting source rows; hide the label when it would cover the pane border or a visible declaration name. - horizontal labels overlay the row above the declaration without shifting source rows; hide the label when it would cover the pane border or a visible declaration name.
- sticky context retains the immediately adjacent declaration as numbered source when its name fits; only use the label after it scrolls farther out.
- [ ] diff view: add change count (added and removed) to the tab name. format could be something like `Diff · <changes> · <buffer name>`. - [ ] diff view: add change count (added and removed) to the tab name. format could be something like `Diff · <changes> · <buffer name>`.
- [ ] diff view: github integration (code review) - add comments on a pr, add review comments at specific lines of files, request changes, approve pr, etc. - [ ] diff view: github integration (code review) - add comments on a pr, add review comments at specific lines of files, request changes, approve pr, etc.
- [ ] diff view: local review comments (same features as for github reviews) that allows review of local changes to deliver specific reviews to ai agents used for coding. - [ ] diff view: local review comments (same features as for github reviews) that allows review of local changes to deliver specific reviews to ai agents used for coding.
+1
View File
@@ -450,6 +450,7 @@ Review marks belong only to the open comparison and survive filtering, file navi
Text panes show change excerpts with **10 unchanged lines above and below** each change, preserving original file line numbers. Overlapping or touching excerpts merge; omitted stretches show an unchanged-line count in both panes. Files with only metadata changes show their notice or “No text changes” rather than unrelated source. Text panes show change excerpts with **10 unchanged lines above and below** each change, preserving original file line numbers. Overlapping or touching excerpts merge; omitted stretches show an unchanged-line count in both panes. Files with only metadata changes show their notice or “No text changes” rather than unrelated source.
**Scope labels** identify named enclosing functions/classes and containers, such as `impl Review › run`. Muted, unnumbered rows supply context when the declaration is vertically outside that pane's viewport; scrolling it back into view removes the label and reclaims its row. If an unwrapped declaration remains vertically visible but its symbol is clipped horizontally, its label instead overlays the preceding content row. On the first visible row, that label is hidden so the pane border stays intact. Horizontal scrolling never inserts a row or shifts source text; scrolling the symbol back into view restores the covered row. Decorators alone do not count as the declaration name. Visibility follows source columns and wrapped segments independently for each side. Each split side uses its own source, so renamed functions keep distinct old/new names. Unified labels use `−` for Before, `+` for After, and `±` for shared names. Narrow panes prefer the innermost name. Labels are presentation only: motions, selections, copying, editing, and line jumps retain original source coordinates. Context comes from the existing Tree-sitter trees, including supported injected code, without requiring an LSP; unsupported syntax gets no guessed label. **Scope labels** identify named enclosing functions/classes and containers, such as `impl Review › run`. Muted, unnumbered rows supply context when the declaration is vertically outside that pane's viewport; scrolling it back into view removes the label and reclaims its row. If an unwrapped declaration remains vertically visible but its symbol is clipped horizontally, its label instead overlays the preceding content row. On the first visible row, that label is hidden so the pane border stays intact. Horizontal scrolling never inserts a row or shifts source text; scrolling the symbol back into view restores the covered row. Decorators alone do not count as the declaration name. Visibility follows source columns and wrapped segments independently for each side. Each split side uses its own source, so renamed functions keep distinct old/new names. Unified labels use `−` for Before, `+` for After, and `±` for shared names. Narrow panes prefer the innermost name. Labels are presentation only: motions, selections, copying, editing, and line jumps retain original source coordinates. Context comes from the existing Tree-sitter trees, including supported injected code, without requiring an LSP; unsupported syntax gets no guessed label.
When the reserved sticky row can show the immediately preceding declaration with its name visible, it renders that declaration as ordinary numbered source instead. Scrolling farther into the body replaces it with the unnumbered label; scrolling back restores the declaration.
Overlays never cover a visible declaration name on the preceding row, including a different Before/After declaration in unified view; the label is hidden instead. Overlays never cover a visible declaration name on the preceding row, including a different Before/After declaration in unified view; the label is hidden instead.
Within an excerpt, additional hunks and alignment gaps do not repeat an unchanged scope label. A new excerpt or an actual scope change gets a fresh label when needed; sticky context is retained independently for each side. Within an excerpt, additional hunks and alignment gaps do not repeat an unchanged scope label. A new excerpt or an actual scope change gets a fresh label when needed; sticky context is retained independently for each side.
+99 -7
View File
@@ -3774,6 +3774,7 @@ fn draw_diff(
let viewport = panels let viewport = panels
.diff_wrap .diff_wrap
.get_or_insert_with(DiffWrapViewport::default); .get_or_insert_with(DiffWrapViewport::default);
viewport.lefts = [before_left, after_left];
viewport.update( viewport.update(
diff, diff,
file, file,
@@ -4561,6 +4562,7 @@ struct DiffWrapViewport {
before: crate::wrap::WrapCache, before: crate::wrap::WrapCache,
after: crate::wrap::WrapCache, after: crate::wrap::WrapCache,
widths: [usize; 2], widths: [usize; 2],
lefts: [usize; 2],
heights: [usize; 2], heights: [usize; 2],
visible_names: [Vec<Point>; 2], visible_names: [Vec<Point>; 2],
top: DiffWrapPosition, top: DiffWrapPosition,
@@ -4763,6 +4765,55 @@ impl DiffWrapViewport {
}); });
} }
if scope.iter().any(|side| *side) && self.rows.len() + 1 < height { if scope.iter().any(|side| *side) && self.rows.len() + 1 < height {
if self.rows.is_empty() {
// The sticky slot can still show the adjacent declaration as source.
let previous = self.step(diff, file, position, false);
let declaration = self.row(diff, file, previous);
let visible = |side: usize| {
let focus = if side == 0 {
Focus::Before
} else {
Focus::After
};
let Some(context) = contexts[side] else {
return false;
};
if diff.display_source_row(focus, previous.display)
!= Some(context.name.row)
{
return false;
}
let source = if side == 0 { &file.before } else { &file.after };
let prefix = &source[context.name.row][..context.name.column];
let column = display_column(prefix, usize::MAX);
if declaration.unwrapped {
let pane = if diff.view_mode == ViewMode::Unified {
usize::from(diff.focus != Focus::Before)
} else {
side
};
self.lefts[pane] <= column
&& column
< self.lefts[pane].saturating_add(self.widths[pane])
} else {
declaration.segments[side].is_some_and(|segment| {
segment.start <= column && column < segment.end
})
}
};
let restore = if diff.view_mode == ViewMode::Unified {
let side = usize::from(
diff.display_focus(previous.display) != Some(Focus::Before),
);
scope[side] && visible(side)
} else {
(0..2).all(|side| !scope[side] || visible(side))
};
if previous != position && restore {
self.rows.push(declaration);
continue;
}
}
self.rows.push(WrappedDiffRow { self.rows.push(WrappedDiffRow {
position, position,
segments: [None; 2], segments: [None; 2],
@@ -8237,7 +8288,6 @@ mod tests {
Some(2), Some(2),
"Keep the nearest enclosing function's sticky context, not earlier declarations" "Keep the nearest enclosing function's sticky context, not earlier declarations"
); );
assert_eq!(viewport.rows[0].scope, [true; 2]);
} }
#[test] #[test]
@@ -8402,6 +8452,48 @@ mod tests {
} }
} }
#[test]
fn diff_scope_sticky_row_keeps_adjacent_declaration_numbered() {
let after = "\n#[allow(clippy::too_many_arguments)]\nfn draw_diff_scope_labels(\n frame: &mut Frame,\n diff: &DiffState,\n) {\n work();\n}\n";
for (mode, axis, wrap) in [
(ViewMode::Split, SplitAxis::Vertical, false),
(ViewMode::Split, SplitAxis::Horizontal, false),
(ViewMode::Split, SplitAxis::Vertical, true),
(ViewMode::Unified, SplitAxis::Vertical, false),
(ViewMode::Unified, SplitAxis::Vertical, true),
] {
let mut file = syntax_diff_file("tui.rs", "", 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.focus = Focus::After;
diff.split_axis = axis;
diff.word_wrap = wrap;
diff.set_view_mode(mode);
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 24)).unwrap();
let mut panels = WheelPanels::default();
for source in [0, 1, 2, 3, 4, 3, 2] {
diff.top = diff.source_display_row(Focus::After, source).unwrap();
render_diff(&mut terminal, &diff, &mut panels);
let area = diff_inner(panels.diff.as_ref().unwrap().regions[2].0);
let text = pane_screen(&terminal, area);
if source <= 3 {
let declaration = text
.lines()
.find(|line| line.contains("fn draw_diff_scope_labels("));
assert!(
declaration.is_some_and(|line| line.contains("3+")),
"The declaration must keep its source number, not become an unnumbered sticky label at top source {source}:\n{text}"
);
} else {
assert!(text.lines().any(|line| {
line.contains("draw_diff_scope_labels") && !line.contains("fn ")
}));
}
}
}
}
#[test] #[test]
fn diff_scope_labels_follow_declaration_visibility() { 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 before = "@decorate()\nclass PolicyActiveStatusEnum(BaseEnum):\n ACTIVE = 1\n EXPIRED = 2\n FUTURE = 3\n UNKNOWN = 4\n";
@@ -8420,7 +8512,7 @@ mod tests {
diff.set_view_mode(mode); diff.set_view_mode(mode);
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 32)).unwrap(); let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 32)).unwrap();
let mut panels = WheelPanels::default(); let mut panels = WheelPanels::default();
for (source, visible) in [(0, false), (2, true), (1, false), (2, true)] { for (source, visible) in [(0, false), (2, false), (3, true), (1, false), (2, false)] {
diff.top = diff.source_display_row(Focus::After, source).unwrap(); diff.top = diff.source_display_row(Focus::After, source).unwrap();
render_diff(&mut terminal, &diff, &mut panels); render_diff(&mut terminal, &diff, &mut panels);
let text = screen_text(&terminal); let text = screen_text(&terminal);
@@ -8436,7 +8528,7 @@ mod tests {
0 0
} }
); );
assert!(text.contains("ACTIVE = 1")); assert!(text.contains("EXPIRED = 2"));
} }
} }
let before = "def old_name():\n shared()\n tail()\n"; let before = "def old_name():\n shared()\n tail()\n";
@@ -8466,7 +8558,7 @@ mod tests {
let mut diff = diff_comparison(vec![file]); let mut diff = diff_comparison(vec![file]);
diff.files_visible = false; diff.files_visible = false;
diff.set_view_mode(mode); diff.set_view_mode(mode);
diff.top = diff.source_display_row(Focus::After, 1).unwrap(); diff.top = diff.source_display_row(Focus::After, 4).unwrap();
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 40)).unwrap(); let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(160, 40)).unwrap();
let mut panels = WheelPanels::default(); let mut panels = WheelPanels::default();
render_diff(&mut terminal, &diff, &mut panels); render_diff(&mut terminal, &diff, &mut panels);
@@ -8520,7 +8612,7 @@ mod tests {
diff.files_visible = false; diff.files_visible = false;
diff.split_axis = axis; diff.split_axis = axis;
diff.word_wrap = wrapping; diff.word_wrap = wrapping;
diff.top = diff.source_display_row(Focus::Before, 2).unwrap(); diff.top = diff.source_display_row(Focus::Before, 3).unwrap();
let mut terminal = let mut terminal =
Terminal::new(ratatui::backend::TestBackend::new(160, 48)).unwrap(); Terminal::new(ratatui::backend::TestBackend::new(160, 48)).unwrap();
let mut panels = WheelPanels::default(); let mut panels = WheelPanels::default();
@@ -8550,7 +8642,7 @@ mod tests {
} }
let syntax = panels.diff_syntax.as_ref().unwrap(); let syntax = panels.diff_syntax.as_ref().unwrap();
let parses = (syntax.before.parse_counts(), syntax.after.parse_counts()); let parses = (syntax.before.parse_counts(), syntax.after.parse_counts());
diff.top = diff.source_display_row(Focus::Before, 2).unwrap(); diff.top = diff.source_display_row(Focus::Before, 3).unwrap();
render_diff(&mut terminal, &diff, &mut panels); render_diff(&mut terminal, &diff, &mut panels);
let rows = &panels.diff_wrap.as_ref().unwrap().rows; let rows = &panels.diff_wrap.as_ref().unwrap().rows;
assert_eq!(rows[0].scope, [true; 2]); assert_eq!(rows[0].scope, [true; 2]);
@@ -8569,7 +8661,7 @@ mod tests {
} }
let mut narrow = diff_comparison(vec![syntax_diff_file("scope.rs", before, &after)]); let mut narrow = diff_comparison(vec![syntax_diff_file("scope.rs", before, &after)]);
narrow.files_visible = false; narrow.files_visible = false;
narrow.top = narrow.source_display_row(Focus::Before, 2).unwrap(); narrow.top = narrow.source_display_row(Focus::Before, 3).unwrap();
let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(40, 30)).unwrap(); let mut terminal = Terminal::new(ratatui::backend::TestBackend::new(40, 30)).unwrap();
let mut panels = WheelPanels::default(); let mut panels = WheelPanels::default();
render_diff(&mut terminal, &narrow, &mut panels); render_diff(&mut terminal, &narrow, &mut panels);