diff options
author | Aram Drevekenin <aram@poor.dev> | 2022-12-14 22:26:48 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-12-14 22:26:48 +0100 |
commit | c3115a428ed5c990cc5ead5629dabb624ae90453 (patch) | |
tree | fe1a3c0f344b6d1056789861d6d28ca5196f838d | |
parent | 177cd20beaf7a89d54b295f1aab498b7ab2c04c1 (diff) |
fix(panes): show visual error when unable to split panes vertically/horizontally (#2025)
* fix(panes): show visual error when failing to split pane vertically/horizontally
* fix: lockfile
-rw-r--r-- | Cargo.lock | 12 | ||||
-rw-r--r-- | zellij-server/src/background_jobs.rs | 79 | ||||
-rw-r--r-- | zellij-server/src/lib.rs | 34 | ||||
-rw-r--r-- | zellij-server/src/panes/plugin_pane.rs | 26 | ||||
-rw-r--r-- | zellij-server/src/panes/terminal_pane.rs | 26 | ||||
-rw-r--r-- | zellij-server/src/panes/tiled_panes/mod.rs | 39 | ||||
-rw-r--r-- | zellij-server/src/screen.rs | 28 | ||||
-rw-r--r-- | zellij-server/src/tab/mod.rs | 80 | ||||
-rw-r--r-- | zellij-server/src/tab/unit/tab_tests.rs | 84 | ||||
-rw-r--r-- | zellij-server/src/thread_bus.rs | 26 | ||||
-rw-r--r-- | zellij-server/src/ui/pane_boundaries_frame.rs | 3 | ||||
-rw-r--r-- | zellij-server/src/ui/pane_contents_and_ui.rs | 4 | ||||
-rw-r--r-- | zellij-server/src/unit/screen_tests.rs | 15 | ||||
-rw-r--r-- | zellij-utils/src/errors.rs | 10 | ||||
-rw-r--r-- | zellij-utils/src/pane_size.rs | 2 |
15 files changed, 449 insertions, 19 deletions
diff --git a/Cargo.lock b/Cargo.lock index 72e0cfc5a..8d96ef7b8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3889,7 +3889,7 @@ dependencies = [ [[package]] name = "zellij" -version = "0.34.4" +version = "0.34.5" dependencies = [ "anyhow", "dialoguer", @@ -3908,7 +3908,7 @@ dependencies = [ [[package]] name = "zellij-client" -version = "0.34.4" +version = "0.34.5" dependencies = [ "insta", "log", @@ -3921,7 +3921,7 @@ dependencies = [ [[package]] name = "zellij-server" -version = "0.34.4" +version = "0.34.5" dependencies = [ "ansi_term", "arrayvec 0.7.2", @@ -3951,7 +3951,7 @@ dependencies = [ [[package]] name = "zellij-tile" -version = "0.34.4" +version = "0.34.5" dependencies = [ "clap", "serde", @@ -3963,14 +3963,14 @@ dependencies = [ [[package]] name = "zellij-tile-utils" -version = "0.34.4" +version = "0.34.5" dependencies = [ "ansi_term", ] [[package]] name = "zellij-utils" -version = "0.34.4" +version = "0.34.5" dependencies = [ "anyhow", "async-std", diff --git a/zellij-server/src/background_jobs.rs b/zellij-server/src/background_jobs.rs new file mode 100644 index 000000000..3c89ed064 --- /dev/null +++ b/zellij-server/src/background_jobs.rs @@ -0,0 +1,79 @@ +use zellij_utils::async_std::task; +use zellij_utils::errors::{prelude::*, BackgroundJobContext, ContextType}; + +use std::collections::HashMap; +use std::time::{Duration, Instant}; + +use crate::panes::PaneId; +use crate::screen::ScreenInstruction; +use crate::thread_bus::Bus; + +#[derive(Debug, Clone, Eq, PartialEq, Hash)] +pub enum BackgroundJob { + DisplayPaneError(PaneId, String), + Exit, +} + +impl From<&BackgroundJob> for BackgroundJobContext { + fn from(background_job: &BackgroundJob) -> Self { + match *background_job { + BackgroundJob::DisplayPaneError(..) => BackgroundJobContext::DisplayPaneError, + BackgroundJob::Exit => BackgroundJobContext::Exit, + } + } +} + +static FLASH_DURATION_MS: u64 = 1000; + +pub(crate) fn background_jobs_main(bus: Bus<BackgroundJob>) -> Result<()> { + let err_context = || "failed to write to pty".to_string(); + let mut running_jobs: HashMap<BackgroundJob, Instant> = HashMap::new(); + + loop { + let (event, mut err_ctx) = bus.recv().with_context(err_context)?; + err_ctx.add_call(ContextType::BackgroundJob((&event).into())); + let job = event.clone(); + match event { + BackgroundJob::DisplayPaneError(pane_id, text) => { + if job_already_running(job, &mut running_jobs) { + continue; + } + task::spawn({ + let senders = bus.senders.clone(); + async move { + let _ = senders.send_to_screen( + ScreenInstruction::AddRedPaneFrameColorOverride(pane_id, Some(text)), + ); + task::sleep(std::time::Duration::from_millis(FLASH_DURATION_MS)).await; + let _ = senders.send_to_screen( + ScreenInstruction::ClearPaneFrameColorOverride(pane_id), + ); + } + }); + }, + BackgroundJob::Exit => { + return Ok(()); + }, + } + } +} + +fn job_already_running( + job: BackgroundJob, + running_jobs: &mut HashMap<BackgroundJob, Instant>, +) -> bool { + match running_jobs.get_mut(&job) { + Some(current_running_job_start_time) => { + if current_running_job_start_time.elapsed() > Duration::from_millis(FLASH_DURATION_MS) { + *current_running_job_start_time = Instant::now(); + false + } else { + true + } + }, + None => { + running_jobs.insert(job.clone(), Instant::now()); + false + }, + } +} diff --git a/zellij-server/src/lib.rs b/zellij-server/src/lib.rs index c024a03d3..40c2edc2f 100644 --- a/zellij-server/src/lib.rs +++ b/zellij-server/src/lib.rs @@ -3,6 +3,7 @@ pub mod output; pub mod panes; pub mod tab; +mod background_jobs; mod logging_pipe; mod plugins; mod pty; @@ -13,6 +14,7 @@ mod terminal_bytes; mod thread_bus; mod ui; +use background_jobs::{background_jobs_main, BackgroundJob}; use log::info; use pty_writer::{pty_writer_main, PtyWriteInstruction}; use std::collections::{HashMap, HashSet}; @@ -110,6 +112,7 @@ pub(crate) struct SessionMetaData { pty_thread: Option<thread::JoinHandle<()>>, plugin_thread: Option<thread::JoinHandle<()>>, pty_writer_thread: Option<thread::JoinHandle<()>>, + background_jobs_thread: Option<thread::JoinHandle<()>>, } impl Drop for SessionMetaData { @@ -118,6 +121,7 @@ impl Drop for SessionMetaData { let _ = self.senders.send_to_screen(ScreenInstruction::Exit); let _ = self.senders.send_to_plugin(PluginInstruction::Exit); let _ = self.senders.send_to_pty_writer(PtyWriteInstruction::Exit); + let _ = self.senders.send_to_background_jobs(BackgroundJob::Exit); if let Some(screen_thread) = self.screen_thread.take() { let _ = screen_thread.join(); } @@ -130,6 +134,9 @@ impl Drop for SessionMetaData { if let Some(pty_writer_thread) = self.pty_writer_thread.take() { let _ = pty_writer_thread.join(); } + if let Some(background_jobs_thread) = self.background_jobs_thread.take() { + let _ = background_jobs_thread.join(); + } } } @@ -638,6 +645,10 @@ fn init_session( channels::unbounded(); let to_pty_writer = SenderWithContext::new(to_pty_writer); + let (to_background_jobs, background_jobs_receiver): ChannelWithContext<BackgroundJob> = + channels::unbounded(); + let to_background_jobs = SenderWithContext::new(to_background_jobs); + // Determine and initialize the data directory let data_dir = opts.data_dir.unwrap_or_else(get_default_data_dir); @@ -664,6 +675,7 @@ fn init_session( Some(&to_plugin), Some(&to_server), Some(&to_pty_writer), + Some(&to_background_jobs), Some(os_input.clone()), ), opts.debug, @@ -684,6 +696,7 @@ fn init_session( Some(&to_plugin), Some(&to_server), Some(&to_pty_writer), + Some(&to_background_jobs), Some(os_input.clone()), ); let max_panes = opts.max_panes; @@ -711,6 +724,7 @@ fn init_session( Some(&to_plugin), None, Some(&to_pty_writer), + Some(&to_background_jobs), None, ); let store = Store::default(); @@ -738,18 +752,37 @@ fn init_session( Some(&to_plugin), Some(&to_server), None, + Some(&to_background_jobs), Some(os_input.clone()), ); || pty_writer_main(pty_writer_bus).fatal() }) .unwrap(); + let background_jobs_thread = thread::Builder::new() + .name("background_jobs".to_string()) + .spawn({ + let background_jobs_bus = Bus::new( + vec![background_jobs_receiver], + Some(&to_screen), + Some(&to_pty), + Some(&to_plugin), + Some(&to_server), + Some(&to_pty_writer), + None, + Some(os_input.clone()), + ); + || background_jobs_main(background_jobs_bus).fatal() + }) + .unwrap(); + SessionMetaData { senders: ThreadSenders { to_screen: Some(to_screen), to_pty: Some(to_pty), to_plugin: Some(to_plugin), to_pty_writer: Some(to_pty_writer), + to_background_jobs: Some(to_background_jobs), to_server: None, should_silently_fail: false, }, @@ -760,5 +793,6 @@ fn init_session( pty_thread: Some(pty_thread), plugin_thread: Some(plugin_thread), pty_writer_thread: Some(pty_writer_thread), + background_jobs_thread: Some(background_jobs_thread), } } diff --git a/zellij-server/src/panes/plugin_pane.rs b/zellij-server/src/panes/plugin_pane.rs index a530b20b2..3a0ec9c83 100644 --- a/zellij-server/src/panes/plugin_pane.rs +++ b/zellij-server/src/panes/plugin_pane.rs @@ -64,6 +64,7 @@ pub(crate) struct PluginPane { prev_pane_name: String, frame: HashMap<ClientId, PaneFrame>, borderless: bool, + pane_frame_color_override: Option<(PaletteColor, Option<String>)>, } impl PluginPane { @@ -102,6 +103,7 @@ impl PluginPane { vte_parsers: HashMap::new(), grids: HashMap::new(), style, + pane_frame_color_override: None, } } } @@ -244,7 +246,13 @@ impl Pane for PluginPane { } if let Some(grid) = self.grids.get(&client_id) { let err_context = || format!("failed to render frame for client {client_id}"); - let pane_title = if self.pane_name.is_empty() + let pane_title = if let Some(text_color_override) = self + .pane_frame_color_override + .as_ref() + .and_then(|(_color, text)| text.as_ref()) + { + text_color_override.into() + } else if self.pane_name.is_empty() && input_mode == InputMode::RenamePane && frame_params.is_main_client { @@ -257,12 +265,15 @@ impl Pane for PluginPane { self.pane_name.clone() }; - let frame = PaneFrame::new( + let mut frame = PaneFrame::new( self.current_geom().into(), grid.scrollback_position_and_length(), pane_title, frame_params, ); + if let Some((frame_color_override, _text)) = self.pane_frame_color_override.as_ref() { + frame.override_color(*frame_color_override); + } let res = match self.frame.get(&client_id) { // TODO: use and_then or something? @@ -469,6 +480,17 @@ impl Pane for PluginPane { )])) .unwrap(); } + fn add_red_pane_frame_color_override(&mut self, error_text: Option<String>) { + self.pane_frame_color_override = Some((self.style.colors.red, error_text)); + } + fn clear_pane_frame_color_override(&mut self) { + self.pane_frame_color_override = None; + } + fn frame_color_override(&self) -> Option<PaletteColor> { + self.pane_frame_color_override + .as_ref() + .map(|(color, _text)| *color) + } } impl PluginPane { diff --git a/zellij-server/src/panes/terminal_pane.rs b/zellij-server/src/panes/terminal_pane.rs index b5c51c659..4d5ed840c 100644 --- a/zellij-server/src/panes/terminal_pane.rs +++ b/zellij-server/src/panes/terminal_pane.rs @@ -109,7 +109,8 @@ pub struct TerminalPane { is_held: Option<(Option<i32>, IsFirstRun, RunCommand)>, // a "held" pane means that its command has either exited and the pane is waiting for a // possible user instruction to be re-run, or that the command has not yet been run banner: Option<String>, // a banner to be rendered inside this TerminalPane, used for panes - // held on startup and can possibly be used to display some errors + // held on startup and can possibly be used to display some errors + pane_frame_color_override: Option<(PaletteColor, Option<String>)>, } impl Pane for TerminalPane { @@ -301,7 +302,13 @@ impl Pane for TerminalPane { ) -> Result<Option<(Vec<CharacterChunk>, Option<String>)>> { let err_context = || format!("failed to render frame for client {client_id}"); // TODO: remove the cursor stuff from here - let pane_title = if self.pane_name.is_empty() + let pane_title = if let Some(text_color_override) = self + .pane_frame_color_override + .as_ref() + .and_then(|(_color, text)| text.as_ref()) + { + text_color_override.into() + } else if self.pane_name.is_empty() && input_mode == InputMode::RenamePane && frame_params.is_main_client { @@ -353,6 +360,9 @@ impl Pane for TerminalPane { frame.add_exit_status(exit_status.as_ref().copied()); } } + if let Some((frame_color_override, _text)) = self.pane_frame_color_override.as_ref() { + frame.override_color(*frame_color_override); + } let res = match self.frame.get(&client_id) { // TODO: use and_then or something? @@ -669,6 +679,17 @@ impl Pane for TerminalPane { } self.set_should_render(true); } + fn add_red_pane_frame_color_override(&mut self, error_text: Option<String>) { + self.pane_frame_color_override = Some((self.style.colors.red, error_text)); + } + fn clear_pane_frame_color_override(&mut self) { + self.pane_frame_color_override = None; + } + fn frame_color_override(&self) -> Option<PaletteColor> { + self.pane_frame_color_override + .as_ref() + .map(|(color, _text)| *color) + } } impl TerminalPane { @@ -717,6 +738,7 @@ impl TerminalPane { search_term: String::new(), is_held: None, banner: None, + pane_frame_color_override: None, } } pub fn get_x(&self) -> usize { diff --git a/zellij-server/src/panes/tiled_panes/mod.rs b/zellij-server/src/panes/tiled_panes/mod.rs index b5caa5916..ee752ea15 100644 --- a/zellij-server/src/panes/tiled_panes/mod.rs +++ b/zellij-server/src/panes/tiled_panes/mod.rs @@ -205,6 +205,19 @@ impl TiledPanes { }) .collect() } + pub fn borderless_pane_geoms(&self) -> Vec<Viewport> { + self.panes + .values() + .filter_map(|p| { + let geom = p.position_and_size(); + if p.borderless() { + Some(geom.into()) + } else { + None + } + }) + .collect() + } pub fn first_selectable_pane_id(&self) -> Option<PaneId> { self.panes .iter() @@ -295,6 +308,19 @@ impl TiledPanes { } false } + pub fn can_split_active_pane_horizontally(&self, client_id: ClientId) -> bool { + let active_pane_id = &self.active_panes.get(&client_id).unwrap(); + let active_pane = self.panes.get(active_pane_id).unwrap(); + let full_pane_size = active_pane.position_and_size(); + if full_pane_size.rows.is_fixed() { + return false; + } + if split(SplitDirection::Horizontal, &full_pane_size).is_some() { + true + } else { + false + } + } pub fn split_pane_horizontally( &mut self, pid: PaneId, @@ -313,6 +339,19 @@ impl TiledPanes { self.relayout(SplitDirection::Vertical); } } + pub fn can_split_active_pane_vertically(&self, client_id: ClientId) -> bool { + let active_pane_id = &self.active_panes.get(&client_id).unwrap(); + let active_pane = self.panes.get(active_pane_id).unwrap(); + let full_pane_size = active_pane.position_and_size(); + if full_pane_size.cols.is_fixed() { + return false; + } + if split(SplitDirection::Vertical, &full_pane_size).is_some() { + true + } else { + false + } + } pub fn split_pane_vertically( &mut self, pid: PaneId, diff --git a/zellij-server/src/screen.rs b/zellij-server/src/screen.rs index f304383be..9ed803bbd 100644 --- a/zellij-server/src/screen.rs +++ b/zellij-server/src/screen.rs @@ -224,6 +224,8 @@ pub enum ScreenInstruction { SearchToggleCaseSensitivity(ClientId), SearchToggleWholeWord(ClientId), SearchToggleWrap(ClientId), + AddRedPaneFrameColorOverride(PaneId, Option<String>), // Option<String> => optional error text + ClearPaneFrameColorOverride(PaneId), } impl From<&ScreenInstruction> for ScreenContext { @@ -355,6 +357,12 @@ impl From<&ScreenInstruction> for ScreenContext { }, ScreenInstruction::SearchToggleWholeWord(..) => ScreenContext::SearchToggleWholeWord, ScreenInstruction::SearchToggleWrap(..) => ScreenContext::SearchToggleWrap, + ScreenInstruction::AddRedPaneFrameColorOverride(..) => { + ScreenContext::AddRedPaneFrameColorOverride + }, + ScreenInstruction::ClearPaneFrameColorOverride(..) => { + ScreenContext::ClearPaneFrameColorOverride + }, } } } @@ -2112,6 +2120,26 @@ pub(crate) fn screen_thread_main( screen.render()?; screen.unblock_input()?; }, + ScreenInstruction::AddRedPaneFrameColorOverride(pane_id, error_text) => { + let all_tabs = screen.get_tabs_mut(); + for tab in all_tabs.values_mut() { + if tab.has_pane_with_pid(&pane_id) { + tab.add_red_pane_frame_color_override(pane_id, error_text); + break; + } + } + screen.render()?; + }, + ScreenInstruction::ClearPaneFrameColorOverride(pane_id) => { + let all_tabs = screen.get_tabs_mut(); + for tab in all_tabs.values_mut() { + if tab.has_pane_with_pid(&pane_id) { + tab.clear_pane_frame_color_override(pane_id); + break; + } + } + screen.render()?; + }, } } Ok(()) diff --git a/zellij-server/src/tab/mod.rs b/zellij-server/src/tab/mod.rs index 63573a08e..a841b39ae 100644 --- a/zellij-server/src/tab/mod.rs +++ b/zellij-server/src/tab/mod.rs @@ -13,6 +13,7 @@ use zellij_utils::input::command::RunCommand; use zellij_utils::position::{Column, Line}; use zellij_utils::{position::Position, serde}; +use crate::background_jobs::BackgroundJob; use crate::pty_writer::PtyWriteInstruction; use crate::screen::CopyOptions; use crate::ui::pane_boundaries_frame::FrameParams; @@ -386,6 +387,9 @@ pub trait Pane { fn hold(&mut self, _exit_status: Option<i32>, _is_first_run: bool, _run_command: RunCommand) { // No-op by default, only terminal panes support holding } + fn add_red_pane_frame_color_override(&mut self, _error_text: Option<String>); + fn clear_pane_frame_color_override(&mut self); + fn frame_color_override(&self) -> Option<PaletteColor>; } #[derive(Clone, Debug)] @@ -618,14 +622,18 @@ impl Tab { .send_to_pty(PtyInstruction::ClosePane(PaneId::Terminal(*unused_pid))) .with_context(err_context)?; } - // FIXME: This is another hack to crop the viewport to fixed-size panes. Once you can have - // non-fixed panes that are part of the viewport, get rid of this! + + // here we offset the viewport from borderless panes that are on the edges of the + // screen, this is so that when we don't have pane boundaries (eg. when they were + // disabled by the user) boundaries won't be drawn around these panes + // geometrically, we can only do this with panes that are on the edges of the + // screen - so it's mostly a best-effort thing let display_area = { let display_area = self.display_area.borrow(); *display_area }; self.resize_whole_tab(display_area); - let boundary_geoms = self.tiled_panes.fixed_pane_geoms(); + let boundary_geoms = self.tiled_panes.borderless_pane_geoms(); for geom in boundary_geoms { self.offset_viewport(&geom) } @@ -1035,6 +1043,20 @@ impl Tab { self.should_clear_display_before_rendering = true; self.tiled_panes.focus_pane(pid, client_id); } + } else { + log::error!("No room to split pane horizontally"); + if let Some(active_pane_id) = self.tiled_panes.get_active_pane_id(client_id) { + self.senders + .send_to_background_jobs(BackgroundJob::DisplayPaneError( + active_pane_id, + "TOO SMALL!".into(), + )) + .with_context(err_context)?; + } + self.senders + .send_to_pty(PtyInstruction::ClosePane(pid)) + .with_context(err_context)?; + return Ok(()); } Ok(()) } @@ -1075,6 +1097,20 @@ impl Tab { self.should_clear_display_before_rendering = true; self.tiled_panes.focus_pane(pid, client_id); } + } else { + log::error!("No room to split pane vertically"); + if let Some(active_pane_id) = self.tiled_panes.get_active_pane_id(client_id) { + self.senders + .send_to_background_jobs(BackgroundJob::DisplayPaneError( + active_pane_id, + "TOO SMALL!".into(), + )) + .with_context(err_context)?; + } + self.senders + .send_to_pty(PtyInstruction::ClosePane(pid)) |