Don't use PTY in the display-only terminal (#39510)

This only affects `codex-acp` for now.

Not using the PTY in display-only terminals means they don't display the
login prompt (or spurious `%`s) at the end of terminal output
renderings.

Release Notes:

- N/A
This commit is contained in:
Richard Feldman 2025-10-04 00:49:33 -04:00 committed by GitHub
parent 6b980ecad3
commit 978951b79a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 127 additions and 51 deletions

View file

@ -2331,12 +2331,10 @@ mod tests {
// Create a display-only terminal and then send Created
let lower = cx.new(|cx| {
let builder = ::terminal::TerminalBuilder::new_display_only(
None,
::terminal::terminal_settings::CursorShape::default(),
::terminal::terminal_settings::AlternateScroll::On,
None,
0,
cx,
)
.unwrap();
builder.subscribe(cx)
@ -2410,12 +2408,10 @@ mod tests {
// Now create a display-only lower-level terminal and send Created
let lower = cx.new(|cx| {
let builder = ::terminal::TerminalBuilder::new_display_only(
None,
::terminal::terminal_settings::CursorShape::default(),
::terminal::terminal_settings::AlternateScroll::On,
None,
0,
cx,
)
.unwrap();
builder.subscribe(cx)

View file

@ -717,12 +717,10 @@ impl acp::Client for ClientDelegate {
// Create a minimal display-only lower-level terminal and register it.
let _ = session.thread.update(&mut self.cx.clone(), |thread, cx| {
let builder = TerminalBuilder::new_display_only(
None,
CursorShape::default(),
AlternateScroll::On,
None,
0,
cx,
)?;
let lower = cx.new(|cx| builder.subscribe(cx));
thread.on_terminal_provider_event(

View file

@ -2716,7 +2716,7 @@ impl AcpThreadView {
let working_dir = working_dir
.as_ref()
.map(|path| format!("{}", path.display()))
.map(|path| path.display().to_string())
.unwrap_or_else(|| "current directory".to_string());
let is_expanded = self.expanded_tool_calls.contains(&tool_call.id);

View file

@ -477,7 +477,7 @@ impl ToolCard for TerminalToolCard {
.as_ref()
.cloned()
.or_else(|| env::current_dir().ok())
.map(|path| format!("{}", path.display()))
.map(|path| path.display().to_string())
.unwrap_or_else(|| "current directory".to_string());
let header = h_flex()

View file

@ -1232,7 +1232,6 @@ impl RunningState {
terminal.read_with(cx, |terminal, _| {
terminal
.pty_info
.pid()
.map(|pid| pid.as_u32())
.context("Terminal was spawned but PID was not available")

View file

@ -41,7 +41,7 @@ use mappings::mouse::{
use collections::{HashMap, VecDeque};
use futures::StreamExt;
use pty_info::PtyProcessInfo;
use pty_info::{ProcessIdGetter, PtyProcessInfo};
use serde::{Deserialize, Serialize};
use settings::Settings;
use smol::channel::{Receiver, Sender};
@ -340,27 +340,74 @@ pub struct TerminalBuilder {
impl TerminalBuilder {
pub fn new_display_only(
working_directory: Option<PathBuf>,
cursor_shape: CursorShape,
alternate_scroll: AlternateScroll,
max_scroll_history_lines: Option<usize>,
window_id: u64,
cx: &App,
) -> Result<TerminalBuilder> {
Self::new(
working_directory,
None,
Shell::System,
HashMap::default(),
cursor_shape,
alternate_scroll,
max_scroll_history_lines,
false,
window_id,
None,
cx,
Vec::new(),
)
// Create a display-only terminal (no actual PTY).
let default_cursor_style = AlacCursorStyle::from(cursor_shape);
let scrolling_history = max_scroll_history_lines
.unwrap_or(DEFAULT_SCROLL_HISTORY_LINES)
.min(MAX_SCROLL_HISTORY_LINES);
let config = Config {
scrolling_history,
default_cursor_style,
..Config::default()
};
let (events_tx, events_rx) = unbounded();
let mut term = Term::new(
config.clone(),
&TerminalBounds::default(),
ZedListener(events_tx),
);
if let AlternateScroll::Off = alternate_scroll {
term.unset_private_mode(PrivateMode::Named(NamedPrivateMode::AlternateScroll));
}
let term = Arc::new(FairMutex::new(term));
let terminal = Terminal {
task: None,
terminal_type: TerminalType::DisplayOnly,
completion_tx: None,
term,
term_config: config,
title_override: None,
events: VecDeque::with_capacity(10),
last_content: Default::default(),
last_mouse: None,
matches: Vec::new(),
selection_head: None,
breadcrumb_text: String::new(),
scroll_px: px(0.),
next_link_id: 0,
selection_phase: SelectionPhase::Ended,
hyperlink_regex_searches: RegexSearches::new(),
vi_mode_enabled: false,
is_ssh_terminal: false,
last_mouse_move_time: Instant::now(),
last_hyperlink_search_position: None,
#[cfg(windows)]
shell_program: None,
activation_script: Vec::new(),
template: CopyTemplate {
shell: Shell::System,
env: HashMap::default(),
cursor_shape,
alternate_scroll,
max_scroll_history_lines,
window_id,
},
child_exited: None,
};
Ok(TerminalBuilder {
terminal,
events_rx,
})
}
pub fn new(
working_directory: Option<PathBuf>,
@ -533,7 +580,10 @@ impl TerminalBuilder {
let mut terminal = Terminal {
task,
pty_tx: Notifier(pty_tx),
terminal_type: TerminalType::Pty {
pty_tx: Notifier(pty_tx),
info: pty_info,
},
completion_tx,
term,
term_config: config,
@ -543,12 +593,10 @@ impl TerminalBuilder {
last_mouse: None,
matches: Vec::new(),
selection_head: None,
pty_info,
breadcrumb_text: String::new(),
scroll_px: px(0.),
next_link_id: 0,
selection_phase: SelectionPhase::Ended,
// hovered_word: false,
hyperlink_regex_searches: RegexSearches::new(),
vi_mode_enabled: false,
is_ssh_terminal,
@ -733,8 +781,16 @@ pub enum SelectionPhase {
Ended,
}
enum TerminalType {
Pty {
pty_tx: Notifier,
info: PtyProcessInfo,
},
DisplayOnly,
}
pub struct Terminal {
pty_tx: Notifier,
terminal_type: TerminalType,
completion_tx: Option<Sender<Option<ExitStatus>>>,
term: Arc<FairMutex<Term<ZedListener>>>,
term_config: Config,
@ -745,7 +801,6 @@ pub struct Terminal {
pub last_content: TerminalContent,
pub selection_head: Option<AlacPoint>,
pub breadcrumb_text: String,
pub pty_info: PtyProcessInfo,
title_override: Option<SharedString>,
scroll_px: Pixels,
next_link_id: usize,
@ -862,8 +917,10 @@ impl Terminal {
AlacTermEvent::Wakeup => {
cx.emit(Event::Wakeup);
if self.pty_info.has_changed() {
cx.emit(Event::TitleChanged);
if let TerminalType::Pty { info, .. } = &mut self.terminal_type {
if info.has_changed() {
cx.emit(Event::TitleChanged);
}
}
}
AlacTermEvent::ColorRequest(index, format) => {
@ -905,7 +962,9 @@ impl Terminal {
self.last_content.terminal_bounds = new_bounds;
self.pty_tx.0.send(Msg::Resize(new_bounds.into())).ok();
if let TerminalType::Pty { pty_tx, .. } = &self.terminal_type {
pty_tx.0.send(Msg::Resize(new_bounds.into())).ok();
}
term.resize(new_bounds);
}
@ -1288,9 +1347,12 @@ impl Terminal {
}
}
///Write the Input payload to the tty.
/// Write the Input payload to the PTY, if applicable.
/// (This is a no-op for display-only terminals.)
fn write_to_pty(&self, input: impl Into<Cow<'static, [u8]>>) {
self.pty_tx.notify(input.into());
if let TerminalType::Pty { pty_tx, .. } = &self.terminal_type {
pty_tx.notify(input.into());
}
}
pub fn input(&mut self, input: impl Into<Cow<'static, [u8]>>) {
@ -1594,7 +1656,7 @@ impl Terminal {
&& let Some(bytes) =
mouse_moved_report(point, e.pressed_button, e.modifiers, self.last_content.mode)
{
self.pty_tx.notify(bytes);
self.write_to_pty(bytes);
}
} else if e.modifiers.secondary() {
self.word_from_position(e.position);
@ -1701,7 +1763,7 @@ impl Terminal {
if let Some(bytes) =
mouse_button_report(point, e.button, e.modifiers, true, self.last_content.mode)
{
self.pty_tx.notify(bytes);
self.write_to_pty(bytes);
}
} else {
match e.button {
@ -1760,7 +1822,7 @@ impl Terminal {
if let Some(bytes) =
mouse_button_report(point, e.button, e.modifiers, false, self.last_content.mode)
{
self.pty_tx.notify(bytes);
self.write_to_pty(bytes);
}
} else {
if e.button == MouseButton::Left && setting.copy_on_select {
@ -1799,7 +1861,7 @@ impl Terminal {
if let Some(scrolls) = scroll_report(point, scroll_lines, e, self.last_content.mode)
{
for scroll in scrolls {
self.pty_tx.notify(scroll);
self.write_to_pty(scroll);
}
};
} else if self
@ -1808,7 +1870,7 @@ impl Terminal {
.contains(TermMode::ALT_SCREEN | TermMode::ALTERNATE_SCROLL)
&& !e.shift
{
self.pty_tx.notify(alt_scroll(scroll_lines))
self.write_to_pty(alt_scroll(scroll_lines));
} else if scroll_lines != 0 {
let scroll = AlacScroll::Delta(scroll_lines);
@ -1879,10 +1941,12 @@ impl Terminal {
/// This does *not* return the working directory of the shell that runs on the
/// remote host, in case Zed is connected to a remote host.
fn client_side_working_directory(&self) -> Option<PathBuf> {
self.pty_info
.current
.as_ref()
.map(|process| process.cwd.clone())
match &self.terminal_type {
TerminalType::Pty { info, .. } => {
info.current.as_ref().map(|process| process.cwd.clone())
}
TerminalType::DisplayOnly => None,
}
}
pub fn title(&self, truncate: bool) -> String {
@ -1899,8 +1963,8 @@ impl Terminal {
.title_override
.as_ref()
.map(|title_override| title_override.to_string())
.unwrap_or_else(|| {
self.pty_info
.unwrap_or_else(|| match &self.terminal_type {
TerminalType::Pty { info, .. } => info
.current
.as_ref()
.map(|fpi| {
@ -1930,7 +1994,8 @@ impl Terminal {
};
format!("{process_file}{process_name}")
})
.unwrap_or_else(|| "Terminal".to_string())
.unwrap_or_else(|| "Terminal".to_string()),
TerminalType::DisplayOnly => "Terminal".to_string(),
}),
}
}
@ -1939,7 +2004,23 @@ impl Terminal {
if let Some(task) = self.task()
&& task.status == TaskStatus::Running
{
self.pty_info.kill_current_process();
if let TerminalType::Pty { info, .. } = &mut self.terminal_type {
info.kill_current_process();
}
}
}
pub fn pid(&self) -> Option<sysinfo::Pid> {
match &self.terminal_type {
TerminalType::Pty { info, .. } => info.pid(),
TerminalType::DisplayOnly => None,
}
}
pub fn pid_getter(&self) -> Option<&ProcessIdGetter> {
match &self.terminal_type {
TerminalType::Pty { info, .. } => Some(info.pid_getter()),
TerminalType::DisplayOnly => None,
}
}
@ -2134,7 +2215,9 @@ unsafe fn append_text_to_term(term: &mut Term<ZedListener>, text_lines: &[&str])
impl Drop for Terminal {
fn drop(&mut self) {
self.pty_tx.0.send(Msg::Shutdown).ok();
if let TerminalType::Pty { pty_tx, .. } = &self.terminal_type {
pty_tx.0.send(Msg::Shutdown).ok();
}
}
}

View file

@ -1142,7 +1142,7 @@ impl Item for TerminalView {
fn tab_tooltip_content(&self, cx: &App) -> Option<TabTooltipContent> {
let terminal = self.terminal().read(cx);
let title = terminal.title(false);
let pid = terminal.pty_info.pid_getter().fallback_pid();
let pid = terminal.pid_getter()?.fallback_pid();
Some(TabTooltipContent::Custom(Box::new(move |_window, cx| {
cx.new(|_| TerminalTooltip::new(title.clone(), pid)).into()