mirror of
https://github.com/openai/codex.git
synced 2026-11-14 21:40:39 +00:00
rollback to original UX: only basenames in user-visible placeholder
This commit is contained in:
@@ -507,33 +507,12 @@ impl ChatComposer {
|
||||
}
|
||||
|
||||
fn display_label_for_image_placeholder(path: &Path) -> String {
|
||||
let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else {
|
||||
return "image".to_string();
|
||||
};
|
||||
|
||||
// Avoid dumping very long absolute paths into the composer UI (common when attaching screenshots).
|
||||
if !path.is_absolute() {
|
||||
return path.display().to_string();
|
||||
}
|
||||
|
||||
// Count normal path segments (including the file name).
|
||||
let mut segments: Vec<String> = Vec::new();
|
||||
for c in path.components() {
|
||||
if let std::path::Component::Normal(seg) = c {
|
||||
segments.push(seg.to_string_lossy().into_owned());
|
||||
}
|
||||
}
|
||||
|
||||
// Keep short absolute paths readable.
|
||||
if segments.len() <= 2 {
|
||||
return path.display().to_string();
|
||||
}
|
||||
|
||||
let top = segments
|
||||
.first()
|
||||
.cloned()
|
||||
.unwrap_or_else(|| "...".to_string());
|
||||
format!("/{top}/.../{file_name}")
|
||||
// Keep the UI placeholder compact: show only the basename (pre-existing behavior).
|
||||
// The model can still receive the full path via model_placeholder expansion.
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.map(str::to_string)
|
||||
.unwrap_or_else(|| "image".to_string())
|
||||
}
|
||||
|
||||
fn model_placeholder_from_display_placeholder(
|
||||
@@ -3320,7 +3299,7 @@ mod tests {
|
||||
let (result, _) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
match result {
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[/tmp/image1.png 32x16] hi"),
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[image1.png 32x16] hi"),
|
||||
_ => panic!("expected Submitted"),
|
||||
}
|
||||
let imgs = composer.take_recent_submission_images();
|
||||
@@ -3343,7 +3322,7 @@ mod tests {
|
||||
let (result, _) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
match result {
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[/tmp/image2.png 10x5]"),
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[image2.png 10x5]"),
|
||||
_ => panic!("expected Submitted"),
|
||||
}
|
||||
let imgs = composer.take_recent_submission_images();
|
||||
@@ -3369,15 +3348,15 @@ mod tests {
|
||||
composer.attach_image(path, 10, 5, "PNG");
|
||||
|
||||
let text = composer.textarea.text().to_string();
|
||||
assert!(text.contains("[/tmp/image_dup.png 10x5]"));
|
||||
assert!(text.contains("[/tmp/image_dup.png 10x5 #2]"));
|
||||
assert!(text.contains("[image_dup.png 10x5]"));
|
||||
assert!(text.contains("[image_dup.png 10x5 #2]"));
|
||||
assert_eq!(
|
||||
composer.attached_images[0].display_placeholder,
|
||||
"[/tmp/image_dup.png 10x5]"
|
||||
"[image_dup.png 10x5]"
|
||||
);
|
||||
assert_eq!(
|
||||
composer.attached_images[1].display_placeholder,
|
||||
"[/tmp/image_dup.png 10x5 #2]"
|
||||
"[image_dup.png 10x5 #2]"
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3445,15 +3424,11 @@ mod tests {
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
|
||||
|
||||
assert_eq!(composer.attached_images.len(), 1);
|
||||
let expected_placeholder_prefix = format!(
|
||||
"[{} 10x5]",
|
||||
PathBuf::from("/tmp/image_multibyte.png").display()
|
||||
);
|
||||
assert!(
|
||||
composer
|
||||
.textarea
|
||||
.text()
|
||||
.starts_with(&expected_placeholder_prefix)
|
||||
.starts_with("[image_multibyte.png 10x5]")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3501,7 +3476,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
vec![AttachedImage {
|
||||
path: path2,
|
||||
display_placeholder: "[/tmp/image_dup2.png 10x5]".to_string(),
|
||||
display_placeholder: "[image_dup2.png 10x5]".to_string(),
|
||||
model_placeholder: "[/tmp/image_dup2.png 10x5]".to_string()
|
||||
}],
|
||||
composer.attached_images,
|
||||
|
||||
@@ -1093,7 +1093,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
|
||||
chat.bottom_pane.insert_str("draft message ");
|
||||
chat.bottom_pane
|
||||
.attach_image(PathBuf::from("/tmp/preview.png"), 24, 42, "png");
|
||||
let placeholder = "[/tmp/preview.png 24x42]";
|
||||
let placeholder = "[preview.png 24x42]";
|
||||
assert!(
|
||||
chat.bottom_pane.composer_text().ends_with(placeholder),
|
||||
"expected placeholder {placeholder:?} in composer text"
|
||||
@@ -1124,7 +1124,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
|
||||
async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
// Use a long absolute path so the display placeholder is shortened.
|
||||
// Use a long absolute path so we can verify display vs model text differ.
|
||||
let image_path = PathBuf::from("/var/tmp/some/dir/image.png");
|
||||
|
||||
chat.bottom_pane.insert_str("see ");
|
||||
@@ -1133,8 +1133,8 @@ async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
|
||||
let display_text = chat.bottom_pane.composer_text();
|
||||
assert!(
|
||||
display_text.contains("[/var/.../image.png 24x42]"),
|
||||
"expected shortened display placeholder, got {display_text:?}"
|
||||
display_text.contains("[image.png 24x42]"),
|
||||
"expected basename display placeholder, got {display_text:?}"
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
@@ -1147,8 +1147,8 @@ async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
);
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains("/var/.../image.png"),
|
||||
"expected transcript to contain shortened placeholder, got {rendered:?}"
|
||||
rendered.contains("image.png 24x42"),
|
||||
"expected transcript to contain basename placeholder, got {rendered:?}"
|
||||
);
|
||||
assert!(
|
||||
!rendered.contains("/var/tmp/some/dir/image.png"),
|
||||
@@ -1176,8 +1176,8 @@ async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
"expected model text to contain full path, got {model_text:?}"
|
||||
);
|
||||
assert!(
|
||||
!model_text.contains("/var/.../image.png"),
|
||||
"expected model text not to contain shortened placeholder, got {model_text:?}"
|
||||
!model_text.contains("[image.png 24x42]"),
|
||||
"expected model text not to contain display placeholder, got {model_text:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -193,33 +193,12 @@ impl ChatComposer {
|
||||
}
|
||||
|
||||
fn display_label_for_image_placeholder(path: &Path) -> String {
|
||||
let Some(file_name) = path.file_name().and_then(|name| name.to_str()) else {
|
||||
return "image".to_string();
|
||||
};
|
||||
|
||||
// Avoid dumping very long absolute paths into the composer UI (common when attaching screenshots).
|
||||
if !path.is_absolute() {
|
||||
return path.display().to_string();
|
||||
}
|
||||
|
||||
// Count normal path segments (including the file name).
|
||||
let mut segments: Vec<String> = Vec::new();
|
||||
for c in path.components() {
|
||||
if let std::path::Component::Normal(seg) = c {
|
||||
segments.push(seg.to_string_lossy().into_owned());
|
||||
}
|
||||
}
|
||||
|
||||
// Keep short absolute paths readable.
|
||||
if segments.len() <= 2 {
|
||||
return path.display().to_string();
|
||||
}
|
||||
|
||||
let top = segments
|
||||
.first()
|
||||
.cloned()
|
||||
.unwrap_or_else(|| "...".to_string());
|
||||
format!("/{top}/.../{file_name}")
|
||||
// Keep the UI placeholder compact: show only the basename (pre-existing behavior).
|
||||
// The model can still receive the full path via model_placeholder expansion.
|
||||
path.file_name()
|
||||
.and_then(|name| name.to_str())
|
||||
.map(str::to_string)
|
||||
.unwrap_or_else(|| "image".to_string())
|
||||
}
|
||||
|
||||
fn layout_areas(&self, area: Rect) -> [Rect; 3] {
|
||||
@@ -3210,7 +3189,7 @@ mod tests {
|
||||
let (result, _) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
match result {
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[/tmp/image1.png 32x16] hi"),
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[image1.png 32x16] hi"),
|
||||
_ => panic!("expected Submitted"),
|
||||
}
|
||||
let imgs = composer.take_recent_submission_images();
|
||||
@@ -3233,7 +3212,7 @@ mod tests {
|
||||
let (result, _) =
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
match result {
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[/tmp/image2.png 10x5]"),
|
||||
InputResult::Submitted(text) => assert_eq!(text, "[image2.png 10x5]"),
|
||||
_ => panic!("expected Submitted"),
|
||||
}
|
||||
let imgs = composer.take_recent_submission_images();
|
||||
@@ -3306,15 +3285,11 @@ mod tests {
|
||||
composer.handle_key_event(KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE));
|
||||
|
||||
assert_eq!(composer.attached_images.len(), 1);
|
||||
let expected_placeholder_prefix = format!(
|
||||
"[{} 10x5]",
|
||||
PathBuf::from("/tmp/image_multibyte.png").display()
|
||||
);
|
||||
assert!(
|
||||
composer
|
||||
.textarea
|
||||
.text()
|
||||
.starts_with(&expected_placeholder_prefix)
|
||||
.starts_with("[image_multibyte.png 10x5]")
|
||||
);
|
||||
}
|
||||
|
||||
@@ -3362,7 +3337,7 @@ mod tests {
|
||||
assert_eq!(
|
||||
vec![AttachedImage {
|
||||
path: path2,
|
||||
display_placeholder: "[/tmp/image_dup2.png 10x5]".to_string(),
|
||||
display_placeholder: "[image_dup2.png 10x5]".to_string(),
|
||||
model_placeholder: "[/tmp/image_dup2.png 10x5]".to_string()
|
||||
}],
|
||||
composer.attached_images,
|
||||
|
||||
@@ -1053,7 +1053,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
|
||||
chat.bottom_pane.insert_str("draft message ");
|
||||
chat.bottom_pane
|
||||
.attach_image(PathBuf::from("/tmp/preview.png"), 24, 42, "png");
|
||||
let placeholder = "[/tmp/preview.png 24x42]";
|
||||
let placeholder = "[preview.png 24x42]";
|
||||
assert!(
|
||||
chat.bottom_pane.composer_text().ends_with(placeholder),
|
||||
"expected placeholder {placeholder:?} in composer text"
|
||||
@@ -1084,7 +1084,7 @@ async fn ctrl_c_cleared_prompt_is_recoverable_via_history() {
|
||||
async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
// Use a long absolute path so the display placeholder is shortened.
|
||||
// Use a long absolute path so we can verify display vs model text differ.
|
||||
let image_path = PathBuf::from("/var/tmp/some/dir/image.png");
|
||||
|
||||
chat.bottom_pane.insert_str("see ");
|
||||
@@ -1093,8 +1093,8 @@ async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
|
||||
let display_text = chat.bottom_pane.composer_text();
|
||||
assert!(
|
||||
display_text.contains("[/var/.../image.png 24x42]"),
|
||||
"expected shortened display placeholder, got {display_text:?}"
|
||||
display_text.contains("[image.png 24x42]"),
|
||||
"expected basename display placeholder, got {display_text:?}"
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
@@ -1107,8 +1107,8 @@ async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
);
|
||||
let rendered = lines_to_single_string(&cells[0]);
|
||||
assert!(
|
||||
rendered.contains("/var/.../image.png"),
|
||||
"expected transcript to contain shortened placeholder, got {rendered:?}"
|
||||
rendered.contains("image.png 24x42"),
|
||||
"expected transcript to contain basename placeholder, got {rendered:?}"
|
||||
);
|
||||
assert!(
|
||||
!rendered.contains("/var/tmp/some/dir/image.png"),
|
||||
@@ -1136,8 +1136,8 @@ async fn submitting_image_shows_short_path_but_sends_full_path_to_model() {
|
||||
"expected model text to contain full path, got {model_text:?}"
|
||||
);
|
||||
assert!(
|
||||
!model_text.contains("/var/.../image.png"),
|
||||
"expected model text not to contain shortened placeholder, got {model_text:?}"
|
||||
!model_text.contains("[image.png 24x42]"),
|
||||
"expected model text not to contain display placeholder, got {model_text:?}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user