mirror of
https://github.com/openai/codex.git
synced 2026-11-11 21:10:39 +00:00
Reuse LocalImageAttachment throughout the TUI composer (#58260)
## What changed Replace the duplicate `AttachedImage` type with `LocalImageAttachment`. Return borrowed attachment slices and move attachment vectors directly when taking submissions or capturing history-search drafts, avoiding conversion and cloning. Use `local_image_paths()` in request-user-input and MCP elicitation drafts to copy only the paths they need. GitOrigin-RevId: b19451f633a479893fc97b8e2b51eb6b38838235
This commit is contained in:
@@ -1817,7 +1817,7 @@ impl ChatComposer {
|
||||
);
|
||||
self.set_pending_pastes(pending_pastes);
|
||||
if !self.config.image_paste_enabled {
|
||||
for image in self.attachments.local_images() {
|
||||
for image in &self.attachments.local_images {
|
||||
if let Some(element) = self.draft.textarea.text_elements().into_iter().find(|e| {
|
||||
e.placeholder(self.draft.textarea.text()) == Some(image.placeholder.as_str())
|
||||
}) {
|
||||
@@ -1837,7 +1837,6 @@ impl ChatComposer {
|
||||
self.current_text_elements()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn local_image_paths(&self) -> Vec<PathBuf> {
|
||||
self.attachments.local_image_paths()
|
||||
}
|
||||
@@ -1847,8 +1846,8 @@ impl ChatComposer {
|
||||
self.footer.status_line_value.as_ref()
|
||||
}
|
||||
|
||||
pub(crate) fn local_images(&self) -> Vec<LocalImageAttachment> {
|
||||
self.attachments.local_images()
|
||||
pub(crate) fn local_images(&self) -> &[LocalImageAttachment] {
|
||||
&self.attachments.local_images
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
@@ -1889,8 +1888,7 @@ impl ChatComposer {
|
||||
}
|
||||
|
||||
pub fn take_recent_submission_images_with_placeholders(&mut self) -> Vec<LocalImageAttachment> {
|
||||
self.attachments
|
||||
.take_recent_submission_images_with_placeholders()
|
||||
std::mem::take(&mut self.attachments.local_images)
|
||||
}
|
||||
|
||||
/// Flushes any due paste-burst state.
|
||||
@@ -5071,7 +5069,6 @@ mod mentions_layout_tests;
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::attachment_state::AttachedImage;
|
||||
use super::*;
|
||||
use crate::test_support::PathBufExt;
|
||||
use crate::test_support::test_path_buf;
|
||||
@@ -11687,7 +11684,7 @@ mod tests {
|
||||
"remaining placeholder relabeled to #1"
|
||||
);
|
||||
assert_eq!(
|
||||
vec![AttachedImage {
|
||||
vec![LocalImageAttachment {
|
||||
path: path2,
|
||||
placeholder: "[Image #1]".to_string()
|
||||
}],
|
||||
@@ -11750,7 +11747,7 @@ mod tests {
|
||||
format!("Test {placeholder1} test ")
|
||||
);
|
||||
assert_eq!(
|
||||
vec![AttachedImage {
|
||||
vec![LocalImageAttachment {
|
||||
path: path2,
|
||||
placeholder: placeholder1
|
||||
}],
|
||||
@@ -12362,10 +12359,13 @@ mod tests {
|
||||
|
||||
let placeholder = local_image_label_text(/*label_number*/ 1);
|
||||
composer.draft.textarea.insert_element(&placeholder);
|
||||
composer.attachments.local_images.push(AttachedImage {
|
||||
placeholder: placeholder.clone(),
|
||||
path: PathBuf::from("img.png"),
|
||||
});
|
||||
composer
|
||||
.attachments
|
||||
.local_images
|
||||
.push(LocalImageAttachment {
|
||||
placeholder: placeholder.clone(),
|
||||
path: PathBuf::from("img.png"),
|
||||
});
|
||||
composer
|
||||
.draft
|
||||
.pending_pastes
|
||||
@@ -12463,10 +12463,13 @@ mod tests {
|
||||
|
||||
let placeholder = local_image_label_text(/*label_number*/ 1);
|
||||
composer.draft.textarea.insert_element(&placeholder);
|
||||
composer.attachments.local_images.push(AttachedImage {
|
||||
placeholder: placeholder.clone(),
|
||||
path: PathBuf::from("img.png"),
|
||||
});
|
||||
composer
|
||||
.attachments
|
||||
.local_images
|
||||
.push(LocalImageAttachment {
|
||||
placeholder: placeholder.clone(),
|
||||
path: PathBuf::from("img.png"),
|
||||
});
|
||||
|
||||
composer.apply_external_edit("No images here".to_string());
|
||||
|
||||
@@ -12575,10 +12578,13 @@ mod tests {
|
||||
|
||||
let placeholder = local_image_label_text(/*label_number*/ 1);
|
||||
composer.draft.textarea.insert_element(&placeholder);
|
||||
composer.attachments.local_images.push(AttachedImage {
|
||||
placeholder: placeholder.clone(),
|
||||
path: PathBuf::from("img.png"),
|
||||
});
|
||||
composer
|
||||
.attachments
|
||||
.local_images
|
||||
.push(LocalImageAttachment {
|
||||
placeholder: placeholder.clone(),
|
||||
path: PathBuf::from("img.png"),
|
||||
});
|
||||
|
||||
composer.apply_external_edit(format!("{placeholder} extra {placeholder}"));
|
||||
|
||||
|
||||
@@ -17,15 +17,9 @@ 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) local_images: Vec<LocalImageAttachment>,
|
||||
pub(super) remote_image_urls: Vec<String>,
|
||||
pub(super) selected_remote_image_index: Option<usize>,
|
||||
}
|
||||
@@ -42,16 +36,6 @@ impl AttachmentState {
|
||||
.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;
|
||||
@@ -84,7 +68,7 @@ impl AttachmentState {
|
||||
local_image_paths
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
.map(|(index, path)| AttachedImage {
|
||||
.map(|(index, path)| LocalImageAttachment {
|
||||
placeholder: local_image_label_text(self.remote_image_urls.len() + index + 1),
|
||||
path,
|
||||
}),
|
||||
@@ -97,7 +81,8 @@ impl AttachmentState {
|
||||
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 });
|
||||
self.local_images
|
||||
.push(LocalImageAttachment { placeholder, path });
|
||||
}
|
||||
|
||||
pub(super) fn prune_local_images_for_submission(
|
||||
@@ -125,18 +110,6 @@ impl AttachmentState {
|
||||
.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()
|
||||
|
||||
@@ -23,7 +23,7 @@ impl ChatComposer {
|
||||
text: draft.text,
|
||||
cursor: draft.cursor,
|
||||
text_elements: draft.text_elements,
|
||||
local_images: attachments.local_images(),
|
||||
local_images: attachments.local_images,
|
||||
remote_image_urls: draft.remote_image_urls,
|
||||
mention_bindings: draft.mention_bindings,
|
||||
pending_pastes: draft.pending_pastes,
|
||||
|
||||
@@ -11,8 +11,8 @@ use std::sync::Arc;
|
||||
use std::sync::Weak;
|
||||
|
||||
use super::ChatComposer;
|
||||
use super::attachment_state::AttachedImage;
|
||||
use super::draft_state::ComposerMentionBinding;
|
||||
use crate::bottom_pane::LocalImageAttachment;
|
||||
use crate::bottom_pane::textarea::register::RegisterEvent;
|
||||
use codex_protocol::models::local_image_label_text;
|
||||
|
||||
@@ -142,7 +142,7 @@ pub(super) fn sync(composer: &mut ChatComposer) {
|
||||
composer
|
||||
.attachments
|
||||
.local_images
|
||||
.push(AttachedImage { placeholder, path });
|
||||
.push(LocalImageAttachment { placeholder, path });
|
||||
}
|
||||
RegisterPayload::Paste(pasted) => {
|
||||
let placeholder =
|
||||
|
||||
@@ -812,12 +812,7 @@ impl McpServerElicitationOverlay {
|
||||
ComposerDraft {
|
||||
text: self.composer.current_text(),
|
||||
text_elements: self.composer.text_elements(),
|
||||
local_image_paths: self
|
||||
.composer
|
||||
.local_images()
|
||||
.into_iter()
|
||||
.map(|img| img.path)
|
||||
.collect(),
|
||||
local_image_paths: self.composer.local_image_paths(),
|
||||
pending_pastes: self.composer.pending_pastes(),
|
||||
}
|
||||
}
|
||||
@@ -1267,12 +1262,7 @@ impl McpServerElicitationOverlay {
|
||||
}
|
||||
|
||||
fn apply_submission_to_draft(&mut self, text: String, text_elements: Vec<TextElement>) {
|
||||
let local_image_paths = self
|
||||
.composer
|
||||
.local_images()
|
||||
.into_iter()
|
||||
.map(|img| img.path)
|
||||
.collect::<Vec<_>>();
|
||||
let local_image_paths = self.composer.local_image_paths();
|
||||
if let Some(answer) = self.current_answer_mut() {
|
||||
answer.draft = ComposerDraft {
|
||||
text: text.clone(),
|
||||
|
||||
@@ -1237,7 +1237,7 @@ impl BottomPane {
|
||||
self.composer.text_elements()
|
||||
}
|
||||
|
||||
pub(crate) fn composer_local_images(&self) -> Vec<LocalImageAttachment> {
|
||||
pub(crate) fn composer_local_images(&self) -> &[LocalImageAttachment] {
|
||||
self.composer.local_images()
|
||||
}
|
||||
|
||||
|
||||
@@ -518,12 +518,7 @@ impl RequestUserInputOverlay {
|
||||
ComposerDraft {
|
||||
text: self.composer.current_text(),
|
||||
text_elements: self.composer.text_elements(),
|
||||
local_image_paths: self
|
||||
.composer
|
||||
.local_images()
|
||||
.into_iter()
|
||||
.map(|img| img.path)
|
||||
.collect(),
|
||||
local_image_paths: self.composer.local_image_paths(),
|
||||
pending_pastes: self.composer.pending_pastes(),
|
||||
}
|
||||
}
|
||||
@@ -1063,12 +1058,7 @@ impl RequestUserInputOverlay {
|
||||
}
|
||||
|
||||
fn apply_submission_to_draft(&mut self, text: String, text_elements: Vec<TextElement>) {
|
||||
let local_image_paths = self
|
||||
.composer
|
||||
.local_images()
|
||||
.into_iter()
|
||||
.map(|img| img.path)
|
||||
.collect::<Vec<_>>();
|
||||
let local_image_paths = self.composer.local_image_paths();
|
||||
if let Some(answer) = self.current_answer_mut() {
|
||||
answer.draft = ComposerDraft {
|
||||
text: text.clone(),
|
||||
|
||||
@@ -701,7 +701,7 @@ impl ChatWidget {
|
||||
args,
|
||||
text_elements,
|
||||
pending_pastes: self.bottom_pane.composer_pending_pastes(),
|
||||
local_images: self.bottom_pane.composer_local_images(),
|
||||
local_images: self.bottom_pane.composer_local_images().to_vec(),
|
||||
remote_image_urls: self.bottom_pane.remote_image_urls(),
|
||||
mention_bindings: Vec::new(),
|
||||
source: SlashCommandDispatchSource::Live,
|
||||
|
||||
Reference in New Issue
Block a user