Skip to content

Commit 978951b

Browse files
authored
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
1 parent 6b980ec commit 978951b

File tree

7 files changed

+127
-51
lines changed

7 files changed

+127
-51
lines changed

crates/acp_thread/src/acp_thread.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2331,12 +2331,10 @@ mod tests {
23312331
// Create a display-only terminal and then send Created
23322332
let lower = cx.new(|cx| {
23332333
let builder = ::terminal::TerminalBuilder::new_display_only(
2334-
None,
23352334
::terminal::terminal_settings::CursorShape::default(),
23362335
::terminal::terminal_settings::AlternateScroll::On,
23372336
None,
23382337
0,
2339-
cx,
23402338
)
23412339
.unwrap();
23422340
builder.subscribe(cx)
@@ -2410,12 +2408,10 @@ mod tests {
24102408
// Now create a display-only lower-level terminal and send Created
24112409
let lower = cx.new(|cx| {
24122410
let builder = ::terminal::TerminalBuilder::new_display_only(
2413-
None,
24142411
::terminal::terminal_settings::CursorShape::default(),
24152412
::terminal::terminal_settings::AlternateScroll::On,
24162413
None,
24172414
0,
2418-
cx,
24192415
)
24202416
.unwrap();
24212417
builder.subscribe(cx)

crates/agent_servers/src/acp.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -717,12 +717,10 @@ impl acp::Client for ClientDelegate {
717717
// Create a minimal display-only lower-level terminal and register it.
718718
let _ = session.thread.update(&mut self.cx.clone(), |thread, cx| {
719719
let builder = TerminalBuilder::new_display_only(
720-
None,
721720
CursorShape::default(),
722721
AlternateScroll::On,
723722
None,
724723
0,
725-
cx,
726724
)?;
727725
let lower = cx.new(|cx| builder.subscribe(cx));
728726
thread.on_terminal_provider_event(

crates/agent_ui/src/acp/thread_view.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2716,7 +2716,7 @@ impl AcpThreadView {
27162716

27172717
let working_dir = working_dir
27182718
.as_ref()
2719-
.map(|path| format!("{}", path.display()))
2719+
.map(|path| path.display().to_string())
27202720
.unwrap_or_else(|| "current directory".to_string());
27212721

27222722
let is_expanded = self.expanded_tool_calls.contains(&tool_call.id);

crates/assistant_tools/src/terminal_tool.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ impl ToolCard for TerminalToolCard {
477477
.as_ref()
478478
.cloned()
479479
.or_else(|| env::current_dir().ok())
480-
.map(|path| format!("{}", path.display()))
480+
.map(|path| path.display().to_string())
481481
.unwrap_or_else(|| "current directory".to_string());
482482

483483
let header = h_flex()

crates/debugger_ui/src/session/running.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1232,7 +1232,6 @@ impl RunningState {
12321232

12331233
terminal.read_with(cx, |terminal, _| {
12341234
terminal
1235-
.pty_info
12361235
.pid()
12371236
.map(|pid| pid.as_u32())
12381237
.context("Terminal was spawned but PID was not available")

crates/terminal/src/terminal.rs

Lines changed: 124 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use mappings::mouse::{
4141

4242
use collections::{HashMap, VecDeque};
4343
use futures::StreamExt;
44-
use pty_info::PtyProcessInfo;
44+
use pty_info::{ProcessIdGetter, PtyProcessInfo};
4545
use serde::{Deserialize, Serialize};
4646
use settings::Settings;
4747
use smol::channel::{Receiver, Sender};
@@ -340,27 +340,74 @@ pub struct TerminalBuilder {
340340

341341
impl TerminalBuilder {
342342
pub fn new_display_only(
343-
working_directory: Option<PathBuf>,
344343
cursor_shape: CursorShape,
345344
alternate_scroll: AlternateScroll,
346345
max_scroll_history_lines: Option<usize>,
347346
window_id: u64,
348-
cx: &App,
349347
) -> Result<TerminalBuilder> {
350-
Self::new(
351-
working_directory,
352-
None,
353-
Shell::System,
354-
HashMap::default(),
355-
cursor_shape,
356-
alternate_scroll,
357-
max_scroll_history_lines,
358-
false,
359-
window_id,
360-
None,
361-
cx,
362-
Vec::new(),
363-
)
348+
// Create a display-only terminal (no actual PTY).
349+
let default_cursor_style = AlacCursorStyle::from(cursor_shape);
350+
let scrolling_history = max_scroll_history_lines
351+
.unwrap_or(DEFAULT_SCROLL_HISTORY_LINES)
352+
.min(MAX_SCROLL_HISTORY_LINES);
353+
let config = Config {
354+
scrolling_history,
355+
default_cursor_style,
356+
..Config::default()
357+
};
358+
359+
let (events_tx, events_rx) = unbounded();
360+
let mut term = Term::new(
361+
config.clone(),
362+
&TerminalBounds::default(),
363+
ZedListener(events_tx),
364+
);
365+
366+
if let AlternateScroll::Off = alternate_scroll {
367+
term.unset_private_mode(PrivateMode::Named(NamedPrivateMode::AlternateScroll));
368+
}
369+
370+
let term = Arc::new(FairMutex::new(term));
371+
372+
let terminal = Terminal {
373+
task: None,
374+
terminal_type: TerminalType::DisplayOnly,
375+
completion_tx: None,
376+
term,
377+
term_config: config,
378+
title_override: None,
379+
events: VecDeque::with_capacity(10),
380+
last_content: Default::default(),
381+
last_mouse: None,
382+
matches: Vec::new(),
383+
selection_head: None,
384+
breadcrumb_text: String::new(),
385+
scroll_px: px(0.),
386+
next_link_id: 0,
387+
selection_phase: SelectionPhase::Ended,
388+
hyperlink_regex_searches: RegexSearches::new(),
389+
vi_mode_enabled: false,
390+
is_ssh_terminal: false,
391+
last_mouse_move_time: Instant::now(),
392+
last_hyperlink_search_position: None,
393+
#[cfg(windows)]
394+
shell_program: None,
395+
activation_script: Vec::new(),
396+
template: CopyTemplate {
397+
shell: Shell::System,
398+
env: HashMap::default(),
399+
cursor_shape,
400+
alternate_scroll,
401+
max_scroll_history_lines,
402+
window_id,
403+
},
404+
child_exited: None,
405+
};
406+
407+
Ok(TerminalBuilder {
408+
terminal,
409+
events_rx,
410+
})
364411
}
365412
pub fn new(
366413
working_directory: Option<PathBuf>,
@@ -533,7 +580,10 @@ impl TerminalBuilder {
533580

534581
let mut terminal = Terminal {
535582
task,
536-
pty_tx: Notifier(pty_tx),
583+
terminal_type: TerminalType::Pty {
584+
pty_tx: Notifier(pty_tx),
585+
info: pty_info,
586+
},
537587
completion_tx,
538588
term,
539589
term_config: config,
@@ -543,12 +593,10 @@ impl TerminalBuilder {
543593
last_mouse: None,
544594
matches: Vec::new(),
545595
selection_head: None,
546-
pty_info,
547596
breadcrumb_text: String::new(),
548597
scroll_px: px(0.),
549598
next_link_id: 0,
550599
selection_phase: SelectionPhase::Ended,
551-
// hovered_word: false,
552600
hyperlink_regex_searches: RegexSearches::new(),
553601
vi_mode_enabled: false,
554602
is_ssh_terminal,
@@ -733,8 +781,16 @@ pub enum SelectionPhase {
733781
Ended,
734782
}
735783

784+
enum TerminalType {
785+
Pty {
786+
pty_tx: Notifier,
787+
info: PtyProcessInfo,
788+
},
789+
DisplayOnly,
790+
}
791+
736792
pub struct Terminal {
737-
pty_tx: Notifier,
793+
terminal_type: TerminalType,
738794
completion_tx: Option<Sender<Option<ExitStatus>>>,
739795
term: Arc<FairMutex<Term<ZedListener>>>,
740796
term_config: Config,
@@ -745,7 +801,6 @@ pub struct Terminal {
745801
pub last_content: TerminalContent,
746802
pub selection_head: Option<AlacPoint>,
747803
pub breadcrumb_text: String,
748-
pub pty_info: PtyProcessInfo,
749804
title_override: Option<SharedString>,
750805
scroll_px: Pixels,
751806
next_link_id: usize,
@@ -862,8 +917,10 @@ impl Terminal {
862917
AlacTermEvent::Wakeup => {
863918
cx.emit(Event::Wakeup);
864919

865-
if self.pty_info.has_changed() {
866-
cx.emit(Event::TitleChanged);
920+
if let TerminalType::Pty { info, .. } = &mut self.terminal_type {
921+
if info.has_changed() {
922+
cx.emit(Event::TitleChanged);
923+
}
867924
}
868925
}
869926
AlacTermEvent::ColorRequest(index, format) => {
@@ -905,7 +962,9 @@ impl Terminal {
905962

906963
self.last_content.terminal_bounds = new_bounds;
907964

908-
self.pty_tx.0.send(Msg::Resize(new_bounds.into())).ok();
965+
if let TerminalType::Pty { pty_tx, .. } = &self.terminal_type {
966+
pty_tx.0.send(Msg::Resize(new_bounds.into())).ok();
967+
}
909968

910969
term.resize(new_bounds);
911970
}
@@ -1288,9 +1347,12 @@ impl Terminal {
12881347
}
12891348
}
12901349

1291-
///Write the Input payload to the tty.
1350+
/// Write the Input payload to the PTY, if applicable.
1351+
/// (This is a no-op for display-only terminals.)
12921352
fn write_to_pty(&self, input: impl Into<Cow<'static, [u8]>>) {
1293-
self.pty_tx.notify(input.into());
1353+
if let TerminalType::Pty { pty_tx, .. } = &self.terminal_type {
1354+
pty_tx.notify(input.into());
1355+
}
12941356
}
12951357

12961358
pub fn input(&mut self, input: impl Into<Cow<'static, [u8]>>) {
@@ -1594,7 +1656,7 @@ impl Terminal {
15941656
&& let Some(bytes) =
15951657
mouse_moved_report(point, e.pressed_button, e.modifiers, self.last_content.mode)
15961658
{
1597-
self.pty_tx.notify(bytes);
1659+
self.write_to_pty(bytes);
15981660
}
15991661
} else if e.modifiers.secondary() {
16001662
self.word_from_position(e.position);
@@ -1701,7 +1763,7 @@ impl Terminal {
17011763
if let Some(bytes) =
17021764
mouse_button_report(point, e.button, e.modifiers, true, self.last_content.mode)
17031765
{
1704-
self.pty_tx.notify(bytes);
1766+
self.write_to_pty(bytes);
17051767
}
17061768
} else {
17071769
match e.button {
@@ -1760,7 +1822,7 @@ impl Terminal {
17601822
if let Some(bytes) =
17611823
mouse_button_report(point, e.button, e.modifiers, false, self.last_content.mode)
17621824
{
1763-
self.pty_tx.notify(bytes);
1825+
self.write_to_pty(bytes);
17641826
}
17651827
} else {
17661828
if e.button == MouseButton::Left && setting.copy_on_select {
@@ -1799,7 +1861,7 @@ impl Terminal {
17991861
if let Some(scrolls) = scroll_report(point, scroll_lines, e, self.last_content.mode)
18001862
{
18011863
for scroll in scrolls {
1802-
self.pty_tx.notify(scroll);
1864+
self.write_to_pty(scroll);
18031865
}
18041866
};
18051867
} else if self
@@ -1808,7 +1870,7 @@ impl Terminal {
18081870
.contains(TermMode::ALT_SCREEN | TermMode::ALTERNATE_SCROLL)
18091871
&& !e.shift
18101872
{
1811-
self.pty_tx.notify(alt_scroll(scroll_lines))
1873+
self.write_to_pty(alt_scroll(scroll_lines));
18121874
} else if scroll_lines != 0 {
18131875
let scroll = AlacScroll::Delta(scroll_lines);
18141876

@@ -1879,10 +1941,12 @@ impl Terminal {
18791941
/// This does *not* return the working directory of the shell that runs on the
18801942
/// remote host, in case Zed is connected to a remote host.
18811943
fn client_side_working_directory(&self) -> Option<PathBuf> {
1882-
self.pty_info
1883-
.current
1884-
.as_ref()
1885-
.map(|process| process.cwd.clone())
1944+
match &self.terminal_type {
1945+
TerminalType::Pty { info, .. } => {
1946+
info.current.as_ref().map(|process| process.cwd.clone())
1947+
}
1948+
TerminalType::DisplayOnly => None,
1949+
}
18861950
}
18871951

18881952
pub fn title(&self, truncate: bool) -> String {
@@ -1899,8 +1963,8 @@ impl Terminal {
18991963
.title_override
19001964
.as_ref()
19011965
.map(|title_override| title_override.to_string())
1902-
.unwrap_or_else(|| {
1903-
self.pty_info
1966+
.unwrap_or_else(|| match &self.terminal_type {
1967+
TerminalType::Pty { info, .. } => info
19041968
.current
19051969
.as_ref()
19061970
.map(|fpi| {
@@ -1930,7 +1994,8 @@ impl Terminal {
19301994
};
19311995
format!("{process_file} — {process_name}")
19321996
})
1933-
.unwrap_or_else(|| "Terminal".to_string())
1997+
.unwrap_or_else(|| "Terminal".to_string()),
1998+
TerminalType::DisplayOnly => "Terminal".to_string(),
19341999
}),
19352000
}
19362001
}
@@ -1939,7 +2004,23 @@ impl Terminal {
19392004
if let Some(task) = self.task()
19402005
&& task.status == TaskStatus::Running
19412006
{
1942-
self.pty_info.kill_current_process();
2007+
if let TerminalType::Pty { info, .. } = &mut self.terminal_type {
2008+
info.kill_current_process();
2009+
}
2010+
}
2011+
}
2012+
2013+
pub fn pid(&self) -> Option<sysinfo::Pid> {
2014+
match &self.terminal_type {
2015+
TerminalType::Pty { info, .. } => info.pid(),
2016+
TerminalType::DisplayOnly => None,
2017+
}
2018+
}
2019+
2020+
pub fn pid_getter(&self) -> Option<&ProcessIdGetter> {
2021+
match &self.terminal_type {
2022+
TerminalType::Pty { info, .. } => Some(info.pid_getter()),
2023+
TerminalType::DisplayOnly => None,
19432024
}
19442025
}
19452026

@@ -2134,7 +2215,9 @@ unsafe fn append_text_to_term(term: &mut Term<ZedListener>, text_lines: &[&str])
21342215

21352216
impl Drop for Terminal {
21362217
fn drop(&mut self) {
2137-
self.pty_tx.0.send(Msg::Shutdown).ok();
2218+
if let TerminalType::Pty { pty_tx, .. } = &self.terminal_type {
2219+
pty_tx.0.send(Msg::Shutdown).ok();
2220+
}
21382221
}
21392222
}
21402223

crates/terminal_view/src/terminal_view.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1142,7 +1142,7 @@ impl Item for TerminalView {
11421142
fn tab_tooltip_content(&self, cx: &App) -> Option<TabTooltipContent> {
11431143
let terminal = self.terminal().read(cx);
11441144
let title = terminal.title(false);
1145-
let pid = terminal.pty_info.pid_getter().fallback_pid();
1145+
let pid = terminal.pid_getter()?.fallback_pid();
11461146

11471147
Some(TabTooltipContent::Custom(Box::new(move |_window, cx| {
11481148
cx.new(|_| TerminalTooltip::new(title.clone(), pid)).into()

0 commit comments

Comments
 (0)