From d7e4ec65db028f1ea231d4bee689be394f95cbf5 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Thu, 30 Sep 2021 10:25:48 +0200 Subject: [PATCH] hotfix(stdin): poll for mouse hold in the stdin thread (#752) * hotfix(stdin): poll for mouse hold in the stdin thread * add missing dont panic * style(fmt): make rustfmt happy --- Cargo.lock | 12 ++--- src/tests/e2e/cases.rs | 2 + src/tests/e2e/remote_runner.rs | 45 ++++++++++++++++++- zellij-client/src/input_handler.rs | 2 - zellij-client/src/lib.rs | 31 ++++++++++++- zellij-client/src/os_input_output.rs | 19 +++----- zellij-client/src/unit/input_handler_tests.rs | 9 +++- zellij-utils/src/errors.rs | 4 +- 8 files changed, 96 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27d84465..45641314 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2716,7 +2716,7 @@ dependencies = [ [[package]] name = "zellij" -version = "0.18.0" +version = "0.19.0" dependencies = [ "insta", "log", @@ -2730,7 +2730,7 @@ dependencies = [ [[package]] name = "zellij-client" -version = "0.18.0" +version = "0.19.0" dependencies = [ "insta", "log", @@ -2742,7 +2742,7 @@ dependencies = [ [[package]] name = "zellij-server" -version = "0.18.0" +version = "0.19.0" dependencies = [ "ansi_term 0.12.1", "async-trait", @@ -2765,7 +2765,7 @@ dependencies = [ [[package]] name = "zellij-tile" -version = "0.18.0" +version = "0.19.0" dependencies = [ "serde", "serde_json", @@ -2775,14 +2775,14 @@ dependencies = [ [[package]] name = "zellij-tile-utils" -version = "0.18.0" +version = "0.19.0" dependencies = [ "ansi_term 0.12.1", ] [[package]] name = "zellij-utils" -version = "0.18.0" +version = "0.19.0" dependencies = [ "async-std", "backtrace", diff --git a/src/tests/e2e/cases.rs b/src/tests/e2e/cases.rs index 77892463..cdabef81 100644 --- a/src/tests/e2e/cases.rs +++ b/src/tests/e2e/cases.rs @@ -926,6 +926,7 @@ pub fn mirrored_sessions() { // if no test timed out, we break the loop and assert the snapshot let mut first_runner = RemoteRunner::new_with_session_name("mirrored_sessions", fake_win_size, session_name) + .dont_panic() .add_step(Step { name: "Split pane to the right", instruction: |mut remote_terminal: RemoteTerminal| -> bool { @@ -959,6 +960,7 @@ pub fn mirrored_sessions() { let mut second_runner = RemoteRunner::new_existing_session("mirrored_sessions", fake_win_size, session_name) + .dont_panic() .add_step(Step { name: "Make sure session appears correctly", instruction: |remote_terminal: RemoteTerminal| -> bool { diff --git a/src/tests/e2e/remote_runner.rs b/src/tests/e2e/remote_runner.rs index b2d517c4..db5c4685 100644 --- a/src/tests/e2e/remote_runner.rs +++ b/src/tests/e2e/remote_runner.rs @@ -211,6 +211,7 @@ pub struct RemoteRunner { without_frames: bool, session_name: Option, attach_to_existing: bool, + panic_on_no_retries_left: bool, pub test_timed_out: bool, } @@ -247,6 +248,7 @@ impl RemoteRunner { session_name: None, attach_to_existing: false, test_timed_out: false, + panic_on_no_retries_left: true, } } pub fn new_with_session_name( @@ -285,6 +287,7 @@ impl RemoteRunner { session_name: Some(String::from(session_name)), attach_to_existing: false, test_timed_out: false, + panic_on_no_retries_left: true, } } pub fn new_existing_session( @@ -323,6 +326,7 @@ impl RemoteRunner { session_name: Some(String::from(session_name)), attach_to_existing: true, test_timed_out: false, + panic_on_no_retries_left: true, } } pub fn new_without_frames(test_name: &'static str, win_size: Size) -> Self { @@ -357,6 +361,7 @@ impl RemoteRunner { session_name: None, attach_to_existing: false, test_timed_out: false, + panic_on_no_retries_left: true, } } pub fn new_with_layout( @@ -396,8 +401,13 @@ impl RemoteRunner { session_name: None, attach_to_existing: false, test_timed_out: false, + panic_on_no_retries_left: true, } } + pub fn dont_panic(mut self) -> Self { + self.panic_on_no_retries_left = false; + self + } pub fn add_step(mut self, step: Step) -> Self { self.steps.push(step); self @@ -405,6 +415,35 @@ impl RemoteRunner { pub fn replace_steps(&mut self, steps: Vec) { self.steps = steps; } + fn display_informative_error(&mut self) { + let test_name = self.test_name; + let current_step_name = self.currently_running_step.as_ref().cloned(); + match current_step_name { + Some(current_step) => { + let remote_terminal = self.current_remote_terminal_state(); + eprintln!("Timed out waiting for data on the SSH channel for test {}. Was waiting for step: {}", test_name, current_step); + eprintln!("{:?}", remote_terminal); + } + None => { + let remote_terminal = self.current_remote_terminal_state(); + eprintln!("Timed out waiting for data on the SSH channel for test {}. Haven't begun running steps yet.", test_name); + eprintln!("{:?}", remote_terminal); + } + } + } + pub fn get_current_snapshot(&mut self) -> String { + take_snapshot(&mut self.terminal_output) + } + fn current_remote_terminal_state(&mut self) -> RemoteTerminal { + let current_snapshot = self.get_current_snapshot(); + let (cursor_x, cursor_y) = self.terminal_output.cursor_coordinates().unwrap_or((0, 0)); + RemoteTerminal { + cursor_x, + cursor_y, + current_snapshot, + channel: &mut self.channel, + } + } pub fn run_next_step(&mut self) { if let Some(next_step) = self.steps.get(self.current_step_index) { let current_snapshot = take_snapshot(&mut self.terminal_output); @@ -482,11 +521,15 @@ impl RemoteRunner { break; } } - Err(_e) => { + Err(e) => { if self.retries_left > 0 { return self.restart_test(); } self.test_timed_out = true; + if self.panic_on_no_retries_left { + self.display_informative_error(); + panic!("timed out waiting for test: {:?}", e); + } } } } diff --git a/zellij-client/src/input_handler.rs b/zellij-client/src/input_handler.rs index e78f59c1..1b2c0f22 100644 --- a/zellij-client/src/input_handler.rs +++ b/zellij-client/src/input_handler.rs @@ -157,8 +157,6 @@ impl InputHandler { } MouseEvent::Hold(point) => { self.dispatch_action(Action::MouseHold(point)); - self.os_input - .start_action_repeater(Action::MouseHold(point)); } } } diff --git a/zellij-client/src/lib.rs b/zellij-client/src/lib.rs index 92b87e70..06f7e32e 100644 --- a/zellij-client/src/lib.rs +++ b/zellij-client/src/lib.rs @@ -20,7 +20,7 @@ use zellij_utils::{ channels::{self, ChannelWithContext, SenderWithContext}, consts::{SESSION_NAME, ZELLIJ_IPC_PIPE}, errors::{ClientContext, ContextType, ErrorInstruction}, - input::{actions::Action, config::Config, options::Options}, + input::{actions::Action, config::Config, mouse::MouseEvent, options::Options}, ipc::{ClientAttributes, ClientToServerMsg, ExitReason, ServerToClientMsg}, termion, }; @@ -197,6 +197,35 @@ pub fn start_client( let stdin_buffer = os_input.read_from_stdin(); for key_result in stdin_buffer.events_and_raw() { let (key_event, raw_bytes) = key_result.unwrap(); + if let termion::event::Event::Mouse(me) = key_event { + let mouse_event = zellij_utils::input::mouse::MouseEvent::from(me); + if let MouseEvent::Hold(_) = mouse_event { + // as long as the user is holding the mouse down (no other stdin, eg. + // MouseRelease) we need to keep sending this instruction to the app, + // because the app itself doesn't have an event loop in the proper + // place + let mut poller = os_input.stdin_poller(); + send_input_instructions + .send(InputInstruction::KeyEvent( + key_event.clone(), + raw_bytes.clone(), + )) + .unwrap(); + loop { + let ready = poller.ready(); + if ready { + break; + } + send_input_instructions + .send(InputInstruction::KeyEvent( + key_event.clone(), + raw_bytes.clone(), + )) + .unwrap(); + } + continue; + } + } send_input_instructions .send(InputInstruction::KeyEvent(key_event, raw_bytes)) .unwrap(); diff --git a/zellij-client/src/os_input_output.rs b/zellij-client/src/os_input_output.rs index b5018c29..c2fdad82 100644 --- a/zellij-client/src/os_input_output.rs +++ b/zellij-client/src/os_input_output.rs @@ -1,4 +1,3 @@ -use zellij_utils::input::actions::Action; use zellij_utils::pane_size::Size; use zellij_utils::{interprocess, libc, nix, signal_hook, termion, zellij_tile}; @@ -97,7 +96,7 @@ pub trait ClientOsApi: Send + Sync { fn enable_mouse(&self); fn disable_mouse(&self); // Repeatedly send action, until stdin is readable again - fn start_action_repeater(&mut self, action: Action); + fn stdin_poller(&self) -> StdinPoller; } impl ClientOsApi for ClientOsInputOutput { @@ -204,16 +203,8 @@ impl ClientOsApi for ClientOsInputOutput { } } - fn start_action_repeater(&mut self, action: Action) { - let mut poller = StdinPoller::default(); - - loop { - let ready = poller.ready(); - if ready { - break; - } - self.send_to_server(ClientToServerMsg::Action(action.clone())); - } + fn stdin_poller(&self) -> StdinPoller { + StdinPoller::default() } } @@ -237,7 +228,7 @@ pub fn get_client_os_input() -> Result { pub const DEFAULT_STDIN_POLL_TIMEOUT_MS: u64 = 10; -struct StdinPoller { +pub struct StdinPoller { poll: Poll, events: Events, timeout: time::Duration, @@ -245,7 +236,7 @@ struct StdinPoller { impl StdinPoller { // use mio poll to check if stdin is readable without blocking - fn ready(&mut self) -> bool { + pub fn ready(&mut self) -> bool { self.poll .poll(&mut self.events, Some(self.timeout)) .expect("could not poll stdin for readiness"); diff --git a/zellij-client/src/unit/input_handler_tests.rs b/zellij-client/src/unit/input_handler_tests.rs index c2cf784b..f4beb53a 100644 --- a/zellij-client/src/unit/input_handler_tests.rs +++ b/zellij-client/src/unit/input_handler_tests.rs @@ -8,7 +8,10 @@ use zellij_utils::termion::event::Key; use zellij_utils::zellij_tile::data::Palette; use crate::InputInstruction; -use crate::{os_input_output::ClientOsApi, ClientInstruction, CommandIsExecuting}; +use crate::{ + os_input_output::{ClientOsApi, StdinPoller}, + ClientInstruction, CommandIsExecuting, +}; use std::path::Path; @@ -133,7 +136,9 @@ impl ClientOsApi for FakeClientOsApi { } fn enable_mouse(&self) {} fn disable_mouse(&self) {} - fn start_action_repeater(&mut self, _action: Action) {} + fn stdin_poller(&self) -> StdinPoller { + unimplemented!() + } } fn extract_actions_sent_to_server( diff --git a/zellij-utils/src/errors.rs b/zellij-utils/src/errors.rs index dc9a9789..cb4a4c26 100644 --- a/zellij-utils/src/errors.rs +++ b/zellij-utils/src/errors.rs @@ -104,7 +104,7 @@ pub fn get_current_ctx() -> ErrorContext { } /// A representation of the call stack. -#[derive(Clone, Copy, Serialize, Deserialize)] +#[derive(Clone, Copy, Serialize, Deserialize, Debug)] pub struct ErrorContext { calls: [ContextType; MAX_THREAD_CALL_STACK], } @@ -161,7 +161,7 @@ impl Display for ErrorContext { /// Complex variants store a variant of a related enum, whose variants can be built from /// the corresponding Zellij MSPC instruction enum variants ([`ScreenInstruction`], /// [`PtyInstruction`], [`ClientInstruction`], etc). -#[derive(Copy, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Copy, Clone, PartialEq, Serialize, Deserialize, Debug)] pub enum ContextType { /// A screen-related call. Screen(ScreenContext),