Skip to content

Commit 2f6788d

Browse files
committed
Fix transcript pager page continuity
Use the pager content height for PageUp/PageDown so transcript pages are continuous and reversible. Fixes #7356.
1 parent aaec8ab commit 2f6788d

File tree

1 file changed

+98
-4
lines changed

1 file changed

+98
-4
lines changed

codex-rs/tui/src/pager_overlay.rs

Lines changed: 98 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,12 +241,12 @@ impl PagerView {
241241
self.scroll_offset = self.scroll_offset.saturating_add(1);
242242
}
243243
e if KEY_PAGE_UP.is_press(e) => {
244-
let area = self.content_area(tui.terminal.viewport_area);
245-
self.scroll_offset = self.scroll_offset.saturating_sub(area.height as usize);
244+
let page_h = self.page_step(tui.terminal.viewport_area);
245+
self.scroll_offset = self.scroll_offset.saturating_sub(page_h);
246246
}
247247
e if KEY_PAGE_DOWN.is_press(e) || KEY_SPACE.is_press(e) => {
248-
let area = self.content_area(tui.terminal.viewport_area);
249-
self.scroll_offset = self.scroll_offset.saturating_add(area.height as usize);
248+
let page_h = self.page_step(tui.terminal.viewport_area);
249+
self.scroll_offset = self.scroll_offset.saturating_add(page_h);
250250
}
251251
e if KEY_HOME.is_press(e) => {
252252
self.scroll_offset = 0;
@@ -263,6 +263,11 @@ impl PagerView {
263263
Ok(())
264264
}
265265

266+
fn page_step(&self, viewport_area: Rect) -> usize {
267+
self.last_content_height
268+
.unwrap_or_else(|| self.content_area(viewport_area).height as usize)
269+
}
270+
266271
fn update_last_content_height(&mut self, height: u16) {
267272
self.last_content_height = Some(height as usize);
268273
}
@@ -812,6 +817,95 @@ mod tests {
812817
assert_snapshot!(term.backend());
813818
}
814819

820+
/// Render transcript overlay and return visible line numbers (`line-NN`) in order.
821+
fn transcript_line_numbers(overlay: &mut TranscriptOverlay, area: Rect) -> Vec<usize> {
822+
let mut buf = Buffer::empty(area);
823+
overlay.render(area, &mut buf);
824+
825+
let top_h = area.height.saturating_sub(3);
826+
let top = Rect::new(area.x, area.y, area.width, top_h);
827+
let content_area = overlay.view.content_area(top);
828+
829+
let mut nums = Vec::new();
830+
for y in content_area.y..content_area.bottom() {
831+
let mut line = String::new();
832+
for x in content_area.x..content_area.right() {
833+
line.push(buf[(x, y)].symbol().chars().next().unwrap_or(' '));
834+
}
835+
if let Some(n) = line
836+
.split_whitespace()
837+
.find_map(|w| w.strip_prefix("line-"))
838+
.and_then(|s| s.parse().ok())
839+
{
840+
nums.push(n);
841+
}
842+
}
843+
nums
844+
}
845+
846+
#[test]
847+
fn transcript_overlay_paging_is_continuous_and_round_trips() {
848+
let mut overlay = TranscriptOverlay::new(
849+
(0..50)
850+
.map(|i| {
851+
Arc::new(TestCell {
852+
lines: vec![Line::from(format!("line-{i:02}"))],
853+
}) as Arc<dyn HistoryCell>
854+
})
855+
.collect(),
856+
);
857+
let area = Rect::new(0, 0, 40, 15);
858+
859+
// Prime layout so last_content_height is populated and we know the page height.
860+
let mut buf = Buffer::empty(area);
861+
overlay.view.scroll_offset = 0;
862+
overlay.render(area, &mut buf);
863+
let page_step = overlay.view.page_step(area);
864+
865+
for &start in &[0_usize, 3, page_step] {
866+
// PageDown continuity.
867+
overlay.view.scroll_offset = start;
868+
let page1 = transcript_line_numbers(&mut overlay, area);
869+
assert!(!page1.is_empty());
870+
let last1 = *page1.last().unwrap();
871+
872+
overlay.view.scroll_offset = overlay.view.scroll_offset.saturating_add(page_step);
873+
let page2 = transcript_line_numbers(&mut overlay, area);
874+
assert!(!page2.is_empty());
875+
assert_eq!(
876+
last1 + 1,
877+
page2[0],
878+
"PageDown from {start} should be continuous"
879+
);
880+
881+
// PageDown then PageUp round-trips.
882+
overlay.view.scroll_offset = start;
883+
let before = transcript_line_numbers(&mut overlay, area);
884+
overlay.view.scroll_offset = overlay.view.scroll_offset.saturating_add(page_step);
885+
let _ = transcript_line_numbers(&mut overlay, area);
886+
overlay.view.scroll_offset = overlay.view.scroll_offset.saturating_sub(page_step);
887+
let after = transcript_line_numbers(&mut overlay, area);
888+
assert_eq!(
889+
before, after,
890+
"PageDown+PageUp from {start} should round-trip"
891+
);
892+
893+
// PageUp then PageDown round-trips for interior offsets.
894+
if start == page_step {
895+
overlay.view.scroll_offset = start;
896+
let before = transcript_line_numbers(&mut overlay, area);
897+
overlay.view.scroll_offset = overlay.view.scroll_offset.saturating_sub(page_step);
898+
let _ = transcript_line_numbers(&mut overlay, area);
899+
overlay.view.scroll_offset = overlay.view.scroll_offset.saturating_add(page_step);
900+
let after = transcript_line_numbers(&mut overlay, area);
901+
assert_eq!(
902+
before, after,
903+
"PageUp+PageDown from {start} should round-trip"
904+
);
905+
}
906+
}
907+
}
908+
815909
#[test]
816910
fn static_overlay_wraps_long_lines() {
817911
let mut overlay = StaticOverlay::with_title(

0 commit comments

Comments
 (0)