refactor(terminal): cleanup deprecated flush logic (#6373)

Removes flush logic that was leftover to test against ratatui's flush
Cleaned up the flush logic so it's a bit more intent revealing.
DrawCommand now owns the Cells that it draws as this works around a
borrow checker problem.
This commit is contained in:
Josh McKinney
2025-11-07 15:54:07 -08:00
committed by GitHub
parent db408b9e62
commit 9fba811764
3 changed files with 48 additions and 40 deletions

13
codex-rs/Cargo.lock generated
View File

@@ -1455,6 +1455,7 @@ dependencies = [
"codex-windows-sandbox", "codex-windows-sandbox",
"color-eyre", "color-eyre",
"crossterm", "crossterm",
"derive_more 2.0.1",
"diffy", "diffy",
"dirs", "dirs",
"dunce", "dunce",
@@ -1658,6 +1659,15 @@ dependencies = [
"unicode-segmentation", "unicode-segmentation",
] ]
[[package]]
name = "convert_case"
version = "0.7.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bb402b8d4c85569410425650ce3eddc7d698ed96d39a73f941b08fb63082f1e7"
dependencies = [
"unicode-segmentation",
]
[[package]] [[package]]
name = "core-foundation" name = "core-foundation"
version = "0.9.4" version = "0.9.4"
@@ -2003,7 +2013,7 @@ version = "1.0.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "cb7330aeadfbe296029522e6c40f315320aba36fc43a5b3632f3795348f3bd22" checksum = "cb7330aeadfbe296029522e6c40f315320aba36fc43a5b3632f3795348f3bd22"
dependencies = [ dependencies = [
"convert_case", "convert_case 0.6.0",
"proc-macro2", "proc-macro2",
"quote", "quote",
"syn 2.0.104", "syn 2.0.104",
@@ -2016,6 +2026,7 @@ version = "2.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "bda628edc44c4bb645fbe0f758797143e4e07926f7ebf4e9bdfbd3d2ce621df3" checksum = "bda628edc44c4bb645fbe0f758797143e4e07926f7ebf4e9bdfbd3d2ce621df3"
dependencies = [ dependencies = [
"convert_case 0.7.1",
"proc-macro2", "proc-macro2",
"quote", "quote",
"syn 2.0.104", "syn 2.0.104",

View File

@@ -42,6 +42,7 @@ codex-ollama = { workspace = true }
codex-protocol = { workspace = true } codex-protocol = { workspace = true }
color-eyre = { workspace = true } color-eyre = { workspace = true }
crossterm = { workspace = true, features = ["bracketed-paste", "event-stream"] } crossterm = { workspace = true, features = ["bracketed-paste", "event-stream"] }
derive_more = { workspace = true, features = ["is_variant"] }
diffy = { workspace = true } diffy = { workspace = true }
dirs = { workspace = true } dirs = { workspace = true }
dunce = { workspace = true } dunce = { workspace = true }

View File

@@ -33,6 +33,7 @@ use crossterm::style::SetBackgroundColor;
use crossterm::style::SetColors; use crossterm::style::SetColors;
use crossterm::style::SetForegroundColor; use crossterm::style::SetForegroundColor;
use crossterm::terminal::Clear; use crossterm::terminal::Clear;
use derive_more::IsVariant;
use ratatui::backend::Backend; use ratatui::backend::Backend;
use ratatui::backend::ClearType; use ratatui::backend::ClearType;
use ratatui::buffer::Buffer; use ratatui::buffer::Buffer;
@@ -120,8 +121,6 @@ where
/// Last known position of the cursor. Used to find the new area when the viewport is inlined /// Last known position of the cursor. Used to find the new area when the viewport is inlined
/// and the terminal resized. /// and the terminal resized.
pub last_known_cursor_pos: Position, pub last_known_cursor_pos: Position,
use_custom_flush: bool,
} }
impl<B> Drop for Terminal<B> impl<B> Drop for Terminal<B>
@@ -151,16 +150,12 @@ where
let cursor_pos = backend.get_cursor_position()?; let cursor_pos = backend.get_cursor_position()?;
Ok(Self { Ok(Self {
backend, backend,
buffers: [ buffers: [Buffer::empty(Rect::ZERO), Buffer::empty(Rect::ZERO)],
Buffer::empty(Rect::new(0, 0, 0, 0)),
Buffer::empty(Rect::new(0, 0, 0, 0)),
],
current: 0, current: 0,
hidden_cursor: false, hidden_cursor: false,
viewport_area: Rect::new(0, cursor_pos.y, 0, 0), viewport_area: Rect::new(0, cursor_pos.y, 0, 0),
last_known_screen_size: screen_size, last_known_screen_size: screen_size,
last_known_cursor_pos: cursor_pos, last_known_cursor_pos: cursor_pos,
use_custom_flush: true,
}) })
} }
@@ -173,11 +168,26 @@ where
} }
} }
/// Gets the current buffer as a reference.
fn current_buffer(&self) -> &Buffer {
&self.buffers[self.current]
}
/// Gets the current buffer as a mutable reference. /// Gets the current buffer as a mutable reference.
pub fn current_buffer_mut(&mut self) -> &mut Buffer { fn current_buffer_mut(&mut self) -> &mut Buffer {
&mut self.buffers[self.current] &mut self.buffers[self.current]
} }
/// Gets the previous buffer as a reference.
fn previous_buffer(&self) -> &Buffer {
&self.buffers[1 - self.current]
}
/// Gets the previous buffer as a mutable reference.
fn previous_buffer_mut(&mut self) -> &mut Buffer {
&mut self.buffers[1 - self.current]
}
/// Gets the backend /// Gets the backend
pub const fn backend(&self) -> &B { pub const fn backend(&self) -> &B {
&self.backend &self.backend
@@ -191,26 +201,12 @@ where
/// Obtains a difference between the previous and the current buffer and passes it to the /// Obtains a difference between the previous and the current buffer and passes it to the
/// current backend for drawing. /// current backend for drawing.
pub fn flush(&mut self) -> io::Result<()> { pub fn flush(&mut self) -> io::Result<()> {
let previous_buffer = &self.buffers[1 - self.current]; let updates = diff_buffers(self.previous_buffer(), self.current_buffer());
let current_buffer = &self.buffers[self.current]; let last_put_command = updates.iter().rfind(|command| command.is_put());
if let Some(&DrawCommand::Put { x, y, .. }) = last_put_command {
if self.use_custom_flush { self.last_known_cursor_pos = Position { x, y };
let updates = diff_buffers(previous_buffer, current_buffer);
if let Some(DrawCommand::Put { x, y, .. }) = updates
.iter()
.rev()
.find(|cmd| matches!(cmd, DrawCommand::Put { .. }))
{
self.last_known_cursor_pos = Position { x: *x, y: *y };
}
draw(&mut self.backend, updates.into_iter())
} else {
let updates = previous_buffer.diff(current_buffer);
if let Some((x, y, _)) = updates.last() {
self.last_known_cursor_pos = Position { x: *x, y: *y };
}
self.backend.draw(updates.into_iter())
} }
draw(&mut self.backend, updates.into_iter())
} }
/// Updates the Terminal so that internal buffers match the requested area. /// Updates the Terminal so that internal buffers match the requested area.
@@ -224,8 +220,8 @@ where
/// Sets the viewport area. /// Sets the viewport area.
pub fn set_viewport_area(&mut self, area: Rect) { pub fn set_viewport_area(&mut self, area: Rect) {
self.buffers[self.current].resize(area); self.current_buffer_mut().resize(area);
self.buffers[1 - self.current].resize(area); self.previous_buffer_mut().resize(area);
self.viewport_area = area; self.viewport_area = area;
} }
@@ -337,7 +333,7 @@ where
self.swap_buffers(); self.swap_buffers();
ratatui::backend::Backend::flush(&mut self.backend)?; Backend::flush(&mut self.backend)?;
Ok(()) Ok(())
} }
@@ -381,13 +377,13 @@ where
.set_cursor_position(self.viewport_area.as_position())?; .set_cursor_position(self.viewport_area.as_position())?;
self.backend.clear_region(ClearType::AfterCursor)?; self.backend.clear_region(ClearType::AfterCursor)?;
// Reset the back buffer to make sure the next update will redraw everything. // Reset the back buffer to make sure the next update will redraw everything.
self.buffers[1 - self.current].reset(); self.previous_buffer_mut().reset();
Ok(()) Ok(())
} }
/// Clears the inactive buffer and swaps it with the current buffer /// Clears the inactive buffer and swaps it with the current buffer
pub fn swap_buffers(&mut self) { pub fn swap_buffers(&mut self) {
self.buffers[1 - self.current].reset(); self.previous_buffer_mut().reset();
self.current = 1 - self.current; self.current = 1 - self.current;
} }
@@ -400,13 +396,13 @@ where
use ratatui::buffer::Cell; use ratatui::buffer::Cell;
use unicode_width::UnicodeWidthStr; use unicode_width::UnicodeWidthStr;
#[derive(Debug)] #[derive(Debug, IsVariant)]
enum DrawCommand<'a> { enum DrawCommand {
Put { x: u16, y: u16, cell: &'a Cell }, Put { x: u16, y: u16, cell: Cell },
ClearToEnd { x: u16, y: u16, bg: Color }, ClearToEnd { x: u16, y: u16, bg: Color },
} }
fn diff_buffers<'a>(a: &'a Buffer, b: &'a Buffer) -> Vec<DrawCommand<'a>> { fn diff_buffers(a: &Buffer, b: &Buffer) -> Vec<DrawCommand> {
let previous_buffer = &a.content; let previous_buffer = &a.content;
let next_buffer = &b.content; let next_buffer = &b.content;
@@ -455,7 +451,7 @@ fn diff_buffers<'a>(a: &'a Buffer, b: &'a Buffer) -> Vec<DrawCommand<'a>> {
updates.push(DrawCommand::Put { updates.push(DrawCommand::Put {
x, x,
y, y,
cell: &next_buffer[i], cell: next_buffer[i].clone(),
}); });
} }
} }
@@ -468,9 +464,9 @@ fn diff_buffers<'a>(a: &'a Buffer, b: &'a Buffer) -> Vec<DrawCommand<'a>> {
updates updates
} }
fn draw<'a, I>(writer: &mut impl Write, commands: I) -> io::Result<()> fn draw<I>(writer: &mut impl Write, commands: I) -> io::Result<()>
where where
I: Iterator<Item = DrawCommand<'a>>, I: Iterator<Item = DrawCommand>,
{ {
let mut fg = Color::Reset; let mut fg = Color::Reset;
let mut bg = Color::Reset; let mut bg = Color::Reset;