Skip to content

Commit

Permalink
Fix terminal on Windows (#8999)
Browse files Browse the repository at this point in the history
### Description

Since [this PR](alacritty/alacritty#7796) has
been merged, so we can delete the `todo`s in `terminal` module.


Release Notes:

- N/A
  • Loading branch information
JunkuiZhang authored Mar 7, 2024
1 parent 2d83580 commit e85d484
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 11 deletions.
5 changes: 2 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion crates/terminal/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ doctest = false


[dependencies]
alacritty_terminal = "0.22.0"
# TODO: when new version of this crate is released, change it
alacritty_terminal = { git = "https://github.com/alacritty/alacritty", rev = "992011a4cd9a35f197acc0a0bd430d89a0d01013" }
anyhow.workspace = true
collections.workspace = true
dirs = "4.0.0"
Expand Down
43 changes: 36 additions & 7 deletions crates/terminal/src/terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,14 @@ use procinfo::LocalProcessInfo;
use serde::{Deserialize, Serialize};
use settings::Settings;
use smol::channel::{Receiver, Sender};
#[cfg(target_os = "windows")]
use std::num::NonZeroU32;
use task::TaskId;
use terminal_settings::{AlternateScroll, Shell, TerminalBlink, TerminalSettings};
use theme::{ActiveTheme, Theme};
use util::truncate_and_trailoff;
#[cfg(target_os = "windows")]
use windows::Win32::{Foundation::HANDLE, System::Threading::GetProcessId};

use std::{
cmp::{self, min},
Expand Down Expand Up @@ -402,7 +406,14 @@ impl TerminalBuilder {

// todo(windows)
#[cfg(windows)]
let (fd, shell_pid) = (-1, 0);
let (fd, shell_pid) = {
let child = pty.child_watcher();
let handle = child.raw_handle();
let pid = child.pid().unwrap_or_else(|| unsafe {
NonZeroU32::new_unchecked(GetProcessId(HANDLE(handle)))
});
(handle, u32::from(pid))
};

//And connect them together
let event_loop = EventLoop::new(
Expand Down Expand Up @@ -667,13 +678,31 @@ impl Terminal {
/// Updates the cached process info, returns whether the Zed-relevant info has changed
fn update_process_info(&mut self) -> bool {
#[cfg(unix)]
let mut pid = unsafe { libc::tcgetpgrp(self.shell_fd as i32) };
// todo(windows)
let pid = {
let ret = unsafe { libc::tcgetpgrp(self.shell_fd as i32) };
if ret < 0 {
self.shell_pid as i32
} else {
ret
}
};

#[cfg(windows)]
let mut pid = unsafe { windows::Win32::System::Threading::GetCurrentProcessId() } as i32;
if pid < 0 {
pid = self.shell_pid as i32;
}
let pid = {
let ret = unsafe { GetProcessId(HANDLE(self.shell_fd as _)) };
// the GetProcessId may fail and returns zero, which will lead to a stack overflow issue
if ret == 0 {
// in the builder process, there is a small chance, almost negligible,
// that this value could be zero, which means child_watcher returns None,
// GetProcessId returns 0.
if self.shell_pid == 0 {
return false;
}
self.shell_pid
} else {
ret
}
} as i32;

if let Some(process_info) = LocalProcessInfo::with_root_pid(pid as u32) {
let res = self
Expand Down

0 comments on commit e85d484

Please sign in to comment.