From 1b87a061d8fa047eac2bc1dc93e60cd2489e477b Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Fri, 4 Mar 2022 18:55:29 -0500 Subject: remove the mutex around exit_info it is small enough that copying it is not really a big deal --- src/shell/event.rs | 12 +-- src/shell/history/entry.rs | 229 ++++++++++++++++++++++++++++++++++++--------- src/shell/history/job.rs | 148 +---------------------------- src/shell/history/mod.rs | 21 ++--- src/shell/mod.rs | 3 +- 5 files changed, 199 insertions(+), 214 deletions(-) (limited to 'src') diff --git a/src/shell/event.rs b/src/shell/event.rs index fe96d5b..dc58e6f 100644 --- a/src/shell/event.rs +++ b/src/shell/event.rs @@ -7,7 +7,7 @@ pub enum Event { PtyOutput, ChildRunPipeline(usize, (usize, usize)), ChildSuspend(usize), - ChildExit(usize, Option), + ChildExit(usize, super::history::ExitInfo, Option), GitInfo(Option), ClockTimer, } @@ -94,7 +94,7 @@ struct Pending { pty_output: bool, child_run_pipeline: std::collections::VecDeque<(usize, (usize, usize))>, child_suspend: std::collections::VecDeque, - child_exit: Option<(usize, Option)>, + child_exit: Option<(usize, super::history::ExitInfo, Option)>, git_info: Option>, clock_timer: bool, done: bool, @@ -121,8 +121,8 @@ impl Pending { if let Some(idx) = self.child_suspend.pop_front() { return Some(Some(Event::ChildSuspend(idx))); } - if let Some((idx, env)) = self.child_exit.take() { - return Some(Some(Event::ChildExit(idx, env))); + if let Some((idx, exit_info, env)) = self.child_exit.take() { + return Some(Some(Event::ChildExit(idx, exit_info, env))); } if let Some(info) = self.git_info.take() { return Some(Some(Event::GitInfo(info))); @@ -152,8 +152,8 @@ impl Pending { Some(Event::ChildSuspend(idx)) => { self.child_suspend.push_back(idx); } - Some(Event::ChildExit(idx, env)) => { - self.child_exit = Some((idx, env)); + Some(Event::ChildExit(idx, exit_info, env)) => { + self.child_exit = Some((idx, exit_info, env)); } Some(Event::GitInfo(info)) => self.git_info = Some(info), Some(Event::ClockTimer) => self.clock_timer = true, diff --git a/src/shell/history/entry.rs b/src/shell/history/entry.rs index 24b90cf..80e7dbe 100644 --- a/src/shell/history/entry.rs +++ b/src/shell/history/entry.rs @@ -4,8 +4,10 @@ pub struct Entry { cmdline: String, env: Env, pty: super::pty::Pty, - job: super::job::Job, fullscreen: Option, + start_instant: std::time::Instant, + start_time: time::OffsetDateTime, + state: State, } impl Entry { @@ -14,17 +16,22 @@ impl Entry { env: Env, size: (u16, u16), event_w: crate::shell::event::Writer, - ) -> Self { + ) -> Result { + let start_instant = std::time::Instant::now(); + let start_time = time::OffsetDateTime::now_utc(); + let (pty, pts) = super::pty::Pty::new(size, event_w.clone()).unwrap(); - let job = super::job::Job::new(&cmdline, env.clone(), &pts, event_w) - .unwrap(); - Self { + let (child, fh) = Self::spawn_command(&cmdline, &env, &pts)?; + tokio::spawn(Self::task(child, fh, env.idx(), event_w)); + Ok(Self { cmdline, env, pty, - job, fullscreen: None, - } + start_instant, + start_time, + state: State::Running((0, 0)), + }) } pub fn render( @@ -32,38 +39,33 @@ impl Entry { out: &mut impl textmode::Textmode, idx: usize, entry_count: usize, - state: &super::job::State, vt: &mut super::pty::Vt, size: (u16, u16), focused: bool, scrolling: bool, offset: time::UtcOffset, ) { - let time = state.exit_info().map_or_else( + let time = self.state.exit_info().map_or_else( || { format!( "[{}]", - crate::format::time( - self.job.start_time().to_offset(offset) - ) + crate::format::time(self.start_time.to_offset(offset)) ) }, |info| { format!( "({}) [{}]", crate::format::duration( - *info.instant() - *self.job.start_instant() - ), - crate::format::time( - self.job.start_time().to_offset(offset) + info.instant - self.start_instant ), + crate::format::time(self.start_time.to_offset(offset)), ) }, ); vt.bell(out, focused); - set_bgcolor(out, idx, focused); + Self::set_bgcolor(out, idx, focused); out.set_fgcolor(textmode::color::YELLOW); let entry_count_width = format!("{}", entry_count + 1).len(); let idx_str = format!("{}", idx + 1); @@ -72,17 +74,16 @@ impl Entry { out.write_str(" "); out.reset_attributes(); - set_bgcolor(out, idx, focused); - if let Some(info) = state.exit_info() { - let status = info.status(); - if status.signal().is_some() { + Self::set_bgcolor(out, idx, focused); + if let Some(info) = self.state.exit_info() { + if info.status.signal().is_some() { out.set_fgcolor(textmode::color::MAGENTA); - } else if status.success() { + } else if info.status.success() { out.set_fgcolor(textmode::color::DARKGREY); } else { out.set_fgcolor(textmode::color::RED); } - out.write_str(&crate::format::exit_status(status)); + out.write_str(&crate::format::exit_status(info.status)); } else { out.write_str(" "); } @@ -91,10 +92,10 @@ impl Entry { if vt.is_bell() { out.set_bgcolor(textmode::Color::Rgb(64, 16, 16)); } else { - set_bgcolor(out, idx, focused); + Self::set_bgcolor(out, idx, focused); } out.write_str("$ "); - set_bgcolor(out, idx, focused); + Self::set_bgcolor(out, idx, focused); let start = usize::from(out.screen().cursor_position().1); let end = usize::from(size.1) - time.len() - 2; let max_len = end - start; @@ -103,7 +104,7 @@ impl Entry { } else { self.cmd() }; - if let super::job::State::Running(span) = state { + if let State::Running(span) = self.state { let span = (span.0.min(cmd.len()), span.1.min(cmd.len())); if !cmd[..span.0].is_empty() { out.write_str(&cmd[..span.0]); @@ -111,7 +112,7 @@ impl Entry { if !cmd[span.0..span.1].is_empty() { out.set_bgcolor(textmode::Color::Rgb(16, 64, 16)); out.write_str(&cmd[span.0..span.1]); - set_bgcolor(out, idx, focused); + Self::set_bgcolor(out, idx, focused); } if !cmd[span.1..].is_empty() { out.write_str(&cmd[span.1..]); @@ -120,13 +121,13 @@ impl Entry { out.write_str(cmd); } if self.cmd().len() > max_len { - if let super::job::State::Running(span) = state { + if let State::Running(span) = self.state { if span.0 < cmd.len() && span.1 > cmd.len() { out.set_bgcolor(textmode::Color::Rgb(16, 64, 16)); } } out.write_str(" "); - if let super::job::State::Running(span) = state { + if let State::Running(span) = self.state { if span.1 > cmd.len() { out.set_bgcolor(textmode::Color::Rgb(16, 64, 16)); } @@ -136,7 +137,7 @@ impl Entry { } out.reset_attributes(); - set_bgcolor(out, idx, focused); + Self::set_bgcolor(out, idx, focused); let cur_pos = out.screen().cursor_position(); out.write_str(&" ".repeat( usize::from(size.1) - time.len() - 1 - usize::from(cur_pos.1), @@ -157,7 +158,7 @@ impl Entry { out.hide_cursor(true); } else { let last_row = - vt.output_lines(focused && !scrolling, state.running()); + vt.output_lines(focused && !scrolling, self.state.running()); let mut max_lines = self.max_lines(entry_count); if last_row > max_lines { out.write(b"\r\n"); @@ -235,7 +236,11 @@ impl Entry { } pub fn running(&self) -> bool { - self.job.running() + self.state.running() + } + + pub fn exited(&mut self, exit_info: ExitInfo) { + self.state = State::Exited(exit_info); } pub fn lines(&self, entry_count: usize, focused: bool) -> usize { @@ -254,12 +259,10 @@ impl Entry { self.pty.lock_vt() } - pub fn lock_state(&self) -> std::sync::MutexGuard { - self.job.lock_state() - } - - pub fn set_span(&self, span: (usize, usize)) { - self.job.set_span(span); + pub fn set_span(&mut self, new_span: (usize, usize)) { + if let State::Running(ref mut span) = self.state { + *span = new_span; + } } fn max_lines(&self, entry_count: usize) -> usize { @@ -269,14 +272,150 @@ impl Entry { 5 } } + + fn set_bgcolor( + out: &mut impl textmode::Textmode, + idx: usize, + focus: bool, + ) { + if focus { + out.set_bgcolor(textmode::Color::Rgb(0x56, 0x1b, 0x8b)); + } else if idx % 2 == 0 { + out.set_bgcolor(textmode::Color::Rgb(0x24, 0x21, 0x00)); + } else { + out.set_bgcolor(textmode::Color::Rgb(0x20, 0x20, 0x20)); + } + } + + fn spawn_command( + cmdline: &str, + env: &Env, + pts: &pty_process::Pts, + ) -> Result<(tokio::process::Child, std::fs::File)> { + let mut cmd = pty_process::Command::new(std::env::current_exe()?); + cmd.args(&["-c", cmdline, "--status-fd", "3"]); + env.apply(&mut cmd); + let (from_r, from_w) = + nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC)?; + // Safety: from_r was just opened above and is not used anywhere else + let fh = unsafe { std::fs::File::from_raw_fd(from_r) }; + // Safety: dup2 is an async-signal-safe function + unsafe { + cmd.pre_exec(move || { + nix::unistd::dup2(from_w, 3)?; + Ok(()) + }); + } + let child = cmd.spawn(pts)?; + nix::unistd::close(from_w)?; + Ok((child, fh)) + } + + async fn task( + mut child: tokio::process::Child, + fh: std::fs::File, + idx: usize, + event_w: crate::shell::event::Writer, + ) { + enum Res { + Read(crate::runner::Event), + Exit(std::io::Result), + } + + let (read_w, read_r) = tokio::sync::mpsc::unbounded_channel(); + tokio::task::spawn_blocking(move || loop { + let event = bincode::deserialize_from(&fh); + match event { + Ok(event) => { + read_w.send(event).unwrap(); + } + Err(e) => { + match &*e { + bincode::ErrorKind::Io(io_e) => { + assert!( + io_e.kind() + == std::io::ErrorKind::UnexpectedEof + ); + } + e => { + panic!("{}", e); + } + } + break; + } + } + }); + + let mut stream: futures_util::stream::SelectAll<_> = [ + tokio_stream::wrappers::UnboundedReceiverStream::new(read_r) + .map(Res::Read) + .boxed(), + futures_util::stream::once(child.wait()) + .map(Res::Exit) + .boxed(), + ] + .into_iter() + .collect(); + let mut exit_status = None; + let mut new_env = None; + while let Some(res) = stream.next().await { + match res { + Res::Read(event) => match event { + crate::runner::Event::RunPipeline(new_span) => { + // we could just update the span in place here, but we + // do this as an event so that we can also trigger a + // refresh + event_w.send(Event::ChildRunPipeline(idx, new_span)); + } + crate::runner::Event::Suspend => { + event_w.send(Event::ChildSuspend(idx)); + } + crate::runner::Event::Exit(env) => { + new_env = Some(env); + } + }, + Res::Exit(status) => { + exit_status = Some(status.unwrap()); + } + } + } + event_w.send(Event::ChildExit( + idx, + ExitInfo::new(exit_status.unwrap()), + new_env, + )); + } } -fn set_bgcolor(out: &mut impl textmode::Textmode, idx: usize, focus: bool) { - if focus { - out.set_bgcolor(textmode::Color::Rgb(0x56, 0x1b, 0x8b)); - } else if idx % 2 == 0 { - out.set_bgcolor(textmode::Color::Rgb(0x24, 0x21, 0x00)); - } else { - out.set_bgcolor(textmode::Color::Rgb(0x20, 0x20, 0x20)); +enum State { + Running((usize, usize)), + Exited(ExitInfo), +} + +impl State { + fn exit_info(&self) -> Option<&ExitInfo> { + match self { + Self::Running(_) => None, + Self::Exited(exit_info) => Some(exit_info), + } + } + + fn running(&self) -> bool { + self.exit_info().is_none() + } +} + +#[derive(Debug)] +pub struct ExitInfo { + status: std::process::ExitStatus, + instant: std::time::Instant, +} + +impl ExitInfo { + fn new(status: std::process::ExitStatus) -> Self { + Self { + status, + instant: std::time::Instant::now(), + } } } diff --git a/src/shell/history/job.rs b/src/shell/history/job.rs index d3d112a..5c73f4d 100644 --- a/src/shell/history/job.rs +++ b/src/shell/history/job.rs @@ -1,10 +1,6 @@ use crate::shell::prelude::*; -pub struct Job { - state: std::sync::Arc>, - start_time: time::OffsetDateTime, - start_instant: std::time::Instant, -} +pub struct Job {} impl Job { pub fn new( @@ -13,8 +9,6 @@ impl Job { pts: &pty_process::Pts, event_w: crate::shell::event::Writer, ) -> Result { - let start_time = time::OffsetDateTime::now_utc(); - let start_instant = std::time::Instant::now(); let (child, fh) = spawn_command(cmdline, &env, pts)?; let state = std::sync::Arc::new(std::sync::Mutex::new( State::Running((0, 0)), @@ -66,144 +60,4 @@ impl Job { } }); } - - async fn task( - mut child: tokio::process::Child, - fh: std::fs::File, - state: std::sync::Arc>, - env: Env, - event_w: crate::shell::event::Writer, - ) { - enum Res { - Read(crate::runner::Event), - Exit(std::io::Result), - } - - let (read_w, read_r) = tokio::sync::mpsc::unbounded_channel(); - tokio::task::spawn_blocking(move || loop { - let event = bincode::deserialize_from(&fh); - match event { - Ok(event) => { - read_w.send(event).unwrap(); - } - Err(e) => { - match &*e { - bincode::ErrorKind::Io(io_e) => { - assert!( - io_e.kind() - == std::io::ErrorKind::UnexpectedEof - ); - } - e => { - panic!("{}", e); - } - } - break; - } - } - }); - - let mut stream: futures_util::stream::SelectAll<_> = [ - tokio_stream::wrappers::UnboundedReceiverStream::new(read_r) - .map(Res::Read) - .boxed(), - futures_util::stream::once(child.wait()) - .map(Res::Exit) - .boxed(), - ] - .into_iter() - .collect(); - let mut exit_status = None; - let mut new_env = None; - while let Some(res) = stream.next().await { - match res { - Res::Read(event) => match event { - crate::runner::Event::RunPipeline(new_span) => { - // we could just update the span in place here, but we - // do this as an event so that we can also trigger a - // refresh - event_w.send(Event::ChildRunPipeline( - env.idx(), - new_span, - )); - } - crate::runner::Event::Suspend => { - event_w.send(Event::ChildSuspend(env.idx())); - } - crate::runner::Event::Exit(env) => { - new_env = Some(env); - } - }, - Res::Exit(status) => { - exit_status = Some(status.unwrap()); - } - } - } - *state.lock().unwrap() = - State::Exited(ExitInfo::new(exit_status.unwrap())); - event_w.send(Event::ChildExit(env.idx(), new_env)); - } -} - -pub enum State { - Running((usize, usize)), - Exited(ExitInfo), -} - -impl State { - pub fn exit_info(&self) -> Option<&ExitInfo> { - match self { - Self::Running(_) => None, - Self::Exited(exit_info) => Some(exit_info), - } - } - - pub fn running(&self) -> bool { - self.exit_info().is_none() - } -} - -pub struct ExitInfo { - status: std::process::ExitStatus, - instant: std::time::Instant, -} - -impl ExitInfo { - fn new(status: std::process::ExitStatus) -> Self { - Self { - status, - instant: std::time::Instant::now(), - } - } - - pub fn status(&self) -> std::process::ExitStatus { - self.status - } - - pub fn instant(&self) -> &std::time::Instant { - &self.instant - } -} - -fn spawn_command( - cmdline: &str, - env: &Env, - pts: &pty_process::Pts, -) -> Result<(tokio::process::Child, std::fs::File)> { - let mut cmd = pty_process::Command::new(std::env::current_exe()?); - cmd.args(&["-c", cmdline, "--status-fd", "3"]); - env.apply(&mut cmd); - let (from_r, from_w) = nix::unistd::pipe2(nix::fcntl::OFlag::O_CLOEXEC)?; - // Safety: from_r was just opened above and is not used anywhere else - let fh = unsafe { std::fs::File::from_raw_fd(from_r) }; - // Safety: dup2 is an async-signal-safe function - unsafe { - cmd.pre_exec(move || { - nix::unistd::dup2(from_w, 3)?; - Ok(()) - }); - } - let child = cmd.spawn(pts)?; - nix::unistd::close(from_w)?; - Ok((child, fh)) } diff --git a/src/shell/history/mod.rs b/src/shell/history/mod.rs index df995e6..8ec9f75 100644 --- a/src/shell/history/mod.rs +++ b/src/shell/history/mod.rs @@ -1,8 +1,7 @@ use crate::shell::prelude::*; mod entry; -pub use entry::Entry; -mod job; +pub use entry::{Entry, ExitInfo}; mod pty; pub struct History { @@ -29,7 +28,7 @@ impl History { offset: time::UtcOffset, ) { let mut cursor = None; - for (idx, used_lines, mut vt, state) in + for (idx, used_lines, mut vt) in self.visible(repl_lines, focus, scrolling).rev() { let focused = focus.map_or(false, |focus| idx == focus); @@ -41,7 +40,6 @@ impl History { out, idx, self.entry_count(), - &*state, &mut *vt, self.size, focused, @@ -83,7 +81,7 @@ impl History { event_w: crate::shell::event::Writer, ) { self.entries - .push(Entry::new(cmdline, env, self.size, event_w)); + .push(Entry::new(cmdline, env, self.size, event_w).unwrap()); } pub fn entry_count(&self) -> usize { @@ -144,7 +142,7 @@ impl History { if used_lines > usize::from(self.size.0) { break; } - iter.add(idx, used_lines, entry.lock_vt(), entry.lock_state()); + iter.add(idx, used_lines, entry.lock_vt()); } iter } @@ -155,7 +153,6 @@ struct VisibleEntries<'a> { usize, usize, std::sync::MutexGuard<'a, pty::Vt>, - std::sync::MutexGuard<'a, job::State>, )>, } @@ -171,20 +168,14 @@ impl<'a> VisibleEntries<'a> { idx: usize, offset: usize, vt: std::sync::MutexGuard<'a, pty::Vt>, - state: std::sync::MutexGuard<'a, job::State>, ) { // push_front because we are adding them in reverse order - self.entries.push_front((idx, offset, vt, state)); + self.entries.push_front((idx, offset, vt)); } } impl<'a> std::iter::Iterator for VisibleEntries<'a> { - type Item = ( - usize, - usize, - std::sync::MutexGuard<'a, pty::Vt>, - std::sync::MutexGuard<'a, job::State>, - ); + type Item = (usize, usize, std::sync::MutexGuard<'a, pty::Vt>); fn next(&mut self) -> Option { self.entries.pop_front() diff --git a/src/shell/mod.rs b/src/shell/mod.rs index 0f42cde..461e359 100644 --- a/src/shell/mod.rs +++ b/src/shell/mod.rs @@ -224,7 +224,8 @@ impl Shell { ); self.scene = self.default_scene(self.focus); } - Event::ChildExit(idx, env) => { + Event::ChildExit(idx, exit_info, env) => { + self.history.entry_mut(idx).exited(exit_info); if self.focus_idx() == Some(idx) { if let Some(env) = env { if self.hide_readline { -- cgit v1.2.3-54-g00ecf