From 7430e9b2e0173dbdd7108ecd1164be3d0df17239 Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Fri, 17 Feb 2023 18:16:38 +0100 Subject: [PATCH 1/2] fix(ux): cache stdin queries on startup --- Cargo.lock | 1 + zellij-client/Cargo.toml | 1 + zellij-client/src/lib.rs | 14 +--- zellij-client/src/stdin_ansi_parser.rs | 54 +++++++++++---- zellij-client/src/stdin_handler.rs | 47 +++++++++---- zellij-client/src/unit/stdin_tests.rs | 95 -------------------------- zellij-server/src/route.rs | 9 +-- 7 files changed, 81 insertions(+), 140 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e782d47764..bc0565f1ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4017,6 +4017,7 @@ dependencies = [ "log", "mio", "serde", + "serde_json", "serde_yaml", "url", "zellij-utils", diff --git a/zellij-client/Cargo.toml b/zellij-client/Cargo.toml index 114257b8e2..f8d6045a10 100644 --- a/zellij-client/Cargo.toml +++ b/zellij-client/Cargo.toml @@ -13,6 +13,7 @@ mio = { version = "0.7.11", features = ['os-ext'] } serde = { version = "1.0", features = ["derive"] } url = { version = "2.2.2", features = ["serde"] } serde_yaml = "0.8" +serde_json = "1.0" zellij-utils = { path = "../zellij-utils/", version = "0.34.5" } log = "0.4.17" diff --git a/zellij-client/src/lib.rs b/zellij-client/src/lib.rs index 51d6c99d63..67a7fb902e 100644 --- a/zellij-client/src/lib.rs +++ b/zellij-client/src/lib.rs @@ -265,16 +265,6 @@ pub fn start_client( os_api.send_to_server(ClientToServerMsg::TerminalResize( os_api.get_terminal_size_using_fd(0), )); - // send a query to the terminal emulator in case the font size changed - // as well - we'll parse the response through STDIN - let terminal_emulator_query_string = stdin_ansi_parser - .lock() - .unwrap() - .window_size_change_query_string(); - let _ = os_api - .get_stdout_writer() - .write(terminal_emulator_query_string.as_bytes()) - .unwrap(); } }), Box::new({ @@ -348,7 +338,7 @@ pub fn start_client( let mut stdout = os_input.get_stdout_writer(); stdout - .write_all("\u{1b}[1mLoading Zellij\u{1b}[m".as_bytes()) + .write_all("\u{1b}[1mLoading Zellij\u{1b}[m\n\r".as_bytes()) .expect("cannot write to stdout"); stdout.flush().expect("could not flush"); @@ -368,7 +358,7 @@ pub fn start_client( match client_instruction { ClientInstruction::StartedParsingStdinQuery => { stdout - .write_all("\n\rQuerying terminal emulator for \u{1b}[32;1mdefault colors\u{1b}[m and \u{1b}[32;1mpixel/cell\u{1b}[m ratio...".as_bytes()) + .write_all("Querying terminal emulator for \u{1b}[32;1mdefault colors\u{1b}[m and \u{1b}[32;1mpixel/cell\u{1b}[m ratio...".as_bytes()) .expect("cannot write to stdout"); stdout.flush().expect("could not flush"); }, diff --git a/zellij-client/src/stdin_ansi_parser.rs b/zellij-client/src/stdin_ansi_parser.rs index 23e869b2ba..53591d7543 100644 --- a/zellij-client/src/stdin_ansi_parser.rs +++ b/zellij-client/src/stdin_ansi_parser.rs @@ -1,11 +1,17 @@ use std::time::{Duration, Instant}; +use zellij_utils::consts::{VERSION, ZELLIJ_CACHE_DIR}; const STARTUP_PARSE_DEADLINE_MS: u64 = 500; -const SIGWINCH_PARSE_DEADLINE_MS: u64 = 200; use zellij_utils::{ ipc::PixelDimensions, lazy_static::lazy_static, pane_size::SizeInPixels, regex::Regex, }; +use serde::{Deserialize, Serialize}; +use std::fs::{File, OpenOptions}; +use std::io::{Read, Write}; +use std::path::PathBuf; +use zellij_utils::anyhow::Result; + #[derive(Debug)] pub struct StdinAnsiParser { raw_buffer: Vec, @@ -43,18 +49,6 @@ impl StdinAnsiParser { Some(Instant::now() + Duration::from_millis(STARTUP_PARSE_DEADLINE_MS)); query_string } - pub fn window_size_change_query_string(&mut self) -> String { - // note that this assumes the String will be sent to the terminal emulator and so starts a - // deadline timeout (self.parse_deadline) - - // [14t => get text area size in pixels, - // [16t => get character cell size in pixels - let query_string = String::from("\u{1b}[14t\u{1b}[16t"); - - self.parse_deadline = - Some(Instant::now() + Duration::from_millis(SIGWINCH_PARSE_DEADLINE_MS)); - query_string - } fn drain_pending_events(&mut self) -> Vec { let mut events = vec![]; events.append(&mut self.pending_events); @@ -82,6 +76,35 @@ impl StdinAnsiParser { } self.drain_pending_events() } + pub fn read_cache(&self) -> Option> { + let path = self.cache_dir_path(); + match OpenOptions::new() + .read(true) + .open(path.as_path()) { + Ok(mut file) => { + let mut json_cache = String::new(); + file.read_to_string(&mut json_cache).ok()?; + let instructions = serde_json::from_str::>(&json_cache).ok()?; + if instructions.is_empty() { + None + } else { + Some(instructions) + } + }, + Err(e) => { + log::error!("Failed to open STDIN cache file: {:?}", e); + None + } + } + } + pub fn write_cache(&self, events: Vec) { + let path = self.cache_dir_path(); + if let Ok(serialized_events) = serde_json::to_string(&events) { + if let Ok(mut file) = File::create(path.as_path()) { + let _ = file.write_all(serialized_events.as_bytes()); + } + }; + } fn parse_byte(&mut self, byte: u8) { if byte == b't' { self.raw_buffer.push(byte); @@ -112,9 +135,12 @@ impl StdinAnsiParser { self.raw_buffer.push(byte); } } + fn cache_dir_path(&self) -> PathBuf { + ZELLIJ_CACHE_DIR.join(&format!("zellij-stdin-cache-v{}", VERSION)) + } } -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub enum AnsiStdinInstruction { PixelDimensions(PixelDimensions), BackgroundColor(String), diff --git a/zellij-client/src/stdin_handler.rs b/zellij-client/src/stdin_handler.rs index 75634a05bc..95cb9d92de 100644 --- a/zellij-client/src/stdin_handler.rs +++ b/zellij-client/src/stdin_handler.rs @@ -27,21 +27,34 @@ pub(crate) fn stdin_loop( let mut holding_mouse = false; let mut input_parser = InputParser::new(); let mut current_buffer = vec![]; - // on startup we send a query to the terminal emulator for stuff like the pixel size and colors - // we get a response through STDIN, so it makes sense to do this here - send_input_instructions - .send(InputInstruction::StartedParsing) - .unwrap(); - let terminal_emulator_query_string = stdin_ansi_parser - .lock() - .unwrap() - .terminal_emulator_query_string(); - let _ = os_input - .get_stdout_writer() - .write(terminal_emulator_query_string.as_bytes()) - .unwrap(); - let query_duration = stdin_ansi_parser.lock().unwrap().startup_query_duration(); - send_done_parsing_after_query_timeout(send_input_instructions.clone(), query_duration); + { + // on startup we send a query to the terminal emulator for stuff like the pixel size and colors + // we get a response through STDIN, so it makes sense to do this here + let mut stdin_ansi_parser = stdin_ansi_parser.lock().unwrap(); + match stdin_ansi_parser.read_cache() { + Some(events) => { + let _ = send_input_instructions + .send(InputInstruction::AnsiStdinInstructions(events)); + let _ = send_input_instructions + .send(InputInstruction::DoneParsing) + .unwrap(); + }, + None => { + send_input_instructions + .send(InputInstruction::StartedParsing) + .unwrap(); + let terminal_emulator_query_string = stdin_ansi_parser + .terminal_emulator_query_string(); + let _ = os_input + .get_stdout_writer() + .write(terminal_emulator_query_string.as_bytes()) + .unwrap(); + let query_duration = stdin_ansi_parser.startup_query_duration(); + send_done_parsing_after_query_timeout(send_input_instructions.clone(), query_duration); + } + } + } + let mut ansi_stdin_events = vec![]; loop { let buf = os_input.read_from_stdin(); { @@ -54,12 +67,16 @@ pub(crate) fn stdin_loop( if stdin_ansi_parser.should_parse() { let events = stdin_ansi_parser.parse(buf); if !events.is_empty() { + ansi_stdin_events.append(&mut events.clone()); let _ = send_input_instructions .send(InputInstruction::AnsiStdinInstructions(events)); } continue; } } + if !ansi_stdin_events.is_empty() { + stdin_ansi_parser.lock().unwrap().write_cache(ansi_stdin_events.drain(..).collect()); + } current_buffer.append(&mut buf.to_vec()); let maybe_more = false; // read_from_stdin should (hopefully) always empty the STDIN buffer completely let mut events = vec![]; diff --git a/zellij-client/src/unit/stdin_tests.rs b/zellij-client/src/unit/stdin_tests.rs index c55283964c..6bdd489bc9 100644 --- a/zellij-client/src/unit/stdin_tests.rs +++ b/zellij-client/src/unit/stdin_tests.rs @@ -317,98 +317,3 @@ pub fn move_focus_left_in_normal_mode() { "All actions sent to server properly" ); } - -#[test] -pub fn terminal_info_queried_from_terminal_emulator() { - let events_sent_to_server = Arc::new(Mutex::new(vec![])); - let command_is_executing = CommandIsExecuting::new(); - let client_os_api = FakeClientOsApi::new(events_sent_to_server, command_is_executing); - - let client_os_api_clone = client_os_api.clone(); - let (send_input_instructions, _receive_input_instructions): ChannelWithContext< - InputInstruction, - > = channels::bounded(50); - let send_input_instructions = SenderWithContext::new(send_input_instructions); - let stdin_ansi_parser = Arc::new(Mutex::new(StdinAnsiParser::new())); - - let stdin_thread = thread::Builder::new() - .name("stdin_handler".to_string()) - .spawn({ - move || { - stdin_loop( - Box::new(client_os_api), - send_input_instructions, - stdin_ansi_parser, - ) - } - }); - std::thread::sleep(std::time::Duration::from_millis(500)); // wait for initial query to be sent - - let extracted_stdout_buffer = client_os_api_clone.stdout_buffer(); - let mut expected_query = - String::from("\u{1b}[14t\u{1b}[16t\u{1b}]11;?\u{1b}\u{5c}\u{1b}]10;?\u{1b}\u{5c}"); - for i in 0..256 { - expected_query.push_str(&format!("\u{1b}]4;{};?\u{1b}\u{5c}", i)); - } - assert_eq!( - String::from_utf8(extracted_stdout_buffer), - Ok(expected_query), - ); - drop(stdin_thread); -} - -#[test] -pub fn pixel_info_sent_to_server() { - let fake_stdin_buffer = read_fixture("terminal_emulator_startup_response"); - let events_sent_to_server = Arc::new(Mutex::new(vec![])); - let command_is_executing = CommandIsExecuting::new(); - let client_os_api = - FakeClientOsApi::new(events_sent_to_server.clone(), command_is_executing.clone()) - .with_stdin_buffer(fake_stdin_buffer); - let config = Config::from_default_assets().unwrap(); - let options = Options::default(); - - let (send_client_instructions, _receive_client_instructions): ChannelWithContext< - ClientInstruction, - > = channels::bounded(50); - let send_client_instructions = SenderWithContext::new(send_client_instructions); - - let (send_input_instructions, receive_input_instructions): ChannelWithContext< - InputInstruction, - > = channels::bounded(50); - let send_input_instructions = SenderWithContext::new(send_input_instructions); - let stdin_ansi_parser = Arc::new(Mutex::new(StdinAnsiParser::new())); - let stdin_thread = thread::Builder::new() - .name("stdin_handler".to_string()) - .spawn({ - let client_os_api = client_os_api.clone(); - move || { - stdin_loop( - Box::new(client_os_api), - send_input_instructions, - stdin_ansi_parser, - ) - } - }); - - let default_mode = InputMode::Normal; - let input_thread = thread::Builder::new() - .name("input_handler".to_string()) - .spawn({ - move || { - input_loop( - Box::new(client_os_api), - config, - options, - command_is_executing, - send_client_instructions, - default_mode, - receive_input_instructions, - ) - } - }); - std::thread::sleep(std::time::Duration::from_millis(1000)); // wait for initial query to be sent - assert_snapshot!(*format!("{:?}", events_sent_to_server.lock().unwrap())); - drop(stdin_thread); - drop(input_thread); -} diff --git a/zellij-server/src/route.rs b/zellij-server/src/route.rs index 3de62c092e..4b5295cf16 100644 --- a/zellij-server/src/route.rs +++ b/zellij-server/src/route.rs @@ -1,4 +1,5 @@ use std::sync::{Arc, RwLock}; +use std::collections::VecDeque; use crate::{ os_input_output::ServerOsApi, @@ -670,7 +671,7 @@ macro_rules! send_to_screen_or_retry_queue { None => { log::warn!("Server not ready, trying to place instruction in retry queue..."); if let Some(retry_queue) = $retry_queue.as_mut() { - retry_queue.push($instruction); + retry_queue.push_back($instruction); } Ok(()) }, @@ -686,7 +687,7 @@ pub(crate) fn route_thread_main( mut receiver: IpcReceiverWithContext, client_id: ClientId, ) -> Result<()> { - let mut retry_queue = vec![]; + let mut retry_queue = VecDeque::new(); let err_context = || format!("failed to handle instruction for client {client_id}"); 'route_loop: loop { match receiver.recv() { @@ -694,7 +695,7 @@ pub(crate) fn route_thread_main( err_ctx.update_thread_ctx(); let rlocked_sessions = session_data.read().to_anyhow().with_context(err_context)?; let handle_instruction = |instruction: ClientToServerMsg, - mut retry_queue: Option<&mut Vec>| + mut retry_queue: Option<&mut VecDeque>| -> Result { let mut should_break = false; match instruction { @@ -837,7 +838,7 @@ pub(crate) fn route_thread_main( } Ok(should_break) }; - for instruction_to_retry in retry_queue.drain(..) { + while let Some(instruction_to_retry) = retry_queue.pop_front() { log::warn!("Server ready, retrying sending instruction."); let should_break = handle_instruction(instruction_to_retry, None)?; if should_break { From 88cd7d3975bfcb0678424dbc17d8d04f8585decd Mon Sep 17 00:00:00 2001 From: Aram Drevekenin Date: Fri, 17 Feb 2023 18:42:10 +0100 Subject: [PATCH 2/2] style(fmt): rustfmt --- zellij-client/src/stdin_ansi_parser.rs | 31 +++++++++++++------------- zellij-client/src/stdin_handler.rs | 20 +++++++++++------ zellij-server/src/route.rs | 6 +++-- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/zellij-client/src/stdin_ansi_parser.rs b/zellij-client/src/stdin_ansi_parser.rs index 53591d7543..4891c4f73d 100644 --- a/zellij-client/src/stdin_ansi_parser.rs +++ b/zellij-client/src/stdin_ansi_parser.rs @@ -78,24 +78,23 @@ impl StdinAnsiParser { } pub fn read_cache(&self) -> Option> { let path = self.cache_dir_path(); - match OpenOptions::new() - .read(true) - .open(path.as_path()) { - Ok(mut file) => { - let mut json_cache = String::new(); - file.read_to_string(&mut json_cache).ok()?; - let instructions = serde_json::from_str::>(&json_cache).ok()?; - if instructions.is_empty() { - None - } else { - Some(instructions) - } - }, - Err(e) => { - log::error!("Failed to open STDIN cache file: {:?}", e); + match OpenOptions::new().read(true).open(path.as_path()) { + Ok(mut file) => { + let mut json_cache = String::new(); + file.read_to_string(&mut json_cache).ok()?; + let instructions = + serde_json::from_str::>(&json_cache).ok()?; + if instructions.is_empty() { None + } else { + Some(instructions) } - } + }, + Err(e) => { + log::error!("Failed to open STDIN cache file: {:?}", e); + None + }, + } } pub fn write_cache(&self, events: Vec) { let path = self.cache_dir_path(); diff --git a/zellij-client/src/stdin_handler.rs b/zellij-client/src/stdin_handler.rs index 95cb9d92de..01f1ab2327 100644 --- a/zellij-client/src/stdin_handler.rs +++ b/zellij-client/src/stdin_handler.rs @@ -33,8 +33,8 @@ pub(crate) fn stdin_loop( let mut stdin_ansi_parser = stdin_ansi_parser.lock().unwrap(); match stdin_ansi_parser.read_cache() { Some(events) => { - let _ = send_input_instructions - .send(InputInstruction::AnsiStdinInstructions(events)); + let _ = + send_input_instructions.send(InputInstruction::AnsiStdinInstructions(events)); let _ = send_input_instructions .send(InputInstruction::DoneParsing) .unwrap(); @@ -43,15 +43,18 @@ pub(crate) fn stdin_loop( send_input_instructions .send(InputInstruction::StartedParsing) .unwrap(); - let terminal_emulator_query_string = stdin_ansi_parser - .terminal_emulator_query_string(); + let terminal_emulator_query_string = + stdin_ansi_parser.terminal_emulator_query_string(); let _ = os_input .get_stdout_writer() .write(terminal_emulator_query_string.as_bytes()) .unwrap(); let query_duration = stdin_ansi_parser.startup_query_duration(); - send_done_parsing_after_query_timeout(send_input_instructions.clone(), query_duration); - } + send_done_parsing_after_query_timeout( + send_input_instructions.clone(), + query_duration, + ); + }, } } let mut ansi_stdin_events = vec![]; @@ -75,7 +78,10 @@ pub(crate) fn stdin_loop( } } if !ansi_stdin_events.is_empty() { - stdin_ansi_parser.lock().unwrap().write_cache(ansi_stdin_events.drain(..).collect()); + stdin_ansi_parser + .lock() + .unwrap() + .write_cache(ansi_stdin_events.drain(..).collect()); } current_buffer.append(&mut buf.to_vec()); let maybe_more = false; // read_from_stdin should (hopefully) always empty the STDIN buffer completely diff --git a/zellij-server/src/route.rs b/zellij-server/src/route.rs index 4b5295cf16..36799b5ada 100644 --- a/zellij-server/src/route.rs +++ b/zellij-server/src/route.rs @@ -1,5 +1,5 @@ -use std::sync::{Arc, RwLock}; use std::collections::VecDeque; +use std::sync::{Arc, RwLock}; use crate::{ os_input_output::ServerOsApi, @@ -695,7 +695,9 @@ pub(crate) fn route_thread_main( err_ctx.update_thread_ctx(); let rlocked_sessions = session_data.read().to_anyhow().with_context(err_context)?; let handle_instruction = |instruction: ClientToServerMsg, - mut retry_queue: Option<&mut VecDeque>| + mut retry_queue: Option< + &mut VecDeque, + >| -> Result { let mut should_break = false; match instruction {