tui: split composer attachment and popup state (#22581)

## Why

`ChatComposer` currently owns text editing alongside attachment
bookkeeping and popup lifecycle state, while `BottomPane` still triggers
a couple of popup resyncs after composer methods that already do that
work internally. That blurs the ownership boundary and makes the
composer harder to simplify safely.

This PR is part 1 of a two-part cleanup. It peels off the composer state
that can move cleanly on its own, so the follow-up can tackle the
heavier draft/editing boundary without mixing every concern into one
diff.

## What changed

- Move local and remote image bookkeeping, placeholder relabeling, and
remote-image keyboard selection into `AttachmentState`.
- Move active-popup and popup-dismissal/query bookkeeping into
`PopupState`.
- Update composer and history-search paths to use those state owners
directly.
- Remove redundant `BottomPane` popup synchronization after paste
handling and `insert_str`.

## Part 2

The follow-up PR will finish the cleanup around the remaining composer
boundary: split out the draft/editing-oriented state and footer/status
presentation concerns that still live in `ChatComposer`, then revisit
the leftover `BottomPane` pass-throughs once those ownership lines are
explicit. The goal is for `ChatComposer` to coordinate a few focused
collaborators instead of continuing to be the landing zone for every
input-path concern.

## Verification

Did manual smoke tests.
This commit is contained in:
Eric Traut
2026-05-14 09:04:27 -07:00
committed by GitHub
Unverified
parent e79e1b42b9
commit a5040d0b39
5 changed files with 501 additions and 388 deletions
File diff suppressed because it is too large Load Diff
@@ -0,0 +1,251 @@
//! Attachment bookkeeping for the chat composer, including local image placeholders,
//! remote-image rows, and keyboard selection over those rows.
use std::collections::HashSet;
use std::path::PathBuf;
use crossterm::event::KeyCode;
use crossterm::event::KeyEvent;
use crossterm::event::KeyEventKind;
use crossterm::event::KeyModifiers;
use ratatui::style::Stylize;
use ratatui::text::Line;
use super::InputResult;
use crate::bottom_pane::LocalImageAttachment;
use crate::bottom_pane::textarea::TextArea;
use codex_protocol::models::local_image_label_text;
use codex_protocol::user_input::TextElement;
#[derive(Clone, Debug, PartialEq)]
pub(super) struct AttachedImage {
pub(super) placeholder: String,
pub(super) path: PathBuf,
}
#[derive(Debug, Default)]
pub(super) struct AttachmentState {
pub(super) local_images: Vec<AttachedImage>,
pub(super) remote_image_urls: Vec<String>,
pub(super) selected_remote_image_index: Option<usize>,
}
impl AttachmentState {
pub(super) fn is_empty(&self) -> bool {
self.local_images.is_empty() && self.remote_image_urls.is_empty()
}
pub(super) fn local_image_paths(&self) -> Vec<PathBuf> {
self.local_images
.iter()
.map(|image| image.path.clone())
.collect()
}
pub(super) fn local_images(&self) -> Vec<LocalImageAttachment> {
self.local_images
.iter()
.map(|image| LocalImageAttachment {
placeholder: image.placeholder.clone(),
path: image.path.clone(),
})
.collect()
}
pub(super) fn set_remote_image_urls(&mut self, urls: Vec<String>, textarea: &mut TextArea) {
self.remote_image_urls = urls;
self.selected_remote_image_index = None;
self.relabel_local_images(textarea);
}
pub(super) fn remote_image_urls(&self) -> Vec<String> {
self.remote_image_urls.clone()
}
pub(super) fn take_remote_image_urls(&mut self, textarea: &mut TextArea) -> Vec<String> {
let urls = std::mem::take(&mut self.remote_image_urls);
self.selected_remote_image_index = None;
self.relabel_local_images(textarea);
urls
}
pub(super) fn clear_remote_image_urls(&mut self) {
self.remote_image_urls.clear();
self.selected_remote_image_index = None;
}
pub(super) fn reset_local_images(
&mut self,
local_image_paths: Vec<PathBuf>,
textarea: &mut TextArea,
) {
self.local_images.clear();
self.local_images.extend(
local_image_paths
.into_iter()
.enumerate()
.map(|(index, path)| AttachedImage {
placeholder: local_image_label_text(self.remote_image_urls.len() + index + 1),
path,
}),
);
self.selected_remote_image_index = None;
self.relabel_local_images(textarea);
}
pub(super) fn attach_image(&mut self, textarea: &mut TextArea, path: PathBuf) {
let image_number = self.remote_image_urls.len() + self.local_images.len() + 1;
let placeholder = local_image_label_text(image_number);
textarea.insert_element(&placeholder);
self.local_images.push(AttachedImage { placeholder, path });
}
pub(super) fn prune_local_images_for_submission(
&mut self,
text: &str,
text_elements: &[TextElement],
) {
if self.local_images.is_empty() {
return;
}
let image_placeholders: HashSet<&str> = text_elements
.iter()
.filter_map(|element| element.placeholder(text))
.collect();
self.local_images
.retain(|image| image_placeholders.contains(image.placeholder.as_str()));
}
#[cfg(test)]
pub(super) fn take_recent_submission_images(&mut self) -> Vec<PathBuf> {
std::mem::take(&mut self.local_images)
.into_iter()
.map(|image| image.path)
.collect()
}
pub(super) fn take_recent_submission_images_with_placeholders(
&mut self,
) -> Vec<LocalImageAttachment> {
std::mem::take(&mut self.local_images)
.into_iter()
.map(|image| LocalImageAttachment {
placeholder: image.placeholder,
path: image.path,
})
.collect()
}
pub(super) fn remote_image_lines(&self) -> Vec<Line<'static>> {
self.remote_image_urls
.iter()
.enumerate()
.map(|(index, _)| {
let label = local_image_label_text(index + 1);
if self.selected_remote_image_index == Some(index) {
label.cyan().reversed().into()
} else {
label.cyan().into()
}
})
.collect()
}
pub(super) fn clear_remote_image_selection(&mut self) {
self.selected_remote_image_index = None;
}
pub(super) fn handle_remote_image_selection_key(
&mut self,
key_event: &KeyEvent,
textarea: &mut TextArea,
) -> Option<(InputResult, bool)> {
if self.remote_image_urls.is_empty()
|| key_event.modifiers != KeyModifiers::NONE
|| key_event.kind != KeyEventKind::Press
{
return None;
}
match key_event.code {
KeyCode::Up => {
if let Some(selected) = self.selected_remote_image_index {
self.selected_remote_image_index = Some(selected.saturating_sub(1));
Some((InputResult::None, true))
} else if textarea.cursor() == 0 {
self.selected_remote_image_index = Some(self.remote_image_urls.len() - 1);
Some((InputResult::None, true))
} else {
None
}
}
KeyCode::Down => {
if let Some(selected) = self.selected_remote_image_index {
if selected + 1 < self.remote_image_urls.len() {
self.selected_remote_image_index = Some(selected + 1);
} else {
self.clear_remote_image_selection();
}
Some((InputResult::None, true))
} else {
None
}
}
KeyCode::Delete | KeyCode::Backspace => {
if let Some(selected) = self.selected_remote_image_index {
self.remove_selected_remote_image(selected, textarea);
Some((InputResult::None, true))
} else {
None
}
}
_ => None,
}
}
pub(super) fn remove_deleted_local_placeholders(
&mut self,
removed_payloads: &[String],
textarea: &mut TextArea,
) -> bool {
let previous_len = self.local_images.len();
self.local_images.retain(|image| {
!removed_payloads
.iter()
.any(|payload| payload == &image.placeholder)
});
let removed_any = self.local_images.len() != previous_len;
if removed_any {
self.relabel_local_images(textarea);
}
removed_any
}
pub(super) fn relabel_local_images(&mut self, textarea: &mut TextArea) {
for (index, image) in self.local_images.iter_mut().enumerate() {
let expected = local_image_label_text(self.remote_image_urls.len() + index + 1);
if image.placeholder == expected {
continue;
}
let current = std::mem::replace(&mut image.placeholder, expected.clone());
let _renamed = textarea.replace_element_payload(&current, &expected);
}
}
fn remove_selected_remote_image(&mut self, selected_index: usize, textarea: &mut TextArea) {
if selected_index >= self.remote_image_urls.len() {
self.clear_remote_image_selection();
return;
}
self.remote_image_urls.remove(selected_index);
self.selected_remote_image_index = if self.remote_image_urls.is_empty() {
None
} else {
Some(selected_index.min(self.remote_image_urls.len() - 1))
};
self.relabel_local_images(textarea);
}
}
@@ -107,13 +107,13 @@ impl ChatComposer {
}
self.paste_burst.clear_window_after_non_char();
if self.current_file_query.is_some() {
if self.popups.current_file_query.is_some() {
self.app_event_tx
.send(AppEvent::StartFileSearch(String::new()));
self.current_file_query = None;
self.popups.current_file_query = None;
}
self.active_popup = ActivePopup::None;
self.selected_remote_image_index = None;
self.popups.active = ActivePopup::None;
self.attachments.clear_remote_image_selection();
self.history_search = Some(HistorySearchSession {
original_draft: self.snapshot_draft(),
query: String::new(),
@@ -0,0 +1,32 @@
//! Popup lifecycle state for the chat composer.
//! Tracks the single active popup plus dismissal/query state used to synchronize it.
use crate::bottom_pane::command_popup::CommandPopup;
use crate::bottom_pane::file_search_popup::FileSearchPopup;
use crate::bottom_pane::mentions_v2::MentionV2Popup;
use crate::bottom_pane::skill_popup::SkillPopup;
#[derive(Default)]
pub(super) struct PopupState {
pub(super) active: ActivePopup,
pub(super) dismissed_file_token: Option<String>,
pub(super) current_file_query: Option<String>,
pub(super) dismissed_mention_token: Option<String>,
}
impl PopupState {
pub(super) fn active(&self) -> bool {
!matches!(self.active, ActivePopup::None)
}
}
/// Popup state - at most one can be visible at any time.
#[derive(Default)]
pub(super) enum ActivePopup {
#[default]
None,
Command(CommandPopup),
File(FileSearchPopup),
Skill(SkillPopup),
MentionV2(MentionV2Popup),
}
-2
View File
@@ -711,7 +711,6 @@ impl BottomPane {
if has_pasted_text {
self.record_composer_activity_at(Instant::now());
}
self.composer.sync_popups();
if needs_redraw {
self.request_redraw();
}
@@ -720,7 +719,6 @@ impl BottomPane {
pub(crate) fn insert_str(&mut self, text: &str) {
self.composer.insert_str(text);
self.composer.sync_popups();
self.request_redraw();
}