terminal: Give child processes time to exit on their own (#47408)
Closes #44003 Supersedes https://github.com/zed-industries/zed/pull/45777 Co-authored-by: Lukas <lukas@zed.dev> Release Notes: - Fixed an issue where the terminal would sometimes fail to write shell history. --------- Co-authored-by: Lukas Wirth <me@lukaswirth.dev>
This commit is contained in:
1 parent
abac902e12
commit
028de6dafc
4 files changed
+71
-21
No files matched your search
@@ -2633,6 +2633,7 @@ mod tests {
|
||||
::terminal::terminal_settings::AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap();
|
||||
builder.subscribe(cx)
|
||||
@@ -2706,6 +2707,7 @@ mod tests {
|
||||
::terminal::terminal_settings::AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap();
|
||||
builder.subscribe(cx)
|
||||
@@ -4080,6 +4082,7 @@ mod tests {
|
||||
::terminal::terminal_settings::AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap();
|
||||
builder.subscribe(cx)
|
||||
@@ -4125,6 +4128,7 @@ mod tests {
|
||||
::terminal::terminal_settings::AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap();
|
||||
builder.subscribe(cx)
|
||||
@@ -4184,6 +4188,7 @@ mod tests {
|
||||
::terminal::terminal_settings::AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap();
|
||||
builder.subscribe(cx)
|
||||
|
||||
@@ -1196,6 +1196,7 @@ impl acp::Client for ClientDelegate {
|
||||
AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)?;
|
||||
let lower = cx.new(|cx| builder.subscribe(cx));
|
||||
thread.on_terminal_provider_event(
|
||||
|
||||
@@ -60,14 +60,14 @@ use std::{
|
||||
path::PathBuf,
|
||||
process::ExitStatus,
|
||||
sync::Arc,
|
||||
time::Instant,
|
||||
time::{Duration, Instant},
|
||||
};
|
||||
use thiserror::Error;
|
||||
|
||||
use gpui::{
|
||||
App, AppContext as _, Bounds, ClipboardItem, Context, EventEmitter, Hsla, Keystroke, Modifiers,
|
||||
MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point, Rgba,
|
||||
ScrollWheelEvent, Size, Task, TouchPhase, Window, actions, black, px,
|
||||
App, AppContext as _, BackgroundExecutor, Bounds, ClipboardItem, Context, EventEmitter, Hsla,
|
||||
Keystroke, Modifiers, MouseButton, MouseDownEvent, MouseMoveEvent, MouseUpEvent, Pixels, Point,
|
||||
Rgba, ScrollWheelEvent, Size, Task, TouchPhase, Window, actions, black, px,
|
||||
};
|
||||
|
||||
use crate::mappings::{colors::to_alac_rgb, keys::to_esc_str};
|
||||
@@ -346,6 +346,7 @@ impl TerminalBuilder {
|
||||
alternate_scroll: AlternateScroll,
|
||||
max_scroll_history_lines: Option<usize>,
|
||||
window_id: u64,
|
||||
background_executor: &BackgroundExecutor,
|
||||
) -> Result<TerminalBuilder> {
|
||||
// Create a display-only terminal (no actual PTY).
|
||||
let default_cursor_style = AlacCursorStyle::from(cursor_shape);
|
||||
@@ -409,6 +410,7 @@ impl TerminalBuilder {
|
||||
},
|
||||
child_exited: None,
|
||||
event_loop_task: Task::ready(Ok(())),
|
||||
background_executor: background_executor.clone(),
|
||||
};
|
||||
|
||||
Ok(TerminalBuilder {
|
||||
@@ -434,6 +436,7 @@ impl TerminalBuilder {
|
||||
activation_script: Vec<String>,
|
||||
) -> Task<Result<TerminalBuilder>> {
|
||||
let version = release_channel::AppVersion::global(cx);
|
||||
let background_executor = cx.background_executor().clone();
|
||||
let fut = async move {
|
||||
// Remove SHLVL so the spawned shell initializes it to 1, matching
|
||||
// the behavior of standalone terminal emulators like iTerm2/Kitty/Alacritty.
|
||||
@@ -636,6 +639,7 @@ impl TerminalBuilder {
|
||||
},
|
||||
child_exited: None,
|
||||
event_loop_task: Task::ready(Ok(())),
|
||||
background_executor,
|
||||
};
|
||||
|
||||
if !activation_script.is_empty() && no_task {
|
||||
@@ -858,6 +862,7 @@ pub struct Terminal {
|
||||
activation_script: Vec<String>,
|
||||
child_exited: Option<ExitStatus>,
|
||||
event_loop_task: Task<Result<(), anyhow::Error>>,
|
||||
background_executor: BackgroundExecutor,
|
||||
}
|
||||
|
||||
struct CopyTemplate {
|
||||
@@ -2353,9 +2358,18 @@ unsafe fn append_text_to_term(term: &mut Term<ZedListener>, text_lines: &[&str])
|
||||
|
||||
impl Drop for Terminal {
|
||||
fn drop(&mut self) {
|
||||
if let TerminalType::Pty { pty_tx, info } = &mut self.terminal_type {
|
||||
info.kill_child_process();
|
||||
if let TerminalType::Pty { pty_tx, mut info } =
|
||||
std::mem::replace(&mut self.terminal_type, TerminalType::DisplayOnly)
|
||||
{
|
||||
pty_tx.0.send(Msg::Shutdown).ok();
|
||||
|
||||
let timer = self.background_executor.timer(Duration::from_millis(100));
|
||||
self.background_executor
|
||||
.spawn(async move {
|
||||
timer.await;
|
||||
info.kill_child_process();
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -2545,9 +2559,15 @@ mod tests {
|
||||
});
|
||||
|
||||
let terminal = cx.new(|cx| {
|
||||
TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
TerminalBuilder::new_display_only(
|
||||
CursorShape::default(),
|
||||
AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
});
|
||||
|
||||
terminal.update(cx, |terminal, cx| {
|
||||
@@ -2921,9 +2941,15 @@ mod tests {
|
||||
#[gpui::test]
|
||||
async fn test_write_output_converts_lf_to_crlf(cx: &mut TestAppContext) {
|
||||
let terminal = cx.new(|cx| {
|
||||
TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
TerminalBuilder::new_display_only(
|
||||
CursorShape::default(),
|
||||
AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
});
|
||||
|
||||
// Test simple LF conversion
|
||||
@@ -2962,9 +2988,15 @@ mod tests {
|
||||
#[gpui::test]
|
||||
async fn test_write_output_preserves_existing_crlf(cx: &mut TestAppContext) {
|
||||
let terminal = cx.new(|cx| {
|
||||
TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
TerminalBuilder::new_display_only(
|
||||
CursorShape::default(),
|
||||
AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
});
|
||||
|
||||
// Test that existing CRLF doesn't get doubled
|
||||
@@ -2997,9 +3029,15 @@ mod tests {
|
||||
#[gpui::test]
|
||||
async fn test_write_output_preserves_bare_cr(cx: &mut TestAppContext) {
|
||||
let terminal = cx.new(|cx| {
|
||||
TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
TerminalBuilder::new_display_only(
|
||||
CursorShape::default(),
|
||||
AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.unwrap()
|
||||
.subscribe(cx)
|
||||
});
|
||||
|
||||
// Test that bare CR (without LF) is preserved
|
||||
|
||||
@@ -556,9 +556,15 @@ mod tests {
|
||||
app_cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
|
||||
|
||||
let terminal = app_cx.new(|cx| {
|
||||
TerminalBuilder::new_display_only(CursorShape::default(), AlternateScroll::On, None, 0)
|
||||
.expect("Failed to create display-only terminal")
|
||||
.subscribe(cx)
|
||||
TerminalBuilder::new_display_only(
|
||||
CursorShape::default(),
|
||||
AlternateScroll::On,
|
||||
None,
|
||||
0,
|
||||
cx.background_executor(),
|
||||
)
|
||||
.expect("Failed to create display-only terminal")
|
||||
.subscribe(cx)
|
||||
});
|
||||
|
||||
let workspace_a = workspace.clone();
|
||||
|
||||
Reference in new issue
Block a user