mirror of
https://github.com/openai/codex.git
synced 2026-10-20 17:38:45 +00:00
fix(tui): coordinate terminal pane surfaces
This commit is contained in:
@@ -152,6 +152,13 @@ impl ConversationPanes {
|
||||
return false;
|
||||
}
|
||||
self.focused = slot;
|
||||
self.for_each_installed_mut(|pane| {
|
||||
pane.chat_widget
|
||||
.set_terminal_title_output_enabled(pane.slot == slot);
|
||||
});
|
||||
if let Some(pane) = self.by_slot_mut(slot) {
|
||||
pane.chat_widget.refresh_terminal_title();
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
@@ -159,13 +166,15 @@ impl ConversationPanes {
|
||||
&mut self,
|
||||
init: ConversationPaneInit,
|
||||
) -> Result<Option<ConversationPane>, ConversationPaneInit> {
|
||||
let pane = ConversationPane::new(init, PaneSlot::Side)?;
|
||||
let mut pane = ConversationPane::new(init, PaneSlot::Side)?;
|
||||
pane.chat_widget
|
||||
.set_terminal_title_output_enabled(self.focused == PaneSlot::Side);
|
||||
Ok(self.side.replace(pane))
|
||||
}
|
||||
|
||||
pub(super) fn take_side(&mut self) -> Option<ConversationPane> {
|
||||
if self.focused == PaneSlot::Side {
|
||||
self.focused = PaneSlot::Parent;
|
||||
self.focus(PaneSlot::Parent);
|
||||
}
|
||||
if self.dispatch == Some(PaneSlot::Side) {
|
||||
self.dispatch = None;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
use super::*;
|
||||
use crate::app_event::HistoryLookupResponse;
|
||||
use crate::bottom_pane::TerminalTitleItem;
|
||||
use crate::chatwidget::tests::constructor::make_chatwidget_for_pane;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
@@ -96,6 +97,104 @@ async fn taking_focused_side_restores_parent_selection() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn only_focused_pane_owns_terminal_title_output() {
|
||||
let (parent, _parent_rx) = pane_init(PaneSlot::Parent).await;
|
||||
let (side, _side_rx) = pane_init(PaneSlot::Side).await;
|
||||
let Ok(mut panes) = ConversationPanes::new_parent(parent) else {
|
||||
panic!("parent pane should install");
|
||||
};
|
||||
assert!(panes.install_side(side).is_ok());
|
||||
|
||||
panes
|
||||
.by_slot_mut(PaneSlot::Parent)
|
||||
.expect("parent pane")
|
||||
.setup_terminal_title(vec![TerminalTitleItem::Project]);
|
||||
panes
|
||||
.by_slot_mut(PaneSlot::Side)
|
||||
.expect("side pane")
|
||||
.setup_terminal_title(vec![TerminalTitleItem::Project]);
|
||||
assert_eq!(
|
||||
panes
|
||||
.by_slot(PaneSlot::Parent)
|
||||
.expect("parent pane")
|
||||
.last_terminal_title
|
||||
.as_deref(),
|
||||
Some("project")
|
||||
);
|
||||
assert_eq!(
|
||||
panes
|
||||
.by_slot(PaneSlot::Side)
|
||||
.expect("side pane")
|
||||
.last_terminal_title,
|
||||
None
|
||||
);
|
||||
|
||||
assert!(panes.focus(PaneSlot::Side));
|
||||
assert_eq!(
|
||||
panes
|
||||
.by_slot(PaneSlot::Parent)
|
||||
.expect("parent pane")
|
||||
.last_terminal_title,
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
panes
|
||||
.by_slot(PaneSlot::Side)
|
||||
.expect("side pane")
|
||||
.last_terminal_title
|
||||
.as_deref(),
|
||||
Some("project")
|
||||
);
|
||||
|
||||
let removed = panes.take_side().expect("side pane");
|
||||
assert_eq!(removed.last_terminal_title, None);
|
||||
assert_eq!(
|
||||
panes
|
||||
.by_slot(PaneSlot::Parent)
|
||||
.expect("parent pane")
|
||||
.last_terminal_title
|
||||
.as_deref(),
|
||||
Some("project")
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn focus_clears_previous_terminal_title_when_new_owner_has_none() {
|
||||
let (parent, _parent_rx) = pane_init(PaneSlot::Parent).await;
|
||||
let (side, _side_rx) = pane_init(PaneSlot::Side).await;
|
||||
let Ok(mut panes) = ConversationPanes::new_parent(parent) else {
|
||||
panic!("parent pane should install");
|
||||
};
|
||||
assert!(panes.install_side(side).is_ok());
|
||||
panes
|
||||
.by_slot_mut(PaneSlot::Parent)
|
||||
.expect("parent pane")
|
||||
.setup_terminal_title(Vec::new());
|
||||
assert!(panes.focus(PaneSlot::Side));
|
||||
panes
|
||||
.by_slot_mut(PaneSlot::Side)
|
||||
.expect("side pane")
|
||||
.last_terminal_title = Some("side-only title".to_string());
|
||||
|
||||
assert!(panes.focus(PaneSlot::Parent));
|
||||
|
||||
assert_eq!(
|
||||
panes
|
||||
.by_slot(PaneSlot::Side)
|
||||
.expect("side pane")
|
||||
.last_terminal_title,
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
panes
|
||||
.by_slot(PaneSlot::Parent)
|
||||
.expect("parent pane")
|
||||
.last_terminal_title,
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn installed_thread_ids_are_parent_first_and_unique() {
|
||||
let (parent, _parent_rx) = pane_init(PaneSlot::Parent).await;
|
||||
|
||||
@@ -2252,7 +2252,9 @@ impl App {
|
||||
match apply_result {
|
||||
Ok(()) => {
|
||||
self.config.tui_terminal_title = Some(ids.clone());
|
||||
self.chat_widget.setup_terminal_title(items);
|
||||
self.chat_widget.for_each_installed_mut(|pane| {
|
||||
pane.chat_widget.setup_terminal_title(items.clone());
|
||||
});
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::error!(error = %err, "failed to persist terminal title items; keeping previous selection");
|
||||
|
||||
@@ -4,7 +4,9 @@ use super::*;
|
||||
|
||||
impl App {
|
||||
pub(super) fn disable_ambient_pet_before_shutdown(&mut self, tui: &mut tui::Tui) -> Result<()> {
|
||||
self.chat_widget.disable_ambient_pet_for_session();
|
||||
self.chat_widget.for_each_installed_mut(|pane| {
|
||||
pane.chat_widget.disable_ambient_pet_for_session();
|
||||
});
|
||||
if let Err(clear_err) = tui.clear_ambient_pet_image() {
|
||||
match clear_err {
|
||||
crate::pets::PetImageRenderError::Terminal(err) => return Err(err.into()),
|
||||
@@ -31,7 +33,9 @@ impl App {
|
||||
error = %err,
|
||||
"failed to render ambient pet image; disabling pet for session"
|
||||
);
|
||||
self.chat_widget.disable_ambient_pet_for_session();
|
||||
self.chat_widget.for_each_installed_mut(|pane| {
|
||||
pane.chat_widget.disable_ambient_pet_for_session();
|
||||
});
|
||||
if let Err(clear_err) = tui.clear_ambient_pet_image() {
|
||||
match clear_err {
|
||||
crate::pets::PetImageRenderError::Terminal(err) => return Err(err.into()),
|
||||
@@ -154,8 +158,18 @@ impl App {
|
||||
{
|
||||
Ok(()) => {
|
||||
self.config.tui_pet = Some(pet_id.clone());
|
||||
self.chat_widget
|
||||
.set_tui_pet_loaded(Some(pet_id), ambient_pet);
|
||||
let selected_origin = self.chat_widget.origin();
|
||||
let mut selected_ambient_pet = Some(ambient_pet);
|
||||
self.chat_widget.for_each_installed_mut(|pane| {
|
||||
if pane.origin() == selected_origin {
|
||||
pane.chat_widget.set_tui_pet_loaded(
|
||||
Some(pet_id.clone()),
|
||||
selected_ambient_pet.take().flatten(),
|
||||
);
|
||||
} else {
|
||||
pane.chat_widget.set_tui_pet(Some(pet_id.clone()));
|
||||
}
|
||||
});
|
||||
}
|
||||
Err(err) => {
|
||||
self.chat_widget
|
||||
@@ -195,3 +209,7 @@ impl App {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "pets_tests.rs"]
|
||||
mod tests;
|
||||
|
||||
@@ -0,0 +1,52 @@
|
||||
use super::*;
|
||||
use crate::app::conversation_panes::ConversationPaneInit;
|
||||
use crate::app::test_support::make_test_app;
|
||||
use crate::chatwidget::tests::constructor::make_chatwidget_for_pane;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tempfile::tempdir;
|
||||
|
||||
async fn install_test_side(app: &mut App) {
|
||||
let (side_widget, _side_rx) = make_chatwidget_for_pane(PaneSlot::Side).await;
|
||||
let file_search = FileSearchManager::new(
|
||||
side_widget.config_ref().cwd.to_path_buf(),
|
||||
side_widget.conversation_event_sender(),
|
||||
);
|
||||
let installed = app.chat_widget.install_side(ConversationPaneInit {
|
||||
chat_widget: side_widget,
|
||||
file_search,
|
||||
owned_screen: None,
|
||||
});
|
||||
assert!(installed.is_ok(), "side pane should install");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn selected_pet_updates_both_installed_widget_configs() -> Result<()> {
|
||||
let mut app = make_test_app().await;
|
||||
install_test_side(&mut app).await;
|
||||
let codex_home = tempdir()?;
|
||||
app.config.codex_home = codex_home.path().to_path_buf().abs();
|
||||
let request_id = app.chat_widget.show_pet_selection_loading_popup();
|
||||
let mut tui = crate::tui::test_support::make_test_tui()?;
|
||||
|
||||
app.handle_pet_selection_loaded(
|
||||
&mut tui,
|
||||
request_id,
|
||||
"chefito".to_string(),
|
||||
/*result*/ Ok(None),
|
||||
)
|
||||
.await?;
|
||||
|
||||
assert_eq!(app.config.tui_pet.as_deref(), Some("chefito"));
|
||||
for slot in [PaneSlot::Parent, PaneSlot::Side] {
|
||||
assert_eq!(
|
||||
app.chat_widget
|
||||
.by_slot(slot)
|
||||
.expect("installed pane")
|
||||
.config_ref()
|
||||
.tui_pet
|
||||
.as_deref(),
|
||||
Some("chefito")
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
@@ -702,6 +702,8 @@ pub(crate) struct ChatWidget {
|
||||
terminal_title_invalid_items_warned: Arc<AtomicBool>,
|
||||
// Last terminal title emitted, to avoid writing duplicate OSC updates.
|
||||
pub(crate) last_terminal_title: Option<String>,
|
||||
// Only the focused conversation may write the process-global terminal title.
|
||||
terminal_title_output_enabled: bool,
|
||||
// Last visible "action required" state observed by the terminal-title renderer.
|
||||
last_terminal_title_requires_action: bool,
|
||||
// Original terminal-title config captured when the setup UI opens.
|
||||
|
||||
@@ -46,6 +46,9 @@ impl ChatWidget {
|
||||
terminal_title_invalid_items_warned,
|
||||
session_telemetry,
|
||||
} = common;
|
||||
let terminal_title_output_enabled = app_event_tx
|
||||
.conversation_origin()
|
||||
.is_none_or(|origin| origin.pane == PaneSlot::Parent);
|
||||
let model = model.filter(|m| !m.trim().is_empty());
|
||||
let mut config = config;
|
||||
config.model = model.clone();
|
||||
@@ -233,6 +236,7 @@ impl ChatWidget {
|
||||
status_line_invalid_items_warned,
|
||||
terminal_title_invalid_items_warned,
|
||||
last_terminal_title: None,
|
||||
terminal_title_output_enabled,
|
||||
last_terminal_title_requires_action: false,
|
||||
terminal_title_setup_original_items: None,
|
||||
terminal_title_animation_origin: Instant::now(),
|
||||
|
||||
@@ -224,6 +224,9 @@ impl ChatWidget {
|
||||
/// When the `activity` item is present in an animated running state, this also
|
||||
/// schedules the next frame so the title animation keeps advancing.
|
||||
fn refresh_terminal_title_from_selections(&mut self, selections: &StatusSurfaceSelections) {
|
||||
if !self.terminal_title_output_enabled {
|
||||
return;
|
||||
}
|
||||
self.last_terminal_title_requires_action =
|
||||
self.terminal_title_shows_action_required_with_selections(selections);
|
||||
if selections.terminal_title_items.is_empty() {
|
||||
@@ -291,6 +294,20 @@ impl ChatWidget {
|
||||
self.refresh_terminal_title_from_selections(&selections);
|
||||
}
|
||||
|
||||
pub(crate) fn set_terminal_title_output_enabled(&mut self, enabled: bool) {
|
||||
if self.terminal_title_output_enabled == enabled {
|
||||
return;
|
||||
}
|
||||
self.terminal_title_output_enabled = enabled;
|
||||
if !enabled {
|
||||
if let Err(err) = self.clear_managed_terminal_title() {
|
||||
tracing::debug!(error = %err, "failed to clear terminal title while transferring ownership");
|
||||
}
|
||||
self.last_terminal_title = None;
|
||||
self.last_terminal_title_requires_action = false;
|
||||
}
|
||||
}
|
||||
|
||||
fn terminal_title_requires_action(&self) -> bool {
|
||||
self.bottom_pane.terminal_title_requires_action()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user