From 3d73fd30a056869b8dfce29f7e31c6d87f207287 Mon Sep 17 00:00:00 2001 From: Jason Jean Date: Tue, 13 May 2025 17:09:39 -0400 Subject: [PATCH] fix(core): make running a single task more transparent (#31163) ## Current Behavior Running a single task is in this uncanny framed state and doesn't exit immediately. image ## Expected Behavior Running a single task isn't framed and exits immediately. This also fixes scrolling in interactive mode based on whether or not the underlying terminal is in application cursor mode or not. ![image](https://github.com/user-attachments/assets/c32c85e7-edd6-4aba-9a15-bc75cc9ee5ba) ## Related Issue(s) Fixes # --------- Co-authored-by: JamesHenry --- packages/nx/src/native/tui/app.rs | 42 ++++-- .../native/tui/components/layout_manager.rs | 52 +------ ...ests__visualize_auto_mode_single_task.snap | 34 ----- .../native/tui/components/terminal_pane.rs | 137 +++++++++++++----- packages/nx/src/native/tui/pty.rs | 57 +++++++- packages/nx/src/native/tui/tui.rs | 5 +- 6 files changed, 197 insertions(+), 130 deletions(-) delete mode 100644 packages/nx/src/native/tui/components/snapshots/nx__native__tui__components__layout_manager__tests__visual_tests__visualize_auto_mode_single_task.snap diff --git a/packages/nx/src/native/tui/app.rs b/packages/nx/src/native/tui/app.rs index c4b061c6cd..c198a5f0eb 100644 --- a/packages/nx/src/native/tui/app.rs +++ b/packages/nx/src/native/tui/app.rs @@ -1,5 +1,5 @@ use color_eyre::eyre::Result; -use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseEventKind}; +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseEvent, MouseEventKind}; use hashbrown::HashSet; use napi::bindgen_prelude::External; use napi::threadsafe_function::{ErrorStrategy, ThreadsafeFunction}; @@ -214,14 +214,22 @@ impl App { return; } - self.begin_exit_countdown() + if self.tasks.len() > 1 { + self.begin_exit_countdown() + } else { + self.quit(); + } + } + + fn quit(&mut self) { + self.quit_at = Some(std::time::Instant::now()); } fn begin_exit_countdown(&mut self) { let countdown_duration = self.tui_config.auto_exit.countdown_seconds(); // If countdown is disabled, exit immediately if countdown_duration.is_none() { - self.quit_at = Some(std::time::Instant::now()); + self.quit(); return; } @@ -708,27 +716,32 @@ impl App { self.user_has_interacted = true; } + if matches!(self.focus, Focus::MultipleOutput(_)) { + self.handle_mouse_event(mouse).ok(); + return Ok(false); + } + match mouse.kind { MouseEventKind::ScrollUp => { - if matches!(self.focus, Focus::MultipleOutput(_)) { + if matches!(self.focus, Focus::TaskList) { + self.dispatch_action(Action::PreviousTask); + } else { self.handle_key_event(KeyEvent::new( KeyCode::Up, KeyModifiers::empty(), )) .ok(); - } else if matches!(self.focus, Focus::TaskList) { - self.dispatch_action(Action::PreviousTask); } } MouseEventKind::ScrollDown => { - if matches!(self.focus, Focus::MultipleOutput(_)) { + if matches!(self.focus, Focus::TaskList) { + self.dispatch_action(Action::NextTask); + } else { self.handle_key_event(KeyEvent::new( KeyCode::Down, KeyModifiers::empty(), )) .ok(); - } else if matches!(self.focus, Focus::TaskList) { - self.dispatch_action(Action::NextTask); } } _ => {} @@ -790,6 +803,7 @@ impl App { } let frame_area = self.frame_area.unwrap(); + let tasks_list_hidden = self.is_task_list_hidden(); let layout_areas = self.layout_areas.as_mut().unwrap(); if self.debug_mode { @@ -970,6 +984,7 @@ impl App { ); let terminal_pane = TerminalPane::new() + .minimal(tasks_list_hidden && self.tasks.len() == 1) .pty_data(terminal_pane_data) .continuous(task.continuous); @@ -1330,6 +1345,15 @@ impl App { let _ = self.handle_pty_resize(); } + fn handle_mouse_event(&mut self, mouse_event: MouseEvent) -> io::Result<()> { + if let Focus::MultipleOutput(pane_idx) = self.focus { + let terminal_pane_data = &mut self.terminal_pane_data[pane_idx]; + terminal_pane_data.handle_mouse_event(mouse_event) + } else { + Ok(()) + } + } + /// Forward key events to the currently focused pane, if any. fn handle_key_event(&mut self, key: KeyEvent) -> io::Result<()> { if let Focus::MultipleOutput(pane_idx) = self.focus { diff --git a/packages/nx/src/native/tui/components/layout_manager.rs b/packages/nx/src/native/tui/components/layout_manager.rs index a574409f12..349164f314 100644 --- a/packages/nx/src/native/tui/components/layout_manager.rs +++ b/packages/nx/src/native/tui/components/layout_manager.rs @@ -138,7 +138,7 @@ impl LayoutManager { self.mode = match self.mode { LayoutMode::Auto => { // If we are in auto mode, we need to figure out our current orientation and set the mode to the opposite. - if self.is_vertical_layout_preferred(area.width, area.height, self.task_count) { + if self.is_vertical_layout_preferred(area.width, area.height) { LayoutMode::Horizontal } else { LayoutMode::Vertical @@ -249,9 +249,7 @@ impl LayoutManager { fn calculate_layout_visible_task_list(&self, area: Rect) -> LayoutAreas { // Determine whether to use vertical or horizontal layout let use_vertical = match self.mode { - LayoutMode::Auto => { - self.is_vertical_layout_preferred(area.width, area.height, self.task_count) - } + LayoutMode::Auto => self.is_vertical_layout_preferred(area.width, area.height), LayoutMode::Vertical => true, LayoutMode::Horizontal => false, }; @@ -401,17 +399,7 @@ impl LayoutManager { /// - Terminal aspect ratio /// - Number of tasks (single task prefers vertical layout) /// - Minimum dimensions requirements - fn is_vertical_layout_preferred( - &self, - terminal_width: u16, - terminal_height: u16, - task_count: usize, - ) -> bool { - // If there's only a single task, prefer vertical layout - if task_count <= 1 { - return true; - } - + fn is_vertical_layout_preferred(&self, terminal_width: u16, terminal_height: u16) -> bool { // Calculate aspect ratio (width/height) let aspect_ratio = terminal_width as f32 / terminal_height as f32; @@ -557,27 +545,6 @@ mod tests { assert_eq!(task_list.height, 100 / 3); } - #[test] - fn test_auto_mode_prefers_vertical_for_single_task() { - let mut layout_manager = LayoutManager::new(5); - let area = create_test_area(200, 60); // Wide terminal that would normally use horizontal - - layout_manager.set_mode(LayoutMode::Auto); - layout_manager.set_task_list_visibility(TaskListVisibility::Visible); - layout_manager.set_pane_arrangement(PaneArrangement::Single); - layout_manager.set_task_count(1); // Single task - - let layout = layout_manager.calculate_layout(area); - assert!(layout.task_list.is_some()); - - // Even though terminal is wide, layout should be vertical for a single task - let task_list = layout.task_list.unwrap(); - assert_eq!(task_list.x, 0); - assert_eq!(task_list.y, 0); - assert_eq!(task_list.width, 200); - assert_eq!(task_list.height, 60 / 3); - } - #[test] fn test_forced_vertical_mode() { let mut layout_manager = LayoutManager::new(5); @@ -840,19 +807,6 @@ mod tests { insta::assert_snapshot!(terminal.backend()); } - /// Visual test for auto mode with single task (should be vertical regardless of terminal size) - #[test] - fn test_visualize_auto_mode_single_task() { - let mut layout_manager = LayoutManager::new(5); - layout_manager.set_mode(LayoutMode::Auto); - layout_manager.set_task_list_visibility(TaskListVisibility::Visible); - layout_manager.set_pane_arrangement(PaneArrangement::Single); - layout_manager.set_task_count(1); - - let terminal = render_layout(120, 30, &layout_manager); - insta::assert_snapshot!(terminal.backend()); - } - /// Visual test for auto mode with tall terminal #[test] fn test_visualize_auto_mode_tall_terminal() { diff --git a/packages/nx/src/native/tui/components/snapshots/nx__native__tui__components__layout_manager__tests__visual_tests__visualize_auto_mode_single_task.snap b/packages/nx/src/native/tui/components/snapshots/nx__native__tui__components__layout_manager__tests__visual_tests__visualize_auto_mode_single_task.snap deleted file mode 100644 index 9d4bee362d..0000000000 --- a/packages/nx/src/native/tui/components/snapshots/nx__native__tui__components__layout_manager__tests__visual_tests__visualize_auto_mode_single_task.snap +++ /dev/null @@ -1,34 +0,0 @@ ---- -source: packages/nx/src/native/tui/components/layout_manager.rs -expression: terminal.backend() ---- -"┌Task List─────────────────────────────────────────────────────────────────────────────────────────────────────────────┐" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"└──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘" -" " -"┌Terminal Pane 1───────────────────────────────────────────────────────────────────────────────────────────────────────┐" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"│ │" -"└──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┘" diff --git a/packages/nx/src/native/tui/components/terminal_pane.rs b/packages/nx/src/native/tui/components/terminal_pane.rs index 9ddd1b7161..ca7b10c7bb 100644 --- a/packages/nx/src/native/tui/components/terminal_pane.rs +++ b/packages/nx/src/native/tui/components/terminal_pane.rs @@ -1,5 +1,5 @@ use arboard::Clipboard; -use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseEvent, MouseEventKind}; use ratatui::{ buffer::Buffer, layout::{Alignment, Rect}, @@ -97,11 +97,8 @@ impl TerminalPaneData { KeyCode::Char(c) => { pty_mut.write_input(c.to_string().as_bytes())?; } - KeyCode::Up => { - pty_mut.write_input(b"\x1b[A")?; - } - KeyCode::Down => { - pty_mut.write_input(b"\x1b[B")?; + KeyCode::Up | KeyCode::Down => { + pty_mut.handle_arrow_keys(key); } KeyCode::Enter => { pty_mut.write_input(b"\r")?; @@ -120,6 +117,26 @@ impl TerminalPaneData { Ok(()) } + pub fn handle_mouse_event(&mut self, event: MouseEvent) -> io::Result<()> { + if let Some(pty) = &mut self.pty { + let mut pty_mut = pty.as_ref().clone(); + if self.is_interactive { + pty_mut.send_mouse_event(event); + } else { + match event.kind { + MouseEventKind::ScrollUp => { + pty_mut.scroll_up(); + } + MouseEventKind::ScrollDown => { + pty_mut.scroll_down(); + } + _ => {} + } + } + } + Ok(()) + } + pub fn set_interactive(&mut self, interactive: bool) { self.is_interactive = interactive; } @@ -171,6 +188,7 @@ impl TerminalPaneState { pub struct TerminalPane<'a> { pty_data: Option<&'a mut TerminalPaneData>, is_continuous: bool, + minimal: bool, } impl<'a> TerminalPane<'a> { @@ -178,6 +196,7 @@ impl<'a> TerminalPane<'a> { Self { pty_data: None, is_continuous: false, + minimal: false, } } @@ -191,6 +210,11 @@ impl<'a> TerminalPane<'a> { self } + pub fn minimal(mut self, minimal: bool) -> Self { + self.minimal = minimal; + self + } + fn get_status_icon(&self, status: TaskStatus) -> Span { match status { TaskStatus::Success @@ -320,34 +344,47 @@ impl<'a> StatefulWidget for TerminalPane<'a> { let status_icon = self.get_status_icon(state.task_status); - let block = Block::default() - .title(Line::from(if state.is_focused { - vec![ - status_icon.clone(), - Span::raw(format!("{} ", state.task_name)) - .style(Style::default().fg(THEME.primary_fg)), - ] + let mut title = vec![]; + + if self.minimal { + title.push(Span::styled( + " NX ", + Style::default().fg(THEME.primary_fg).bold().bg(base_style + .fg + .expect("Base style should have foreground color")), + )); + title.push(Span::raw(" ")); + } else { + title.push(status_icon.clone()); + } + title.push(Span::styled( + format!("{} ", state.task_name), + Style::default().fg(if state.is_focused { + THEME.primary_fg } else { - vec![ - status_icon.clone(), - Span::raw(format!("{} ", state.task_name)) - .style(Style::default().fg(THEME.secondary_fg)), - if state.is_next_tab_target { - let tab_target_text = Span::raw("Press to focus output ") - .remove_modifier(Modifier::DIM); - // In light themes, use the primary fg color for the tab target text to make sure it's clearly visible - if !THEME.is_dark_mode { - tab_target_text.fg(THEME.primary_fg) - } else { - tab_target_text - } - } else { - Span::raw("") - }, - ] - })) + THEME.secondary_fg + }), + )); + + if state.is_next_tab_target { + let tab_target_text = + Span::raw("Press to focus output ").remove_modifier(Modifier::DIM); + // In light themes, use the primary fg color for the tab target text to make sure it's clearly visible + if !THEME.is_dark_mode { + title.push(tab_target_text.fg(THEME.primary_fg)); + } else { + title.push(tab_target_text); + } + } + + let block = Block::default() + .title(title) .title_alignment(Alignment::Left) - .borders(Borders::ALL) + .borders(if self.minimal { + Borders::NONE + } else { + Borders::ALL + }) .border_type(if state.is_focused { BorderType::Thick } else { @@ -485,8 +522,40 @@ impl<'a> StatefulWidget for TerminalPane<'a> { scrollbar.render(safe_area, buf, &mut state.scrollbar_state); } - // Show interactive/readonly status for focused, in progress tasks - if state.task_status == TaskStatus::InProgress && state.is_focused { + // Show instructions to quit in minimal mode if somehow terminal became non-interactive + if self.minimal && !self.is_currently_interactive() { + let top_text = Line::from(vec![ + Span::styled("quit: ", Style::default().fg(THEME.primary_fg)), + Span::styled("q ", Style::default().fg(THEME.info)), + ]); + + let mode_width = top_text + .spans + .iter() + .map(|span| span.content.len()) + .sum::(); + + // Ensure text doesn't extend past safe area + if mode_width as u16 + 3 < safe_area.width { + let top_right_area = Rect { + x: safe_area.x + safe_area.width - mode_width as u16 - 3, + y: safe_area.y, + width: mode_width as u16 + 2, + height: 1, + }; + + Paragraph::new(top_text) + .alignment(Alignment::Right) + .style(border_style) + .render(top_right_area, buf); + } + + // Show interactive/readonly status for focused, in progress tasks, when not in minimal mode + } else if state.task_status == TaskStatus::InProgress + && state.is_focused + && pty_data.can_be_interactive + && !self.minimal + { // Bottom right status let bottom_text = if self.is_currently_interactive() { Line::from(vec![ diff --git a/packages/nx/src/native/tui/pty.rs b/packages/nx/src/native/tui/pty.rs index 6f9e6022ee..a675628862 100644 --- a/packages/nx/src/native/tui/pty.rs +++ b/packages/nx/src/native/tui/pty.rs @@ -1,11 +1,12 @@ +use super::utils::normalize_newlines; +use crossterm::event::{KeyCode, KeyEvent, MouseEvent, MouseEventKind}; use std::{ io::{self, Write}, sync::{Arc, Mutex, RwLock}, }; +use tracing::debug; use vt100_ctt::Parser; -use super::utils::normalize_newlines; - /// A wrapper that provides access to the terminal screen without cloning /// /// This struct uses a read lock guard internally to maintain the lock on the parser while @@ -93,6 +94,58 @@ impl PtyInstance { Ok(()) } + pub fn handle_arrow_keys(&mut self, event: KeyEvent) { + let alternative_screen = self.parser.read().unwrap().screen().alternate_screen(); + debug!("Alternate Screen: {:?}", alternative_screen); + if !alternative_screen { + match event.code { + KeyCode::Up => { + self.scroll_up(); + } + KeyCode::Down => { + self.scroll_down(); + } + _ => {} + } + } else { + match event.code { + KeyCode::Up => { + self.write_input(b"\x1b[A").ok(); + } + KeyCode::Down => { + self.write_input(b"\x1b[B").ok(); + } + _ => {} + } + } + } + + pub fn send_mouse_event(&mut self, event: MouseEvent) { + let alternative_screen = self.parser.read().unwrap().screen().alternate_screen(); + debug!("Alternate Screen: {:?}", alternative_screen); + if !alternative_screen { + match event.kind { + MouseEventKind::ScrollUp => { + self.scroll_up(); + } + MouseEventKind::ScrollDown => { + self.scroll_down(); + } + _ => {} + } + } else { + match event.kind { + MouseEventKind::ScrollUp => { + self.write_input(b"\x1b[A").ok(); + } + MouseEventKind::ScrollDown => { + self.write_input(b"\x1b[B").ok(); + } + _ => {} + } + } + } + pub fn get_screen(&self) -> Option { self.parser .read() diff --git a/packages/nx/src/native/tui/tui.rs b/packages/nx/src/native/tui/tui.rs index 75a4f5841e..a2356eb53c 100644 --- a/packages/nx/src/native/tui/tui.rs +++ b/packages/nx/src/native/tui/tui.rs @@ -3,6 +3,7 @@ use color_eyre::eyre::Result; use crossterm::{ cursor, event::{Event as CrosstermEvent, KeyEvent, KeyEventKind, MouseEvent}, + execute, terminal::{EnterAlternateScreen, LeaveAlternateScreen}, }; use futures::{FutureExt, StreamExt}; @@ -170,7 +171,7 @@ impl Tui { let _ = THEME.is_dark_mode; debug!("Enabling Raw Mode"); crossterm::terminal::enable_raw_mode()?; - crossterm::execute!(std::io::stderr(), EnterAlternateScreen, cursor::Hide)?; + execute!(std::io::stderr(), EnterAlternateScreen, cursor::Hide)?; self.start(); Ok(()) } @@ -179,7 +180,7 @@ impl Tui { self.stop()?; if crossterm::terminal::is_raw_mode_enabled()? { self.flush()?; - crossterm::execute!(std::io::stderr(), LeaveAlternateScreen, cursor::Show)?; + execute!(std::io::stderr(), LeaveAlternateScreen, cursor::Show)?; crossterm::terminal::disable_raw_mode()?; } Ok(())