diff --git a/Cargo.toml b/Cargo.toml index 9d889ed5..03963cdc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,4 +88,4 @@ ctrlc = "3" # adding it directly here so the feature gate is explicit. # `SetConsoleCtrlHandler` for `Win32_System_Console`: clearing the inherited # ignore-Ctrl-C flag so panes get an interrupt, not just the 0x03 byte. -windows-sys = { version = "0.61", features = ["Win32_System_Time", "Win32_Storage_FileSystem", "Win32_Foundation", "Win32_System_Console", "Win32_UI_Shell", "Win32_UI_WindowsAndMessaging"] } +windows-sys = { version = "0.61", features = ["Win32_System_Time", "Win32_Storage_FileSystem", "Win32_Foundation", "Win32_System_Console", "Win32_System_JobObjects", "Win32_System_Threading", "Win32_UI_Shell", "Win32_UI_WindowsAndMessaging"] } diff --git a/docs/architecture/session.md b/docs/architecture/session.md index 43d56524..d125fe3d 100644 --- a/docs/architecture/session.md +++ b/docs/architecture/session.md @@ -17,9 +17,9 @@ trait TerminalBackend { } ``` -`PtyBackend`는 `portable-pty`와 reader/waiter thread로 로컬 child를 소유하고, `HubBackend`는 daemon hub에 요청만 보낸다. pane id·title·resize·reorder는 즉시 로컬 상태로 확정하지 않고 `Created`, `Resized`, `Reordered`, `Exited` 같은 backend event를 따른다. `drain_events`는 보고만 하며 `Exited`를 받은 owner가 `destroy_pane`을 호출해 자원을 회수한다. VT parsing은 두 backend 모두 client-side `PaneEmulator`가 담당한다. pane child의 환경은 daemon이 상속한 값이 아니라 pane이 실제로 렌더되는 emulator를 기준으로 맞춘다: `TERM=xterm-256color`, `COLORTERM=truecolor`를 강제하고 `NO_COLOR`는 제거한다. daemon은 agent shell이나 service manager처럼 터미널이 아닌 곳에서 시작될 수 있고, 그런 부모는 자기 자식용으로 `NO_COLOR=1`, `TERM=dumb`를 내보내는 일이 흔하기 때문이다. +`PtyBackend`는 `portable-pty`와 reader/waiter thread로 로컬 child를 소유하고, `HubBackend`는 daemon hub에 요청만 보낸다. 각 pane은 생성 직후 Unix session 또는 Windows Job Object를 종료 경계로 삼아 `destroy_pane`과 hub 종료가 그 경계 안의 subprocess를 함께 종료한다. 단순 detach·quit·browser disconnect는 pane destroy 경로를 호출하지 않으므로 프로세스를 유지한다. pane id·title·resize·reorder는 즉시 로컬 상태로 확정하지 않고 `Created`, `Resized`, `Reordered`, `Exited` 같은 backend event를 따른다. `drain_events`는 보고만 하며 `Exited`를 받은 owner가 `destroy_pane`을 호출해 자원을 회수한다. VT parsing은 두 backend 모두 client-side `PaneEmulator`가 담당한다. pane child의 환경은 daemon이 상속한 값이 아니라 pane이 실제로 렌더되는 emulator를 기준으로 맞춘다: `TERM=xterm-256color`, `COLORTERM=truecolor`를 강제하고 `NO_COLOR`는 제거한다. daemon은 agent shell이나 service manager처럼 터미널이 아닌 곳에서 시작될 수 있고, 그런 부모는 자기 자식용으로 `NO_COLOR=1`, `TERM=dumb`를 내보내는 일이 흔하기 때문이다. -세션 상한은 repository당 PTY 8개, pane 크기 1–500행 × 1–1100열, pane당 reconnect scrollback 256 KiB다. 명령 queue가 가득 찼다는 이유로 close/resize의 성공을 가정하지 않는다. +세션 상한은 repository당 PTY 8개, pane 크기 1–500행 × 1–1100열, pane당 reconnect scrollback 256 KiB다. close와 resize는 bounded input queue 밖의 전용 latest-state 경로로 보내 queue 포화에도 마지막 요청을 잃지 않는다. ## Shared state diff --git a/docs/architecture/ui.md b/docs/architecture/ui.md index d9341dc4..21a2fe5a 100644 --- a/docs/architecture/ui.md +++ b/docs/architecture/ui.md @@ -24,7 +24,7 @@ TUI workspace state는 `~/.nightcrow/workspace.json`에 저장한다. 열린 pro `ui::chrome::chrome_areas`가 project tabs, body, notice, hint 네 영역을 항상 만든다. notice와 hint는 배치와 무관하게 화면 아래 두 행이고(`bottom_rows`), project tabs는 `[layout] tabs`에 따라 그 위의 첫 행(`top`) 또는 좌측 `STRIP_WIDTH`(20) 열(`left`)이며 body는 남은 영역이다. body의 upper/lower split은 TUI layout config에서 계산하고, terminal pane rect는 [terminal.md](terminal.md)의 단일 기하 출처를 사용한다. notice나 dialog 때문에 행을 추가·삭제하지 않는다. -입력·PTY output·snapshot/load 결과·tree watch·resize·recovery·title 변화는 dirty frame을 요청한다. event loop는 16 ms마다 queue를 poll하지만 변경 없는 tick에는 `Terminal::draw`를 호출하지 않는다. ` r`만 front buffer를 비우는 명시적 full repaint다. status의 hot-file fade와 attention/search caret 경계도 timer event로 dirty를 만든다. +입력·PTY output·snapshot/load 결과·tree watch·resize·recovery·title 변화는 dirty frame을 요청한다. event loop는 16 ms마다 queue를 poll하지만 변경 없는 tick에는 `Terminal::draw`를 호출하지 않는다. ` r`만 front buffer를 비우는 명시적 full repaint다. status의 hot-file fade와 attention/search caret 경계도 timer event로 dirty를 만든다. repo 입력을 열면 notice row의 좌우 border가 고정된 채 accent/dim 두 pulse를 보여 주고, 두 pulse가 끝나면 dim으로 유지한다. 이 focus cue는 phase 경계에서만 repaint하며 완료 뒤 timer를 더 예약하지 않는다. ## Notice row diff --git a/src/application/event_loop.rs b/src/application/event_loop.rs index 1df9366e..97422ff9 100644 --- a/src/application/event_loop.rs +++ b/src/application/event_loop.rs @@ -8,7 +8,7 @@ use crate::application::terminal_guard::TuiTerminal; use crate::workspace::Workspace; use crossterm::event::{self, Event}; use ratatui::layout::Rect; -use std::time::{Duration, SystemTime}; +use std::time::{Duration, Instant, SystemTime}; use syntect::highlighting::ThemeSet; use syntect::parsing::SyntaxSet; @@ -123,6 +123,9 @@ pub(crate) fn main_loop( .active() .is_some_and(crate::app::App::search_overlay_active); redraw.observe_caret(caret_active, crate::ui::current_caret_lit()); + if ws.advance_repo_input_focus_flash(Instant::now()) { + redraw.request(RedrawCause::RepoInputFocus); + } let active_tab = ws.active_index(); let empty_notice = ws.empty_notice().cloned(); let prefix_armed = ws.prefix_armed(); diff --git a/src/application/redraw.rs b/src/application/redraw.rs index 74bd3113..1f11d8bf 100644 --- a/src/application/redraw.rs +++ b/src/application/redraw.rs @@ -19,6 +19,7 @@ pub(crate) enum RedrawCause { Log, AttentionBlink, CaretBlink, + RepoInputFocus, HotFile, Session, Redraw, @@ -124,6 +125,7 @@ mod tests { RedrawCause::Git, RedrawCause::Log, RedrawCause::HotFile, + RedrawCause::RepoInputFocus, RedrawCause::Session, RedrawCause::Redraw, ]; diff --git a/src/application/tests/prefix.rs b/src/application/tests/prefix.rs index 04b389ee..ed54b124 100644 --- a/src/application/tests/prefix.rs +++ b/src/application/tests/prefix.rs @@ -58,7 +58,7 @@ fn leader_x_asks_the_workspace_to_close_the_project() { let mut app = app_with_files(vec!["a.rs"]); let _ = handle_key(&mut app, leader()); - let outcome = handle_key(&mut app, press(KeyCode::Char('x'), KeyModifiers::NONE)); + let outcome = handle_key(&mut app, press(KeyCode::Char('x'), KeyModifiers::CONTROL)); assert_eq!(outcome, KeyOutcome::Project(ProjectRequest::Close)); assert!( @@ -208,7 +208,7 @@ fn handle_key_leader_w_closes_pane_with_terminal_focus() { app.terminal.create_pane_now().unwrap(); let before = app.terminal.panes.len(); let _ = handle_key(&mut app, leader()); - let _ = handle_key(&mut app, press(KeyCode::Char('w'), KeyModifiers::NONE)); + let _ = handle_key(&mut app, press(KeyCode::Char('w'), KeyModifiers::CONTROL)); // Closing is a request; the pane goes when its exit arrives. app.poll_terminal(); assert_eq!(app.terminal.panes.len(), before - 1); diff --git a/src/backend/pty.rs b/src/backend/pty.rs index 527b2194..054b7540 100644 --- a/src/backend/pty.rs +++ b/src/backend/pty.rs @@ -1,6 +1,7 @@ use super::slot::{PaneSlot, PaneSlots}; use super::{BackendEvent, PaneId, ResizeOutcome, TerminalBackend}; use crate::config::ShellConfig; +use crate::platform::process_tree::ProcessTree; use crate::platform::threading::try_timed_join; use anyhow::Result; use portable_pty::PtySize; @@ -56,6 +57,7 @@ pub(super) struct PtyPane { pub(super) master: Option>, pub(super) writer: Option>, pub(super) killer: Box, + pub(super) process_tree: ProcessTree, pub(super) rx: Receiver, pub(super) reader_handle: Option>, pub(super) wait_handle: Option>, @@ -64,7 +66,13 @@ pub(super) struct PtyPane { impl Drop for PtyPane { fn drop(&mut self) { - // Best-effort kill: the child may already be gone. + // Terminate the pane's process boundary before the PTY handles and + // waiter are reaped, so child jobs cannot outlive an explicit close. + if let Err(error) = self.process_tree.terminate() { + tracing::warn!(%error, "could not terminate pane process tree"); + } + // Best-effort direct kill covers a child that exited before its tree + // termination raced with teardown. let _ = self.killer.kill(); // Drop writer/master so the reader's blocked `read()` returns EOF; // without this, joining the reader would hang. diff --git a/src/backend/pty_spawn.rs b/src/backend/pty_spawn.rs index 5e04b306..6609b567 100644 --- a/src/backend/pty_spawn.rs +++ b/src/backend/pty_spawn.rs @@ -2,6 +2,7 @@ use super::{ExitPhase, PtyBackend, PtyEvent, PtyPane}; use crate::backend::PaneId; use crate::backend::identity::{PANE_TOKEN_ENV, PLUGIN_RUNTIME_DIR_ENV, PaneIdentity}; use crate::backend::slot::{PaneLaunch, resume_command_line}; +use crate::platform::process_tree::ProcessTree; use anyhow::Result; use portable_pty::{CommandBuilder, NativePtySystem, PtySize, PtySystem}; use std::io::Read; @@ -111,6 +112,16 @@ impl PtyBackend { cmd.cwd(canonical); } let mut child = pair.slave.spawn_command(cmd)?; + let process_tree = match ProcessTree::attach(&*child, &*pair.master) { + Ok(tree) => tree, + Err(error) => { + // Do not leave a process behind when establishing the tree + // boundary fails before the pane is inserted in the backend. + let _ = child.kill(); + let _ = child.wait(); + return Err(error.context("establishing pane process tree")); + } + }; let killer = child.clone_killer(); drop(pair.slave); @@ -154,6 +165,7 @@ impl PtyBackend { master: Some(pair.master), writer: Some(writer), killer, + process_tree, rx, reader_handle: Some(reader_handle), wait_handle: Some(wait_handle), diff --git a/src/backend/pty_tests/lifecycle.rs b/src/backend/pty_tests/lifecycle.rs index 20b66206..5571f761 100644 --- a/src/backend/pty_tests/lifecycle.rs +++ b/src/backend/pty_tests/lifecycle.rs @@ -9,6 +9,110 @@ fn pty_backend_create_and_destroy_pane() { assert!(!backend.panes.contains_key(&id)); } +#[test] +fn destroying_a_pane_terminates_its_subprocess() { + let dir = tempfile::tempdir().expect("tempdir"); + let marker = dir.path().join("child.pid"); + let (shell, command) = descendant_command(&marker); + let mut backend = PtyBackend::new(".", shell); + let id = backend + .open_pane(24, 80, Some(&command)) + .expect("open pane with descendant"); + let pid = wait_for_pid(&mut backend, id, &marker); + + backend.destroy_pane(id); + + let deadline = Instant::now() + Duration::from_secs(5); + while process_is_alive(pid) && Instant::now() < deadline { + thread::sleep(Duration::from_millis(20)); + } + assert!( + !process_is_alive(pid), + "pane descendant {pid} survived close" + ); +} + +#[cfg(unix)] +fn descendant_command(marker: &std::path::Path) -> (ShellConfig, String) { + ( + ShellConfig::default(), + format!("sh -c 'echo $$ > \"{}\"; exec sleep 60'", marker.display()), + ) +} + +#[cfg(windows)] +fn descendant_command(marker: &std::path::Path) -> (ShellConfig, String) { + let shell = ShellConfig { + program: Some("powershell.exe".to_string()), + command_args: vec!["-NoProfile".to_string(), "-Command".to_string()], + }; + let command = format!( + "$p = Start-Process cmd.exe -WindowStyle Hidden -ArgumentList '/C','ping -t 127.0.0.1' -PassThru; [IO.File]::WriteAllText('{}', [string]$p.Id); $p.WaitForExit()", + marker.display() + ); + (shell, command) +} + +fn wait_for_pid(backend: &mut PtyBackend, id: PaneId, marker: &std::path::Path) -> u32 { + let deadline = Instant::now() + Duration::from_secs(10); + let mut output = Vec::new(); + while Instant::now() < deadline { + if let Ok(text) = std::fs::read_to_string(marker) + && let Ok(pid) = text.trim().parse() + { + return pid; + } + for event in backend.drain_events() { + if let BackendEvent::Output { pane, data } = event + && pane == id + { + if data.windows(4).any(|window| window == b"\x1b[6n") { + let _ = backend.send_input(id, b"\x1b[1;1R"); + } + output.extend(data); + } + } + thread::sleep(Duration::from_millis(20)); + } + panic!( + "subprocess did not write {}: {}", + marker.display(), + String::from_utf8_lossy(&output) + ); +} + +#[cfg(any(target_os = "linux", target_os = "android"))] +fn process_is_alive(pid: u32) -> bool { + if let Ok(stat) = std::fs::read_to_string(format!("/proc/{pid}/stat")) + && stat.rfind(')').and_then(|end| stat.as_bytes().get(end + 2)) == Some(&b'Z') + { + return false; + } + let result = unsafe { libc::kill(pid as libc::pid_t, 0) }; + result == 0 || std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) +} + +#[cfg(all(unix, not(any(target_os = "linux", target_os = "android"))))] +fn process_is_alive(pid: u32) -> bool { + let result = unsafe { libc::kill(pid as libc::pid_t, 0) }; + result == 0 || std::io::Error::last_os_error().raw_os_error() == Some(libc::EPERM) +} + +#[cfg(windows)] +fn process_is_alive(pid: u32) -> bool { + use windows_sys::Win32::Foundation::{CloseHandle, WAIT_TIMEOUT}; + use windows_sys::Win32::System::Threading::{OpenProcess, WaitForSingleObject}; + + const SYNCHRONIZE_ACCESS: u32 = 0x0010_0000; + let process = unsafe { OpenProcess(SYNCHRONIZE_ACCESS, 0, pid) }; + if process.is_null() { + return false; + } + let alive = unsafe { WaitForSingleObject(process, 0) } == WAIT_TIMEOUT; + unsafe { CloseHandle(process) }; + alive +} + #[test] fn resizing_an_unknown_pane_is_reported() { let mut backend = PtyBackend::new(".", ShellConfig::default()); diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 355af73f..4fe7730d 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -7,6 +7,7 @@ mod link_target_paths; pub(crate) mod links; pub(crate) mod logging; pub(crate) mod paths; +pub(crate) mod process_tree; pub(crate) mod self_replace; pub(crate) mod signals; pub(crate) mod threading; diff --git a/src/platform/process_tree.rs b/src/platform/process_tree.rs new file mode 100644 index 00000000..62e1ae93 --- /dev/null +++ b/src/platform/process_tree.rs @@ -0,0 +1,264 @@ +//! The lifetime boundary for a terminal pane's child processes. +//! +//! `portable-pty` creates a new Unix session for every pane. The shell may +//! then put jobs in separate process groups, so killing only the shell's group +//! leaves jobs behind. Linux and macOS enumerate the session before teardown +//! and kill every group in it; other Unix targets still get the root and +//! foreground groups, followed by closing the PTY master in `PtyPane::drop`. Windows uses +//! a Job Object, whose kernel membership includes descendants created after +//! the child is assigned. + +use anyhow::{Context as _, Result}; +use portable_pty::{Child, MasterPty}; +use std::io; + +#[cfg(unix)] +#[derive(Debug)] +pub(crate) struct ProcessTree { + process_group: libc::pid_t, + session: libc::pid_t, + tty: Option, +} + +#[cfg(windows)] +#[derive(Debug)] +pub(crate) struct ProcessTree { + job: std::os::windows::io::OwnedHandle, +} + +impl ProcessTree { + /// Establish the platform boundary immediately after the PTY child is + /// created. A failure is returned so an unowned pane is never admitted. + pub(crate) fn attach(child: &dyn Child, master: &dyn MasterPty) -> Result { + #[cfg(unix)] + { + let pid = child + .process_id() + .ok_or_else(|| anyhow::anyhow!("PTY child did not expose a process id"))? + as libc::pid_t; + if pid <= 1 { + anyhow::bail!("refusing unsafe PTY process id {pid}"); + } + + // portable-pty calls setsid() before exec, which makes the child a + // session and process-group leader. Check the session while the + // child is alive so a recycled PID can never be mistaken for ours. + let process_group = unsafe { libc::getpgid(pid) }; + let session = unsafe { libc::getsid(pid) }; + if process_group <= 1 || session <= 1 { + return Err(io::Error::last_os_error()).context("querying PTY process session"); + } + Ok(Self { + process_group, + session, + tty: master.as_raw_fd(), + }) + } + + #[cfg(windows)] + { + use std::os::windows::io::{AsRawHandle, FromRawHandle}; + use windows_sys::Win32::Foundation::HANDLE; + use windows_sys::Win32::System::JobObjects::{ + AssignProcessToJobObject, CreateJobObjectW, JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, + JOBOBJECT_EXTENDED_LIMIT_INFORMATION, JobObjectExtendedLimitInformation, + SetInformationJobObject, + }; + + let process = child + .as_raw_handle() + .ok_or_else(|| anyhow::anyhow!("PTY child did not expose a process handle"))?; + let _ = master; + let raw_job = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; + if raw_job.is_null() { + return Err(io::Error::last_os_error()).context("creating pane Job Object"); + } + + // OwnedHandle closes the job on every return path. The limit flag + // makes that close a final tree termination if setup is interrupted + // before an explicit TerminateJobObject call. + let job = unsafe { std::os::windows::io::OwnedHandle::from_raw_handle(raw_job) }; + let mut limits = JOBOBJECT_EXTENDED_LIMIT_INFORMATION::default(); + limits.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + let job_handle = job.as_raw_handle() as HANDLE; + let set_ok = unsafe { + SetInformationJobObject( + job_handle, + JobObjectExtendedLimitInformation, + (&limits as *const JOBOBJECT_EXTENDED_LIMIT_INFORMATION).cast(), + std::mem::size_of::() as u32, + ) + }; + if set_ok == 0 { + return Err(io::Error::last_os_error()).context("configuring pane Job Object"); + } + // Assignment is necessarily after portable-pty's CreateProcessW; + // the direct child kill in `PtyPane::drop` covers a setup failure, + // while the job owns descendants created after this point. + let assign_ok = unsafe { AssignProcessToJobObject(job_handle, process as HANDLE) }; + if assign_ok == 0 { + return Err(io::Error::last_os_error()) + .context("assigning PTY child to pane Job Object"); + } + Ok(Self { job }) + } + } + + /// Terminate the pane's process tree. An already exited tree is normal + /// during backend teardown and is treated as success. + pub(crate) fn terminate(&self) -> io::Result<()> { + #[cfg(unix)] + { + let mut first_error = None; + for pass in 0..3 { + let mut groups = std::collections::BTreeSet::new(); + if pass == 0 { + groups.insert(self.process_group); + if let Some(foreground) = self.foreground_group() { + groups.insert(foreground); + } + } + #[cfg(any(target_os = "linux", target_os = "android", target_os = "macos"))] + groups.extend(session_process_groups(self.session)); + + for &group in &groups { + if let Err(error) = kill_group(group) { + first_error.get_or_insert(error); + } + } + } + first_error.map_or(Ok(()), Err) + } + + #[cfg(windows)] + { + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::Foundation::ERROR_INVALID_HANDLE; + use windows_sys::Win32::System::JobObjects::TerminateJobObject; + + let result = unsafe { TerminateJobObject(self.job.as_raw_handle() as _, 1) }; + if result != 0 { + return Ok(()); + } + let error = io::Error::last_os_error(); + if error.raw_os_error() == Some(ERROR_INVALID_HANDLE as i32) { + Ok(()) + } else { + Err(error) + } + } + } + + #[cfg(unix)] + fn foreground_group(&self) -> Option { + let tty = self.tty?; + let group = unsafe { libc::tcgetpgrp(tty) }; + if group <= 1 { + return None; + } + // `tcgetpgrp` identifies the current controlling terminal's group. A + // session check prevents a stale/reused group id from escaping the + // pane boundary. + let session = unsafe { libc::getsid(group) }; + (session == self.session).then_some(group) + } +} + +#[cfg(unix)] +fn kill_group(group: libc::pid_t) -> io::Result<()> { + if group <= 1 { + return Ok(()); + } + let result = unsafe { libc::kill(-group, libc::SIGKILL) }; + if result == 0 { + Ok(()) + } else { + let error = io::Error::last_os_error(); + if error.raw_os_error() == Some(libc::ESRCH) { + Ok(()) + } else { + Err(error) + } + } +} + +#[cfg(any(target_os = "linux", target_os = "android"))] +fn session_process_groups(session: libc::pid_t) -> impl Iterator { + let mut groups = std::collections::BTreeSet::new(); + let Ok(entries) = std::fs::read_dir("/proc") else { + return groups.into_iter(); + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let Some(name) = name.to_str() else { continue }; + if !name.bytes().all(|byte| byte.is_ascii_digit()) { + continue; + } + let Ok(stat) = std::fs::read_to_string(entry.path().join("stat")) else { + continue; + }; + let Some((_, group, proc_session)) = parse_proc_stat(&stat) else { + continue; + }; + if proc_session == session && group > 1 { + groups.insert(group); + } + } + groups.into_iter() +} + +#[cfg(target_os = "macos")] +fn session_process_groups(session: libc::pid_t) -> impl Iterator { + let mut groups = std::collections::BTreeSet::new(); + let Ok(output) = std::process::Command::new("/bin/ps") + .args(["-axo", "pgid=,sid="]) + .output() + else { + return groups.into_iter(); + }; + if !output.status.success() { + return groups.into_iter(); + } + for line in String::from_utf8_lossy(&output.stdout).lines() { + let mut fields = line.split_whitespace(); + let Some(group) = fields + .next() + .and_then(|value| value.parse::().ok()) + else { + continue; + }; + let Some(proc_session) = fields + .next() + .and_then(|value| value.parse::().ok()) + else { + continue; + }; + if proc_session == session && group > 1 { + groups.insert(group); + } + } + groups.into_iter() +} + +#[cfg(any(target_os = "linux", target_os = "android"))] +fn parse_proc_stat(stat: &str) -> Option<(libc::pid_t, libc::pid_t, libc::pid_t)> { + let close = stat.rfind(')')?; + let pid = stat[..stat.find(' ')?].parse().ok()?; + let mut fields = stat.get(close + 2..)?.split_whitespace(); + fields.next()?; // state + let _parent = fields.next()?.parse::().ok()?; + let group = fields.next()?.parse::().ok()?; + let session = fields.next()?.parse::().ok()?; + Some((pid, group, session)) +} + +#[cfg(all(test, any(target_os = "linux", target_os = "android")))] +mod tests { + use super::parse_proc_stat; + + #[test] + fn proc_stat_parser_handles_spaces_and_parentheses_in_command_names() { + let stat = "123 (worker with ) mark) S 1 122 122 0 0 0"; + assert_eq!(parse_proc_stat(stat), Some((123, 122, 122))); + } +} diff --git a/src/session/terminal/hub_helpers.rs b/src/session/terminal/hub_helpers.rs index 553b3cd0..851da56b 100644 --- a/src/session/terminal/hub_helpers.rs +++ b/src/session/terminal/hub_helpers.rs @@ -31,9 +31,6 @@ pub enum Command { data: Vec, client: u64, }, - Close { - pane: PaneId, - }, Reorder { order: Vec, }, diff --git a/src/session/terminal/hub_panes.rs b/src/session/terminal/hub_panes.rs index 29e4dda1..4239aa81 100644 --- a/src/session/terminal/hub_panes.rs +++ b/src/session/terminal/hub_panes.rs @@ -16,6 +16,21 @@ use crate::session::limits; use std::collections::VecDeque; impl TerminalHub { + pub(super) fn queue_close(&self, pane: PaneId) { + self.pending_closes + .lock() + .expect("terminal close queue poisoned") + .insert(pane); + } + + pub(super) fn take_pending_closes(&self) -> Vec { + let mut pending = self + .pending_closes + .lock() + .expect("terminal close queue poisoned"); + std::mem::take(&mut *pending).into_iter().collect() + } + /// Whether another terminal fits under the cap, counting slots already /// held for a startup set that has been claimed but not created yet. pub(super) fn has_free_slot(&self) -> bool { diff --git a/src/session/terminal/hub_run.rs b/src/session/terminal/hub_run.rs index 3126c42b..9f32df25 100644 --- a/src/session/terminal/hub_run.rs +++ b/src/session/terminal/hub_run.rs @@ -26,6 +26,21 @@ impl TerminalHub { let mut clears = ClearWatch::default(); while !stop.load(Ordering::Acquire) { + for pane in self.take_pending_closes() { + if !self.pane_is_live(pane) { + continue; + } + if plugins.owner(pane).is_some() { + plugins.pane_closed(&backend, pane); + plugins.forget(&backend, pane); + backend.retire_slot(pane); + self.end_recovery(pane); + } + modes.forget(pane); + clears.forget(pane); + backend.destroy_pane(pane); + self.remove_pane_and_announce(pane); + } let mut commands_since_resize = 0; while let Ok(command) = commands.try_recv() { if resize_due_before_command(&mut commands_since_resize) { @@ -76,18 +91,6 @@ impl TerminalHub { plugins.user_input(&backend, pane); let _ = backend.send_input(pane, &data); } - Command::Close { pane } if self.pane_is_live(pane) => { - // Closed for good, unlike an exit: the slot goes with - // the process, so there is nothing left to relaunch. - if plugins.owner(pane).is_some() { - plugins.pane_closed(&backend, pane); - plugins.forget(&backend, pane); - backend.retire_slot(pane); - self.end_recovery(pane); - } - backend.destroy_pane(pane); - self.remove_pane_and_announce(pane); - } Command::Reorder { order } => self.reorder_panes(order), // Deliberately not gated on the pane being live: a pane with // a recovery pending is one whose process has already ended, diff --git a/src/session/terminal/mod.rs b/src/session/terminal/mod.rs index 93c3cc97..63c8df55 100644 --- a/src/session/terminal/mod.rs +++ b/src/session/terminal/mod.rs @@ -48,7 +48,7 @@ pub use session::TerminalSession; use crate::session::size_owner::SizeOwnership; use hub_helpers::{Command, PendingResize, Shared}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::mpsc::{self, SyncSender}; use std::sync::{Arc, Mutex}; @@ -75,6 +75,8 @@ const DEFAULT_PANE_SIZE: PaneSize = PaneSize { rows: 24, cols: 80 }; pub struct TerminalHub { pub(super) commands: SyncSender, + /// Close is lossless control state, separate from the bounded input queue. + pending_closes: Mutex>, /// Latest resize per connection and pane. Separate from `commands` so a /// full input queue cannot discard the final width of a window drag. pending_resizes: Mutex>, @@ -120,6 +122,7 @@ impl TerminalHub { let (commands, command_rx) = mpsc::sync_channel::(256); let hub = Arc::new(Self { commands, + pending_closes: Mutex::new(BTreeSet::new()), pending_resizes: Mutex::new(BTreeMap::new()), state: Mutex::new(Shared { clients: Vec::new(), diff --git a/src/session/terminal/session.rs b/src/session/terminal/session.rs index 22176d46..ea490199 100644 --- a/src/session/terminal/session.rs +++ b/src/session/terminal/session.rs @@ -81,7 +81,10 @@ impl TerminalSession { .queue_resize(pane, size.rows, size.cols, self.id, self.connection); return; } - ClientMessage::Close { pane } => Command::Close { pane }, + ClientMessage::Close { pane } => { + self.hub.queue_close(pane); + return; + } ClientMessage::Reorder { order } => Command::Reorder { order }, ClientMessage::CancelRecovery { pane } => Command::CancelRecovery { pane }, // Off the worker queue: it rearranges the panel and never reaches a diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 7513dd65..1a008923 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -94,13 +94,17 @@ pub fn draw_empty( // Shares `render_notice_row`'s row assignment so the dialog looks the // same wherever it opens: the input on this row, its reports and keys on // the hint row. With no project there is no repo header to fall back to. - let notice_line = if chrome.repo_input.active { - repo_dialog::repo_input_line(chrome.repo_input, accent, rows.notice.width) + if chrome.repo_input.active { + frame.render_widget( + repo_dialog::render_repo_input_row(chrome.repo_input, accent, rows.notice.width), + rows.notice, + ); } else { - notice::notice_or_candidates(notice, chrome.repo_input, None, rows.notice.width) - .unwrap_or_default() - }; - frame.render_widget(Paragraph::new(notice_line), rows.notice); + let notice_line = + notice::notice_or_candidates(notice, chrome.repo_input, None, rows.notice.width) + .unwrap_or_default(); + frame.render_widget(Paragraph::new(notice_line), rows.notice); + } // The armed prefix shows the same chip as the project screen: pressing // the leader here has to look like it did something. diff --git a/src/ui/notice.rs b/src/ui/notice.rs index 9007654b..44f6be57 100644 --- a/src/ui/notice.rs +++ b/src/ui/notice.rs @@ -20,9 +20,7 @@ pub(crate) fn render_notice_row<'a>( // left, the input names the one being opened. Notices follow the dialog // down to the hint row so nothing covers the path being typed. if repo_input.active { - return Paragraph::new(crate::ui::repo_dialog::repo_input_line( - repo_input, accent, width, - )); + return crate::ui::repo_dialog::render_repo_input_row(repo_input, accent, width); } match notice_or_candidates( app.notice.as_ref(), diff --git a/src/ui/repo_dialog.rs b/src/ui/repo_dialog.rs index df7ee43c..d5b140bd 100644 --- a/src/ui/repo_dialog.rs +++ b/src/ui/repo_dialog.rs @@ -7,8 +7,28 @@ use crate::ui::status_view::RepoInput; use ratatui::{ style::{Color, Style}, text::{Line, Span}, + widgets::{Block, Borders, Paragraph}, }; +/// Render the active field with a fixed two-cell side rail. Keeping the rail +/// in both phases means the path and caret never shift while focus is cued. +pub(crate) fn render_repo_input_row<'a>( + repo_input: &'a RepoInput, + accent: Color, + width: u16, +) -> Paragraph<'a> { + let border_color = if repo_input.focus_flash_bright() { + accent + } else { + Color::DarkGray + }; + Paragraph::new(repo_input_line(repo_input, accent, width.saturating_sub(2))).block( + Block::default() + .borders(Borders::LEFT | Borders::RIGHT) + .border_style(Style::default().fg(border_color)), + ) +} + /// The dialog's input line. Drawn on the notice row, in the repo header's /// place: the header names the repo being left, the input names the one being /// opened, and only one of those is being decided right now. A path longer diff --git a/src/ui/status_view.rs b/src/ui/status_view.rs index 8a447260..cf9078e7 100644 --- a/src/ui/status_view.rs +++ b/src/ui/status_view.rs @@ -2,7 +2,7 @@ use crate::git::diff::ChangedFile; use crate::ui::SearchQuery; use std::cell::Cell; use std::collections::HashMap; -use std::time::SystemTime; +use std::time::{Duration, Instant, SystemTime}; #[derive(Default)] pub struct StatusView { @@ -105,4 +105,42 @@ pub struct RepoInput { /// dialog's keys; the field stays on screen below it and keeps the text, so /// closing the browser returns to exactly what was being typed. pub picker: Option, + /// Monotonic phase for the short focus cue shown when the dialog opens. + pub(crate) focus_flash_started: Option, + pub(crate) focus_flash_phase: u8, +} + +/// One bright/dim half-cycle of the repo-input focus cue. +pub(crate) const REPO_INPUT_FLASH_PHASE: Duration = Duration::from_millis(100); +const REPO_INPUT_FLASH_PHASES: u8 = 4; + +impl RepoInput { + pub(crate) fn start_focus_flash(&mut self) { + self.start_focus_flash_at(Instant::now()); + } + + pub(crate) fn start_focus_flash_at(&mut self, now: Instant) { + self.focus_flash_started = Some(now); + self.focus_flash_phase = 0; + } + + /// Move to the phase containing `now`; returns whether a repaint is needed. + pub(crate) fn advance_focus_flash_at(&mut self, now: Instant) -> bool { + let Some(started) = self.focus_flash_started else { + return false; + }; + let phase = (now.saturating_duration_since(started).as_millis() + / REPO_INPUT_FLASH_PHASE.as_millis()) + .min(u128::from(REPO_INPUT_FLASH_PHASES)) as u8; + let changed = phase != self.focus_flash_phase; + self.focus_flash_phase = phase; + if phase >= REPO_INPUT_FLASH_PHASES { + self.focus_flash_started = None; + } + changed + } + + pub(crate) fn focus_flash_bright(&self) -> bool { + self.focus_flash_started.is_some() && self.focus_flash_phase.is_multiple_of(2) + } } diff --git a/src/ui/tests/chrome_tests.rs b/src/ui/tests/chrome_tests.rs index 505df4b4..d4deceb0 100644 --- a/src/ui/tests/chrome_tests.rs +++ b/src/ui/tests/chrome_tests.rs @@ -43,6 +43,7 @@ fn the_empty_screen_shows_the_dialog_and_its_rejection() { buf: "/definitely/not/here".to_string(), candidates: Vec::new(), picker: None, + ..RepoInput::default() }; let notice = crate::app::Notice::new(NoticeKind::RepoInput, "no such directory"); @@ -64,6 +65,7 @@ fn the_project_screen_puts_the_dialog_on_the_notice_row_and_its_reports_below() buf: "/definitely/not/here".to_string(), candidates: Vec::new(), picker: None, + ..RepoInput::default() }; let paths = vec![".".to_string()]; let mut terminal = Terminal::new(TestBackend::new(120, 20)).unwrap(); diff --git a/src/ui/tests/notice_tests.rs b/src/ui/tests/notice_tests.rs index 3acdc2bb..2b6a0053 100644 --- a/src/ui/tests/notice_tests.rs +++ b/src/ui/tests/notice_tests.rs @@ -76,6 +76,7 @@ fn dialog_offering(candidates: &[&str]) -> crate::ui::status_view::RepoInput { buf: "/repos/".to_string(), candidates: candidates.iter().map(|c| c.to_string()).collect(), picker: None, + ..crate::ui::status_view::RepoInput::default() } } diff --git a/src/ui/tests/repo_picker_tests.rs b/src/ui/tests/repo_picker_tests.rs index 19d50c58..790d9bdc 100644 --- a/src/ui/tests/repo_picker_tests.rs +++ b/src/ui/tests/repo_picker_tests.rs @@ -1,8 +1,9 @@ use super::common::*; -use crate::ui::repo_dialog::{repo_dialog_hint_line, repo_input_line}; +use crate::ui::repo_dialog::{render_repo_input_row, repo_dialog_hint_line, repo_input_line}; use crate::ui::status_view::RepoInput; use crate::workspace::PathTree; -use ratatui::style::Color; +use ratatui::{Terminal, backend::TestBackend, style::Color}; +use std::time::Instant; use tempfile::TempDir; /// The dialog's state with the browser open on a temp directory holding `dirs`. @@ -24,6 +25,7 @@ fn browsing(dirs: &[&str]) -> (TempDir, RepoInput) { buf, candidates: Vec::new(), picker: Some(picker), + ..RepoInput::default() }, ) } @@ -34,6 +36,7 @@ fn field_only() -> RepoInput { buf: "/repos/current".to_string(), candidates: Vec::new(), picker: None, + ..RepoInput::default() } } @@ -150,6 +153,52 @@ fn a_row_too_narrow_for_any_path_keeps_the_prompt_and_caret_alone() { assert_eq!(line, " repo: |", "got: {line}"); } +#[test] +fn the_focus_border_keeps_its_rail_while_pulsing_twice() { + let mut field = field_only(); + let started = Instant::now(); + field.start_focus_flash_at(started); + let mut rendered = Vec::new(); + + for phase in 0u32..=4 { + field.advance_focus_flash_at( + started + crate::ui::status_view::REPO_INPUT_FLASH_PHASE * phase, + ); + let mut terminal = Terminal::new(TestBackend::new(40, 1)).expect("a terminal"); + terminal + .draw(|frame| { + frame.render_widget( + render_repo_input_row(&field, Color::Yellow, frame.area().width), + frame.area(), + ) + }) + .expect("draw"); + let buf = terminal.backend().buffer(); + let text = (0..buf.area.width) + .map(|x| buf[(x, 0)].symbol()) + .collect::(); + rendered.push((text, buf[(0, 0)].style().fg)); + } + + assert!(rendered.iter().all(|(text, _)| text.starts_with('│'))); + assert!(rendered.iter().all(|(text, _)| text.ends_with('│'))); + assert!( + rendered + .iter() + .all(|(text, _)| ratatui::text::Span::raw(text).width() == 40) + ); + assert_eq!( + rendered.iter().map(|(_, color)| *color).collect::>(), + vec![ + Some(Color::Yellow), + Some(Color::DarkGray), + Some(Color::Yellow), + Some(Color::DarkGray), + Some(Color::DarkGray), + ] + ); +} + #[test] fn a_rejection_takes_the_dialogs_hint_row_over_the_legend() { // The same priority the notice row applies when the dialog is closed: the diff --git a/src/workspace/repo_input.rs b/src/workspace/repo_input.rs index c7498b43..2cebc3cf 100644 --- a/src/workspace/repo_input.rs +++ b/src/workspace/repo_input.rs @@ -23,6 +23,7 @@ impl Workspace { .map(|p| p.repository_path().to_string()) .unwrap_or_default(); self.repo_input.active = true; + self.repo_input.start_focus_flash(); self.repo_input.candidates.clear(); self.repo_input.picker = None; self.clear_notice(NoticeKind::RepoInput); @@ -30,6 +31,7 @@ impl Workspace { pub fn cancel_repo_input(&mut self) { self.repo_input.active = false; + self.repo_input.focus_flash_started = None; self.repo_input.buf.clear(); self.repo_input.candidates.clear(); self.repo_input.picker = None; @@ -61,6 +63,7 @@ impl Workspace { .to_string_lossy() .to_string(); self.repo_input.active = false; + self.repo_input.focus_flash_started = None; self.repo_input.buf.clear(); self.repo_input.candidates.clear(); self.repo_input.picker = None; @@ -68,6 +71,12 @@ impl Workspace { RepoInputResult::Open(resolved) } + /// Advance the short focus cue without keeping a frame clock alive after + /// its two pulses have completed. + pub(crate) fn advance_repo_input_focus_flash(&mut self, now: std::time::Instant) -> bool { + self.repo_input.advance_focus_flash_at(now) + } + /// Extend the path from disk and offer what it could still become. Bound to /// Tab — the one field where a path is typed blind. pub fn repo_input_complete(&mut self) { diff --git a/src/workspace/tests/repo_input_tests.rs b/src/workspace/tests/repo_input_tests.rs index fb6ad3c3..ee0d9b0a 100644 --- a/src/workspace/tests/repo_input_tests.rs +++ b/src/workspace/tests/repo_input_tests.rs @@ -1,6 +1,7 @@ use super::common::*; use super::*; use crate::app::tests::app_with_files; +use std::time::Duration; #[test] fn typing_extends_the_prefilled_repo_path() { @@ -184,3 +185,41 @@ fn 새_workspace는_프로젝트_하나를_활성으로_갖는다() { assert_eq!(ws.projects().len(), 1); assert_eq!(ws.active().unwrap().git.repo_path, "."); } + +#[test] +fn repo_input_focus_flash_pulses_twice_then_stays_dim() { + let mut ws = workspace_on(&["/repos/current"]); + ws.start_repo_input(); + let started = ws + .repo_input + .focus_flash_started + .expect("opening the dialog starts the focus cue"); + let phase = crate::ui::status_view::REPO_INPUT_FLASH_PHASE; + + assert!(ws.repo_input.focus_flash_bright()); + for expected_bright in [false, true, false] { + let now = started + phase * (ws.repo_input.focus_flash_phase as u32 + 1); + assert!(ws.advance_repo_input_focus_flash(now)); + assert_eq!(ws.repo_input.focus_flash_bright(), expected_bright); + } + + let finished = started + phase * 4; + assert!(ws.advance_repo_input_focus_flash(finished)); + assert!(!ws.repo_input.focus_flash_bright()); + assert!(ws.repo_input.focus_flash_started.is_none()); + assert!(!ws.advance_repo_input_focus_flash(finished + Duration::from_secs(1))); +} + +#[test] +fn reopening_repo_input_restarts_the_focus_flash() { + let mut ws = workspace_on(&["/repos/current"]); + ws.start_repo_input(); + let first = ws.repo_input.focus_flash_started.expect("first start"); + ws.advance_repo_input_focus_flash(first + crate::ui::status_view::REPO_INPUT_FLASH_PHASE * 4); + + ws.start_repo_input(); + + assert!(ws.repo_input.focus_flash_started.is_some()); + assert!(ws.repo_input.focus_flash_bright()); + assert!(ws.repo_input.focus_flash_started.unwrap() >= first); +}