diff options
author | Christian Duerr <contact@christianduerr.com> | 2021-04-17 23:20:13 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-04-17 23:20:13 +0000 |
commit | 28abb1f9c78ab316126bdf94e2ca12f034f1d8fd (patch) | |
tree | 623eabe9b4760fc4674f36a8f953e344b0e655e3 | |
parent | a312e415951fcb9156572f124b42f68c09f60ae9 (diff) | |
download | alacritty-28abb1f9c78ab316126bdf94e2ca12f034f1d8fd.tar.gz alacritty-28abb1f9c78ab316126bdf94e2ca12f034f1d8fd.zip |
Fix out of order terminal query responses
This forces all responses made to the PTY through the indirection of the
UI event loop, making sure that the writes to the PTY are in the same
order as the original requests.
This just delays all escape sequences by forcing them through the event
loop, ideally all responses which are not asynchronous (like a clipboard
read) would be made immediately. However since some escapes require
feedback from the UI to mutable structures like the config (e.g. color
query escapes), this would require additional locking.
Fixes #4872.
-rw-r--r-- | alacritty/src/event.rs | 1 | ||||
-rw-r--r-- | alacritty_terminal/src/ansi.rs | 89 | ||||
-rw-r--r-- | alacritty_terminal/src/event.rs | 4 | ||||
-rw-r--r-- | alacritty_terminal/src/event_loop.rs | 4 | ||||
-rw-r--r-- | alacritty_terminal/src/term/mod.rs | 29 | ||||
-rw-r--r-- | alacritty_terminal/tests/ref.rs | 4 |
6 files changed, 64 insertions, 67 deletions
diff --git a/alacritty/src/event.rs b/alacritty/src/event.rs index 612a0cc0..a895514f 100644 --- a/alacritty/src/event.rs +++ b/alacritty/src/event.rs @@ -1222,6 +1222,7 @@ impl<N: Notify + OnResize> Processor<N> { let text = format(processor.ctx.display.colors[index]); processor.ctx.write_to_pty(text.into_bytes()); }, + TerminalEvent::PtyWrite(text) => processor.ctx.write_to_pty(text.into_bytes()), TerminalEvent::MouseCursorDirty => processor.reset_mouse_cursor(), TerminalEvent::Exit => (), TerminalEvent::CursorBlinkingChange(_) => { diff --git a/alacritty_terminal/src/ansi.rs b/alacritty_terminal/src/ansi.rs index 44c92d3e..55492d36 100644 --- a/alacritty_terminal/src/ansi.rs +++ b/alacritty_terminal/src/ansi.rs @@ -2,7 +2,7 @@ use std::convert::TryFrom; use std::time::{Duration, Instant}; -use std::{io, iter, str}; +use std::{iter, str}; use log::{debug, trace}; use serde::{Deserialize, Serialize}; @@ -153,29 +153,27 @@ impl Processor { /// Process a new byte from the PTY. #[inline] - pub fn advance<H, W>(&mut self, handler: &mut H, byte: u8, writer: &mut W) + pub fn advance<H>(&mut self, handler: &mut H, byte: u8) where H: Handler, - W: io::Write, { if self.state.sync_state.timeout.is_none() { - let mut performer = Performer::new(&mut self.state, handler, writer); + let mut performer = Performer::new(&mut self.state, handler); self.parser.advance(&mut performer, byte); } else { - self.advance_sync(handler, byte, writer); + self.advance_sync(handler, byte); } } /// End a synchronized update. - pub fn stop_sync<H, W>(&mut self, handler: &mut H, writer: &mut W) + pub fn stop_sync<H>(&mut self, handler: &mut H) where H: Handler, - W: io::Write, { // Process all synchronized bytes. for i in 0..self.state.sync_state.buffer.len() { let byte = self.state.sync_state.buffer[i]; - let mut performer = Performer::new(&mut self.state, handler, writer); + let mut performer = Performer::new(&mut self.state, handler); self.parser.advance(&mut performer, byte); } @@ -198,16 +196,15 @@ impl Processor { /// Process a new byte during a synchronized update. #[cold] - fn advance_sync<H, W>(&mut self, handler: &mut H, byte: u8, writer: &mut W) + fn advance_sync<H>(&mut self, handler: &mut H, byte: u8) where H: Handler, - W: io::Write, { self.state.sync_state.buffer.push(byte); // Handle sync DCS escape sequences. match self.state.sync_state.pending_dcs { - Some(_) => self.advance_sync_dcs_end(handler, byte, writer), + Some(_) => self.advance_sync_dcs_end(handler, byte), None => self.advance_sync_dcs_start(), } } @@ -228,10 +225,9 @@ impl Processor { } /// Parse the DCS termination sequence for synchronized updates. - fn advance_sync_dcs_end<H, W>(&mut self, handler: &mut H, byte: u8, writer: &mut W) + fn advance_sync_dcs_end<H>(&mut self, handler: &mut H, byte: u8) where H: Handler, - W: io::Write, { match byte { // Ignore DCS passthrough characters. @@ -243,7 +239,7 @@ impl Processor { Some(Dcs::SyncStart) => { self.state.sync_state.timeout = Some(Instant::now() + SYNC_UPDATE_TIMEOUT); }, - Some(Dcs::SyncEnd) => self.stop_sync(handler, writer), + Some(Dcs::SyncEnd) => self.stop_sync(handler), None => (), }, } @@ -254,21 +250,16 @@ impl Processor { /// /// Processor creates a Performer when running advance and passes the Performer /// to `vte::Parser`. -struct Performer<'a, H: Handler, W: io::Write> { +struct Performer<'a, H: Handler> { state: &'a mut ProcessorState, handler: &'a mut H, - writer: &'a mut W, } -impl<'a, H: Handler + 'a, W: io::Write> Performer<'a, H, W> { +impl<'a, H: Handler + 'a> Performer<'a, H> { /// Create a performer. #[inline] - pub fn new<'b>( - state: &'b mut ProcessorState, - handler: &'b mut H, - writer: &'b mut W, - ) -> Performer<'b, H, W> { - Performer { state, handler, writer } + pub fn new<'b>(state: &'b mut ProcessorState, handler: &'b mut H) -> Performer<'b, H> { + Performer { state, handler } } } @@ -308,10 +299,10 @@ pub trait Handler { fn move_down(&mut self, _: usize) {} /// Identify the terminal (should write back to the pty stream). - fn identify_terminal<W: io::Write>(&mut self, _: &mut W, _intermediate: Option<char>) {} + fn identify_terminal(&mut self, _intermediate: Option<char>) {} /// Report device status. - fn device_status<W: io::Write>(&mut self, _: &mut W, _: usize) {} + fn device_status(&mut self, _: usize) {} /// Move cursor forward `cols`. fn move_forward(&mut self, _: Column) {} @@ -461,10 +452,10 @@ pub trait Handler { fn pop_title(&mut self) {} /// Report text area size in pixels. - fn text_area_size_pixels<W: io::Write>(&mut self, _: &mut W) {} + fn text_area_size_pixels(&mut self) {} /// Report text area size in characters. - fn text_area_size_chars<W: io::Write>(&mut self, _: &mut W) {} + fn text_area_size_chars(&mut self) {} } /// Terminal cursor configuration. @@ -883,10 +874,9 @@ impl StandardCharset { } } -impl<'a, H, W> vte::Perform for Performer<'a, H, W> +impl<'a, H> vte::Perform for Performer<'a, H> where H: Handler + 'a, - W: io::Write + 'a, { #[inline] fn print(&mut self, c: char) { @@ -1115,7 +1105,6 @@ where let mut params_iter = params.iter(); let handler = &mut self.handler; - let writer = &mut self.writer; let mut next_param_or = |default: u16| { params_iter.next().map(|param| param[0]).filter(|¶m| param != 0).unwrap_or(default) @@ -1136,7 +1125,7 @@ where }, ('C', []) | ('a', []) => handler.move_forward(Column(next_param_or(1) as usize)), ('c', intermediates) if next_param_or(0) == 0 => { - handler.identify_terminal(writer, intermediates.get(0).map(|&i| i as char)) + handler.identify_terminal(intermediates.get(0).map(|&i| i as char)) }, ('D', []) => handler.move_backward(Column(next_param_or(1) as usize)), ('d', []) => handler.goto_line(Line(next_param_or(1) as i32 - 1)), @@ -1218,7 +1207,7 @@ where } } }, - ('n', []) => handler.device_status(writer, next_param_or(0) as usize), + ('n', []) => handler.device_status(next_param_or(0) as usize), ('P', []) => handler.delete_chars(next_param_or(1) as usize), ('q', [b' ']) => { // DECSCUSR (CSI Ps SP q) -- Set Cursor Style. @@ -1249,8 +1238,8 @@ where ('s', []) => handler.save_cursor_position(), ('T', []) => handler.scroll_down(next_param_or(1) as usize), ('t', []) => match next_param_or(1) as usize { - 14 => handler.text_area_size_pixels(writer), - 18 => handler.text_area_size_chars(writer), + 14 => handler.text_area_size_pixels(), + 18 => handler.text_area_size_chars(), 22 => handler.push_title(), 23 => handler.pop_title(), _ => unhandled!(), @@ -1298,7 +1287,7 @@ where }, (b'H', []) => self.handler.set_horizontal_tabstop(), (b'M', []) => self.handler.reverse_index(), - (b'Z', []) => self.handler.identify_terminal(self.writer, None), + (b'Z', []) => self.handler.identify_terminal(None), (b'c', []) => self.handler.reset_state(), (b'0', intermediates) => { configure_charset!(StandardCharset::SpecialCharacterAndLineDrawing, intermediates) @@ -1494,11 +1483,9 @@ pub mod C0 { // Byte sequences used in these tests are recording of pty stdout. #[cfg(test)] mod tests { - use super::{ - parse_number, xparse_color, Attr, CharsetIndex, Color, Handler, Processor, StandardCharset, - }; + use super::*; + use crate::term::color::Rgb; - use std::io; struct MockHandler { index: CharsetIndex, @@ -1521,7 +1508,7 @@ mod tests { self.index = index; } - fn identify_terminal<W: io::Write>(&mut self, _: &mut W, _intermediate: Option<char>) { + fn identify_terminal(&mut self, _intermediate: Option<char>) { self.identity_reported = true; } @@ -1549,7 +1536,7 @@ mod tests { let mut handler = MockHandler::default(); for byte in BYTES { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert_eq!(handler.attr, Some(Attr::Bold)); @@ -1563,7 +1550,7 @@ mod tests { let mut handler = MockHandler::default(); for byte in bytes { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert!(!handler.identity_reported); @@ -1572,7 +1559,7 @@ mod tests { let bytes: &[u8] = &[0x1b, b'[', b'c']; for byte in bytes { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert!(handler.identity_reported); @@ -1581,7 +1568,7 @@ mod tests { let bytes: &[u8] = &[0x1b, b'[', b'0', b'c']; for byte in bytes { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert!(handler.identity_reported); @@ -1595,7 +1582,7 @@ mod tests { let mut handler = MockHandler::default(); for byte in bytes { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert!(handler.identity_reported); @@ -1607,7 +1594,7 @@ mod tests { let mut handler = MockHandler::default(); for byte in bytes { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert!(!handler.identity_reported); @@ -1625,7 +1612,7 @@ mod tests { let mut handler = MockHandler::default(); for byte in BYTES { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } let spec = Rgb { r: 128, g: 66, b: 255 }; @@ -1656,7 +1643,7 @@ mod tests { let mut parser = Processor::new(); for byte in BYTES { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } } @@ -1667,7 +1654,7 @@ mod tests { let mut handler = MockHandler::default(); for byte in BYTES { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert_eq!(handler.index, CharsetIndex::G0); @@ -1681,14 +1668,14 @@ mod tests { let mut handler = MockHandler::default(); for byte in &BYTES[..3] { - parser.advance(&mut handler, *byte, &mut io::sink()); + parser.advance(&mut handler, *byte); } assert_eq!(handler.index, CharsetIndex::G1); assert_eq!(handler.charset, StandardCharset::SpecialCharacterAndLineDrawing); let mut handler = MockHandler::default(); - parser.advance(&mut handler, BYTES[3], &mut io::sink()); + parser.advance(&mut handler, BYTES[3]); assert_eq!(handler.index, CharsetIndex::G1); } diff --git a/alacritty_terminal/src/event.rs b/alacritty_terminal/src/event.rs index 70d16127..fac7a56a 100644 --- a/alacritty_terminal/src/event.rs +++ b/alacritty_terminal/src/event.rs @@ -35,6 +35,9 @@ pub enum Event { /// expected escape sequence format. ColorRequest(usize, Arc<dyn Fn(Rgb) -> String + Sync + Send + 'static>), + /// Write some text to the PTY. + PtyWrite(String), + /// Cursor blinking state has changed. CursorBlinkingChange(bool), @@ -57,6 +60,7 @@ impl Debug for Event { Event::ClipboardStore(ty, text) => write!(f, "ClipboardStore({:?}, {})", ty, text), Event::ClipboardLoad(ty, _) => write!(f, "ClipboardLoad({:?})", ty), Event::ColorRequest(index, _) => write!(f, "ColorRequest({})", index), + Event::PtyWrite(text) => write!(f, "PtyWrite({})", text), Event::Wakeup => write!(f, "Wakeup"), Event::Bell => write!(f, "Bell"), Event::Exit => write!(f, "Exit"), diff --git a/alacritty_terminal/src/event_loop.rs b/alacritty_terminal/src/event_loop.rs index c3224dfe..098ee896 100644 --- a/alacritty_terminal/src/event_loop.rs +++ b/alacritty_terminal/src/event_loop.rs @@ -240,7 +240,7 @@ where // Run the parser. for byte in &buf[..got] { - state.parser.advance(&mut **terminal, *byte, &mut self.pty.writer()); + state.parser.advance(&mut **terminal, *byte); } // Exit if we've processed enough bytes. @@ -334,7 +334,7 @@ where // Handle synchronized update timeout. if events.is_empty() { - state.parser.stop_sync(&mut *self.terminal.lock(), &mut self.pty.writer()); + state.parser.stop_sync(&mut *self.terminal.lock()); self.event_proxy.send_event(Event::Wakeup); continue; } diff --git a/alacritty_terminal/src/term/mod.rs b/alacritty_terminal/src/term/mod.rs index c4425916..199fd207 100644 --- a/alacritty_terminal/src/term/mod.rs +++ b/alacritty_terminal/src/term/mod.rs @@ -3,7 +3,7 @@ use std::cmp::{max, min}; use std::ops::{Index, IndexMut, Range}; use std::sync::Arc; -use std::{io, mem, ptr, str}; +use std::{mem, ptr, str}; use bitflags::bitflags; use log::{debug, trace}; @@ -968,32 +968,35 @@ impl<T: EventListener> Handler for Term<T> { } #[inline] - fn identify_terminal<W: io::Write>(&mut self, writer: &mut W, intermediate: Option<char>) { + fn identify_terminal(&mut self, intermediate: Option<char>) { match intermediate { None => { trace!("Reporting primary device attributes"); - let _ = writer.write_all(b"\x1b[?6c"); + let text = String::from("\x1b[?6c"); + self.event_proxy.send_event(Event::PtyWrite(text)); }, Some('>') => { trace!("Reporting secondary device attributes"); let version = version_number(env!("CARGO_PKG_VERSION")); - let _ = writer.write_all(format!("\x1b[>0;{};1c", version).as_bytes()); + let text = format!("\x1b[>0;{};1c", version); + self.event_proxy.send_event(Event::PtyWrite(text)); }, _ => debug!("Unsupported device attributes intermediate"), } } #[inline] - fn device_status<W: io::Write>(&mut self, writer: &mut W, arg: usize) { + fn device_status(&mut self, arg: usize) { trace!("Reporting device status: {}", arg); match arg { 5 => { - let _ = writer.write_all(b"\x1b[0n"); + let text = String::from("\x1b[0n"); + self.event_proxy.send_event(Event::PtyWrite(text)); }, 6 => { let pos = self.grid.cursor.point; - let response = format!("\x1b[{};{}R", pos.line + 1, pos.column + 1); - let _ = writer.write_all(response.as_bytes()); + let text = format!("\x1b[{};{}R", pos.line + 1, pos.column + 1); + self.event_proxy.send_event(Event::PtyWrite(text)); }, _ => debug!("unknown device status query: {}", arg), }; @@ -1687,15 +1690,17 @@ impl<T: EventListener> Handler for Term<T> { } #[inline] - fn text_area_size_pixels<W: io::Write>(&mut self, writer: &mut W) { + fn text_area_size_pixels(&mut self) { let width = self.cell_width * self.columns(); let height = self.cell_height * self.screen_lines(); - let _ = write!(writer, "\x1b[4;{};{}t", height, width); + let text = format!("\x1b[4;{};{}t", height, width); + self.event_proxy.send_event(Event::PtyWrite(text)); } #[inline] - fn text_area_size_chars<W: io::Write>(&mut self, writer: &mut W) { - let _ = write!(writer, "\x1b[8;{};{}t", self.screen_lines(), self.columns()); + fn text_area_size_chars(&mut self) { + let text = format!("\x1b[8;{};{}t", self.screen_lines(), self.columns()); + self.event_proxy.send_event(Event::PtyWrite(text)); } } diff --git a/alacritty_terminal/tests/ref.rs b/alacritty_terminal/tests/ref.rs index e229b6d2..a9968736 100644 --- a/alacritty_terminal/tests/ref.rs +++ b/alacritty_terminal/tests/ref.rs @@ -2,7 +2,7 @@ use serde::Deserialize; use serde_json as json; use std::fs::{self, File}; -use std::io::{self, Read}; +use std::io::Read; use std::path::Path; use alacritty_terminal::ansi; @@ -108,7 +108,7 @@ fn ref_test(dir: &Path) { let mut parser = ansi::Processor::new(); for byte in recording { - parser.advance(&mut terminal, byte, &mut io::sink()); + parser.advance(&mut terminal, byte); } // Truncate invisible lines from the grid. |